New issue
Advanced search Search tips

Issue 652905 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Telemetry benchmarks start scrolling before page is initialized

Project Member Reported by bokan@chromium.org, Oct 5 2016

Issue description

I noted this while investigating why my patch in https://codereview.chromium.org/2365793002/ regressed the scheduler.tough_scheduling_cases benchmark in issue 650029. It turns out that benchmark starts scrolling before the page is completely initialized. This means that the ScrollBegin comes in and we target the current outer viewport scroll layer. After this, the active tree receives a new outer viewport causing us to lose the current scrolling layer and thus dropping the scrolls on the floor.

Previously, we'd target the inner viewport which doesn't change so this wasn't a problem. Really though, there's no reason to treat the viewport as special. We shouldn't start sending events until the active tree is fully initialized.
 
How do you define "page is completely initialized"? 

Comment 2 by bokan@chromium.org, Oct 5 2016

All the layers for the page are ready to scroll on the compositor's active tree. Currently, it could be that we wait until the main thread/Blink says the page is loaded, but we start sending scroll events to the compositor before we've committed those changes. This means we could be scrolling layers belonging to the previous page and could be a source of flakiness (in addition to making my change above more painful).
So you suggest we wait for things like DOMContentLoaded?

Comment 4 by bokan@chromium.org, Oct 5 2016

I'm not sure what the solution is yet, but I doubt there's a web-API available for this. We'd likely need to add a private hook somewhere or some small delay, I'll investigate.

Comment 5 by bokan@chromium.org, Oct 7 2016

Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 11 2016

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

commit eb735dcc1ff10e3c333345bd0902c1baed793543
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Tue Oct 11 17:06:48 2016

Roll src/third_party/catapult/ a1fdeaa2c..840e04295 (2 commits).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/a1fdeaa2c402..840e04295a00

$ git log a1fdeaa2c..840e04295 --date=short --no-merges --format='%ad %ae %s'
2016-10-11 nednguyen Output error message & exit gracefully when telemetry cannot find the browser
2016-10-11 bokan Wait a frame before finishing navigation

BUG= 652905 

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=catapult-sheriff@chromium.org

Review-Url: https://codereview.chromium.org/2407323002
Cr-Commit-Position: refs/heads/master@{#424463}

[modify] https://crrev.com/eb735dcc1ff10e3c333345bd0902c1baed793543/DEPS

Comment 7 by bokan@chromium.org, Oct 21 2016

Status: Fixed (was: Started)

Sign in to add a comment