New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 837105 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

UpdatePolicy infinite loop due to DidHandleInputEventOnMainThread being not properly called

Reported by yuyi...@xiaomi.com, Apr 26 2018

Issue description

Steps to reproduce the problem:
1. with Chrome for Android, open a scrollable page with touch handler (so that touch events are forwarded to blink main thread), e.g. https://jsfiddle.net/h62yjty4/show/, 
2. scroll up or down several times
3. put Chrome to background, play with the home screen for ~1min

What is the expected behavior?
There should be no background activity for Chrome.

What went wrong?
Chrome appears on 'adb shell top' once in a while, consuming ~1% cpu usage. Tracing shows that UpdatePolicy is repeatedly getting called on renderer, as attached.

Did this work before? N/A 

Chrome version: 66.0.3359.126  Channel: stable
OS Version: 8.0
Flash Version: 

It is expected by the MainThreadScheduler that every event forwarded to the main thread results in a corresponding call of DidHandleInputEventOnMainThread, but for coalesced events only the blocking ones are getting the callback:

https://cs.chromium.org/chromium/src/content/renderer/input/main_thread_event_queue.cc?q=main_thread_event&sq=package:chromium&l=128

In turn, UserModel has its pending_input_event_count_ never get zeroed, and when UpdatePolicy, the wrong user model makes the scheduler repeatedly wake up itself.

The issue can be fixed by simply add the number of non-blocking coalesced events in the code above, but I noticed that the call times are explicitly tested in unit tests. So I wonder if there is any specific reason for distinguishing non-blocking events. If that's the case I suppose this should be left to the experts.
 
trace_repeated_UpdatePolicy_on_CrRendererMain.json
159 KB View Download
Cc: nzolghadr@chromium.org
Components: Blink>Scheduling Blink>Input
Labels: Needs-triage-Mobile
Cc: sandeepkumars@chromium.org
Labels: Needs-Feedback Triaged-Mobile
Tested the issue using #66.0.3359.126 on Android 8.1.0 Pixel 2 as per the steps mentioned below. Did not observe such behavior

Steps:
1. Launched Browser
2. Navigated to https://jsfiddle.net/h62yjty4/show/,
3. Scrolled up or down 
4. Kept Chrome in background
5. Navigated to homescreen and played for 1 min.

@yuyifei: Could you please find the above observations and attach a screencast for further triaging of the issue?

Thanks!!

Comment 4 by yuyi...@xiaomi.com, Apr 27 2018

@sandeepkumars: I find your reproducing steps precise, but it might be easier to see the issue if one can add a simple line of log to the function MainThreadSchedulerImpl::UpdatePolicy(); it's being called repeatedly.

Or if you do a tracing or open the attached tracing file above, you see something like this picture: the CrRendererMain thread is activated for ~ every 100ms.

Cheers.
Screenshot_note_CrRendererMain.png
51.7 KB View Download
Project Member

Comment 5 by sheriffbot@chromium.org, Apr 27 2018

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by yuyi...@xiaomi.com, Apr 27 2018

I've uploaded a CL at: https://chromium-review.googlesource.com/1032468 for reference, also made corresponding changes for unit test to pass, though it might well break other things.

Comment 7 by bokan@chromium.org, May 3 2018

Cc: bokan@chromium.org
Owner: dtapu...@chromium.org
Status: Assigned (was: Unconfirmed)
Assigning to dtapuska@ as he's reviewing the patch and I can't assign to yuyifei@.
Project Member

Comment 8 by bugdroid1@chromium.org, May 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/473e303db226e5395476c35e4de482922a800cfd

commit 473e303db226e5395476c35e4de482922a800cfd
Author: Yifei Yu <yuyifei@xiaomi.com>
Date: Fri May 04 16:14:20 2018

Balance scheduler callbacks for forwarded events.

The scheduler expects balanced callbacks of DidHandleInputEventOnMainThread()
and DidHandleInputEventOnCompositorThread(EVENT_FORWARDED_TO_MAIN_THREAD).
If the expectation is broken, the wrongly-maintained UserModel would produce
an endless loop of UpdatePolicy(), which consumes device power.

Also makes corresponding changes to unit test.

Bug:  837105 
Change-Id: Ib5193041728a97f055df757a33680e788dd4c876
Reviewed-on: https://chromium-review.googlesource.com/1032468
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556077}
[modify] https://crrev.com/473e303db226e5395476c35e4de482922a800cfd/AUTHORS
[modify] https://crrev.com/473e303db226e5395476c35e4de482922a800cfd/content/renderer/input/main_thread_event_queue.cc
[modify] https://crrev.com/473e303db226e5395476c35e4de482922a800cfd/content/renderer/input/main_thread_event_queue.h
[modify] https://crrev.com/473e303db226e5395476c35e4de482922a800cfd/content/renderer/input/main_thread_event_queue_unittest.cc
[modify] https://crrev.com/473e303db226e5395476c35e4de482922a800cfd/content/renderer/input/widget_input_handler_manager.cc

Status: Fixed (was: Assigned)

Sign in to add a comment