Scrolling simple static pages should not create commits. |
|||||||||||
Issue descriptionChrome Version: 57.0.2987.133 (Official Build) (64-bit) OS: Linux What steps will reproduce the problem? (1) Open https://en.wikipedia.org/wiki/Cat (2) Start tracing web developer categories in chrome://tracing (3) Examine trace What is the expected result? The commits from ThreadProxy::BeginMainFrame should be aborted with: TRACE_EVENT_INSTANT0("cc", "EarlyOut_NoUpdates", TRACE_EVENT_SCOPE_THREAD); What happens instead? Instead it looks like we commit a new frame after the scroll even though there should be no changes. Please use labels and text to provide additional information. For graphics-related bugs, please copy/paste the contents of the about:gpu page at the end of this report.
,
Apr 5 2017
It doesn't look like it, this seems to happen on any page that I take a trace on.
,
Apr 5 2017
From chat:
TRACE_EVENT_INSTANT0("cc", "EarlyOut_NoUpdates", TRACE_EVENT_SCOPE_THREAD); should happen. That's in proxy_main.cc https://cs.chromium.org/chromium/src/cc/trees/proxy_main.cc?rcl=56ab65b0a75d340fdb41bd5530d8c27bd2ad283e&l=226 when UpdateLayers() is false on 218. It should be possible to printf debug back from there where the true is coming from.
,
Apr 5 2017
,
Apr 5 2017
This seems to happen on windows (guessing all OS's) on a google search result as well.
,
Apr 5 2017
Seems to be that the scrollbar invalidation from Scrollbar::offsetDidChange causes us to invalidate paint setting the final pipeline stage to UPDATE_LAYERS_PIPELINE_STAGE and then the scrollbar layer paints: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/scroll/Scrollbar.cpp?sq=package:chromium&type=cs&q=Scrollbar::offsetDidChange&l=135 This must not have always been the case though, it seems pretty heavyweight to do a full property tree rebuild and commit every scroll.
,
Apr 5 2017
What are the invalidParts there? It used to be hardcoded to none until https://codereview.chromium.org/1591953005/diff/20001/third_party/WebKit/Source/platform/scroll/Scrollbar.cpp
,
Apr 5 2017
,
Apr 5 2017
It is still none, but it still invalidates on none.
,
Apr 5 2017
Updating layers shouldn't on its own be sufficient for triggering a property tree rebuild. Something must be setting needs_rebuild on LayerTreeHost::property_trees_ (e.g. a call to Layer::SetPropertyTreesNeedRebuild).
,
Apr 5 2017
Hmm, property_trees->needs_rebuild does seem to be false on my test actually. we still record it in tracing regardless as LayerTreeHostInProcess::UpdateLayers::BuildPropertyTrees: https://cs.chromium.org/chromium/src/cc/trees/layer_tree_host.cc?type=cs&q=LayerTreeHostInProcess::UpdateLayers::BuildPropertyTrees&sq=package:chromium&l=681
,
Apr 11 2017
Is this really a P1? If so, it needs to be fixed within 30 days.
,
Apr 12 2017
Assigning to flackr to drive priority and bug ownership.
,
Apr 14 2017
,
May 2 2017
I have no reason to suspect this is a regression. Dana, you said that we shouldn't be producing commits for updating the scrollbar?
,
May 2 2017
Sorry, should elaborate, given that this is the behavior on stable and older versions of chrome I have no reason to suspect it is a recent regression that we should block releases on.
,
May 2 2017
ajuma pointed out that we make commits to send more recordings periodically, but that is not related to the scrollbar. If the page fits within the recording viewport then there would be no reason for commits, but cats does not. |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by chrishtr@chromium.org
, Apr 5 2017