Don't skip logic in in UpdateLifecyclePhasesInternal for kCompositingInputsClean

third_party/WebKit/Source/core/frame/FrameView.cpp

 /*
  * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
  *                     1999 Lars Knoll <knoll@kde.org>
  *                     1999 Antti Koivisto <koivisto@kde.org>
  *                     2000 Dirk Mueller <mueller@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  *           (C) 2006 Graham Dennis (graham.dennis@gmail.com)
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
@@ skipping 3089 matching lines @@
 
       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;
-      }
+      if (target_state >= DocumentLifecycle::kCompositingClean) {
+        ScrollContentsIfNeededRecursive();
 
-      ScrollContentsIfNeededRecursive();
-      DCHECK(RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled() ||
-             Lifecycle().GetState() >= DocumentLifecycle::kCompositingClean);
+        frame_->GetPage()
+            ->GlobalRootScrollerController()
+            .DidUpdateCompositing();
 
-      frame_->GetPage()->GlobalRootScrollerController().DidUpdateCompositing();
+        if (target_state >= DocumentLifecycle::kPrePaintClean) {

[flackr, 2017/04/28 19:38:18]

Can we unnest this condition if there's no follow-on work to be done?

[smcgruer, 2017/04/28 19:43:44]

Uh. Yes. I am silly.

+          if (!RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
+            InvalidateTreeIfNeededRecursive();
 
-      if (target_state >= DocumentLifecycle::kPrePaintClean) {
-        if (!RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
-          InvalidateTreeIfNeededRecursive();
+          if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
+            if (view.Compositor()->InCompositingMode())
+              GetScrollingCoordinator()->UpdateAfterCompositingChangeIfNeeded();
+          }
 
-        if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
-          if (view.Compositor()->InCompositingMode())
-            GetScrollingCoordinator()->UpdateAfterCompositingChangeIfNeeded();
+          if (LocalFrame* local_frame = frame_->LocalFrameRoot()) {
+            // This is needed since, at present, the ScrollingCoordinator
+            // doesn't send rects for oopif sub-frames.
+            // TODO(wjmaclean): Remove this pathway when ScrollingCoordinator
+            // operates on a per-frame basis. https://crbug.com/680606
+            GetFrame()
+                .GetPage()
+                ->GetChromeClient()
+                .UpdateEventRectsForSubframeIfNecessary(local_frame);
+          }
+          UpdateCompositedSelectionIfNeeded();
         }
 
-        if (LocalFrame* local_frame = frame_->LocalFrameRoot()) {
-          // This is needed since, at present, the ScrollingCoordinator doesn't
-          // send rects for oopif sub-frames.
-          // TODO(wjmaclean): Remove this pathway when ScrollingCoordinator
-          // operates on a per-frame basis. https://crbug.com/680606
-          GetFrame()
-              .GetPage()
-              ->GetChromeClient()
-              .UpdateEventRectsForSubframeIfNecessary(local_frame);
-        }
-        UpdateCompositedSelectionIfNeeded();
+        // TODO(pdr): prePaint should be under the "Paint" devtools timeline
+        // step for slimming paint v2.
+        if (target_state >= DocumentLifecycle::kPrePaintClean)

[flackr, 2017/04/28 19:38:18]

Unless my eyes deceive me, this is the identical check to the branch immeidately
above it and target_state doesn't change, we should just move the code up.

[smcgruer, 2017/04/28 19:43:44]

Done.

+          PrePaint();
       }
-
-      // TODO(pdr): prePaint should be under the "Paint" devtools timeline step
-      // for slimming paint v2.
-      if (target_state >= DocumentLifecycle::kPrePaintClean)
-        PrePaint();
     }
 
     if (target_state == DocumentLifecycle::kPaintClean) {
       if (!frame_->GetDocument()->Printing() ||
           RuntimeEnabledFeatures::printBrowserEnabled())
         PaintTree();
 
       if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
         Optional<CompositorElementIdSet> composited_element_ids =
             CompositorElementIdSet();
@@ skipping 2196 matching lines @@
 void FrameView::SetAnimationHost(
     std::unique_ptr<CompositorAnimationHost> host) {
   animation_host_ = std::move(host);
 }
 
 LayoutUnit FrameView::CaretWidth() const {
   return LayoutUnit(GetChromeClient()->WindowToViewportScalar(1));
 }
 
 }  // namespace blink