New issue
Advanced search Search tips

Issue 716150 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Early-exit in UpdateLifecyclePhasesInternal for kCompositingInputsClean skips required logic

Project Member Reported by smcgruer@chromium.org, Apr 27 2017

Issue description

If UpdateLifecyclePhasesInternal is called with a target_state of kCompositingInputsClean, we early-exit after the call to UpdateIfNeededRecursive:

...

      if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
        view.Compositor()->UpdateIfNeededRecursive(target_state);
      } else {
        ForAllNonThrottledFrameViews([](FrameView& frame_view) {
          frame_view.GetLayoutView()->Layer()->UpdateDescendantDependentFlags();
          frame_view.GetLayoutView()->CommitPendingSelection();
        });
      }

      if (target_state == DocumentLifecycle::kCompositingInputsClean) {
        DCHECK_EQ(Lifecycle().GetState(),
                  DocumentLifecycle::kCompositingInputsClean);
        return;
      }

This accidentally skips the following logic at the bottom of this function, which was intended to run for all lifecycles > kLayoutClean:

    ForAllNonThrottledFrameViews([](FrameView& frame_view) {
      frame_view.CheckDoesNotNeedLayout();
      frame_view.allows_layout_invalidation_after_layout_clean_ = true;
    });

This causes us to hit DCHECKs when targeting kCompositingInputsClean.
 
Summary: Early-exit in UpdateLifecyclePhasesInternal for kCompositingInputsClean skips required logic (was: UpdateLifecyclePhasesInternal doesnt reset allows_layout_invalidation_after_layout_clean_ for target_state == kCompositingInputsClean)
Actually, on reflection we also skip UpdateViewportIntersectionsForSubtree currently, updated title.
Project Member

Comment 2 by bugdroid1@chromium.org, May 3 2017

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

commit dc545e9fc108b42ff88ae4d696ea4b5ed3bffcc1
Author: smcgruer <smcgruer@chromium.org>
Date: Wed May 03 23:12:32 2017

Don't skip logic in in UpdateLifecyclePhasesInternal for kCompositingInputsClean

This was an oversight when I originally implemented kCompositingInputsUpdate;
the early-exit from UpdateLifecyclePhasesInternal meant that some necessary
logic was being skipped accidentally.

BUG= 716150 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/dc545e9fc108b42ff88ae4d696ea4b5ed3bffcc1/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/dc545e9fc108b42ff88ae4d696ea4b5ed3bffcc1/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositorTest.cpp

Status: Fixed (was: Started)

Sign in to add a comment