Simplify and solidify FrameView dirty layout check

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 147 matching lines @@
     , m_inUpdateScrollbars(false)
     , m_frameTimingRequestsDirty(true)
     , m_viewportIntersectionValid(false)
     , m_hiddenForThrottling(false)
     , m_crossOriginForThrottling(false)
     , m_subtreeThrottled(false)
     , m_currentUpdateLifecyclePhasesTargetState(DocumentLifecycle::Uninitialized)
     , m_scrollAnchor(this)
     , m_needsScrollbarsUpdate(false)
     , m_suppressAdjustViewSize(false)
-    , m_inPluginUpdate(false)
-    , m_inForcedLayoutByChildEmbeddedReplacedContent(false)
-    , m_allowsLayoutInvalidationAfterLayoutClean(false)
+    , m_allowsLayoutInvalidationAfterLayoutClean(true)
 {
     ASSERT(m_frame);
     init();
 }
 
 FrameView* FrameView::create(LocalFrame* frame)
 {
     FrameView* view = new FrameView(frame);
     view->show();
     return view;
@@ skipping 584 matching lines @@
         return;
 
     // If the embedded SVG document appears the first time, the ownerLayoutObject has already finished
     // layout without knowing about the existence of the embedded SVG document, because LayoutReplaced
     // embeddedReplacedContent() returns 0, as long as the embedded document isn't loaded yet. Before
     // bothering to lay out the SVG document, mark the ownerLayoutObject needing layout and ask its
     // FrameView for a layout. After that the LayoutEmbeddedObject (ownerLayoutObject) carries the
     // correct size, which LayoutSVGRoot::computeReplacedLogicalWidth/Height rely on, when laying
     // out for the first time, or when the LayoutSVGRoot size has changed dynamically (eg. via <script>).
     FrameView* frameView = ownerLayoutObject->frame()->view();
-    TemporaryChange<bool> t(frameView->m_inForcedLayoutByChildEmbeddedReplacedContent, true);
 
     // Mark the owner layoutObject as needing layout.
     ownerLayoutObject->setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation(LayoutInvalidationReason::Unknown);
 
     // Synchronously enter layout, to layout the view containing the host object/embed/iframe.
     ASSERT(frameView);
     frameView->layout();
 }
 
 void FrameView::performPreLayoutTasks()
@@ skipping 153 matching lines @@
     ASSERT(m_frame->page());
 
     ScriptForbiddenScope forbidScript;
 
     if (isInPerformLayout() || shouldThrottleRendering() || !m_frame->document()->isActive())
         return;
 
     TRACE_EVENT0("blink,benchmark", "FrameView::layout");
     TRACE_EVENT_SCOPED_SAMPLING_STATE("blink", "Layout");
 
-    TemporaryChange<bool> allowsLayoutInvalidation(m_allowsLayoutInvalidationAfterLayoutClean, true);
-
     if (m_autoSizeInfo)
         m_autoSizeInfo->autoSizeIfNeeded();
 
     m_hasPendingLayout = false;
     DocumentLifecycle::Scope lifecycleScope(lifecycle(), DocumentLifecycle::LayoutClean);
 
     TRACE_EVENT_BEGIN1("devtools.timeline", "Layout", "beginData", InspectorLayoutEvent::beginData(this));
 
     performPreLayoutTasks();
 
@@ skipping 268 matching lines @@
 
     for (auto part : parts) {
         // Script or plugins could detach the frame so abort processing if that happens.
         if (layoutViewItem().isNull())
             break;
 
         if (Widget* widget = part->widget()) {
             if (widget->isFrameView()) {
                 FrameView* frameView = toFrameView(widget);
                 bool didNeedLayout = frameView->needsLayout();
-                // LayoutPart::updateWidgetGeometry() may invalidate and update layout of the sub-FrameView. This is
-                // allowed, but layout should be clean after updateWidgetGeometry unless the FrameView is throttled.
-                TemporaryChange<bool> allowLayoutInvalidation(frameView->m_allowsLayoutInvalidationAfterLayoutClean, true);
                 part->updateWidgetGeometry();
                 if (!didNeedLayout && !frameView->shouldThrottleRendering())
                     frameView->checkDoesNotNeedLayout();
             } else {
                 part->updateWidgetGeometry();
             }
         }
     }
 }
 
@@ skipping 573 matching lines @@
             || !root->styleRef().logicalHeight().isIntrinsicOrAuto()) {
             continue;
         }
         LayoutState layoutState(*root);
         root->layout();
     }
 }
 
 void FrameView::checkLayoutInvalidationIsAllowed() const
 {
-    CHECK(!m_inPluginUpdate);
+    if (m_allowsLayoutInvalidationAfterLayoutClean)
+        return;
 
     if (!m_frame->document())
         return;
 
-    // TODO(crbug.com/442939): These are hacks to support embedded SVG. This is called from
-    // FrameView::forceLayoutParentViewIfNeeded() and the dirty layout will be cleaned up immediately.
-    // This is for the parent view of the view containing the embedded SVG.
-    if (m_inForcedLayoutByChildEmbeddedReplacedContent)
-        return;
-    // This is for the view containing the embedded SVG.
-    if (embeddedReplacedContent()) {
-        if (const LayoutObject* ownerLayoutObject = m_frame->ownerLayoutObject()) {
-            if (LocalFrame* frame = ownerLayoutObject->frame()) {
-                if (frame->view()->m_inForcedLayoutByChildEmbeddedReplacedContent)
-                    return;
-            }
-        }
-    }
-
-    CHECK(lifecycle().stateAllowsLayoutInvalidation());
-
-    if (m_allowsLayoutInvalidationAfterLayoutClean)
-        return;
-
     // If we are updating all lifecycle phases beyond LayoutClean, we don't expect dirty layout after LayoutClean.
-    if (FrameView* rootFrameView = m_frame->localFrameRoot()->view()) {
-        if (rootFrameView->m_currentUpdateLifecyclePhasesTargetState > DocumentLifecycle::LayoutClean)
-            CHECK(lifecycle().state() < DocumentLifecycle::LayoutClean);
-    }
+    CHECK(lifecycle().state() < DocumentLifecycle::LayoutClean);
 }
 
 void FrameView::scheduleRelayout()
 {
     DCHECK(m_frame->view() == this);
 
     if (!m_layoutSchedulingEnabled)
         return;
 
     checkLayoutInvalidationIsAllowed();
@@ skipping 649 matching lines @@
 
     updateStyleAndLayoutIfNeededRecursive();
     DCHECK(lifecycle().state() >= DocumentLifecycle::LayoutClean);
 
     if (targetState == DocumentLifecycle::LayoutClean) {
         updateViewportIntersectionsForSubtree(targetState);
         return;
     }
 
     if (LayoutViewItem view = layoutViewItem()) {
+        forAllNonThrottledFrameViews([](FrameView& frameView) {

[szager1, 2016/07/07 18:55:29]

It's probably a minor point, but this becomes O(N^2), because
forAllNonThrottledFrameViews recurses all the way down the frame tree, and
updateStyleAndLayoutIfNeededRecursive will recurse back into this method.

Probably not a big deal in practice, but maybe worth a comment.

[Xianzhu, 2016/07/07 19:06:06]

It seems wrong that this function is recursed. Trying by adding
CHECK(frameView.m_allowsLayoutInvalidationAfterLayoutClean) to see if there is
recursion.

[szager1, 2016/07/07 19:12:41]

Oh yes, you are right; never mind my comment.

+            frameView.checkDoesNotNeedLayout();
+            frameView.m_allowsLayoutInvalidationAfterLayoutClean = false;
+        });
+
         {
             TRACE_EVENT1("devtools.timeline", "UpdateLayerTree", "data", InspectorUpdateLayerTreeEvent::data(m_frame.get()));
 
             // This was required for slimming paint v1 but is only temporarily
             // needed for slimming paint v2.
             view.compositor()->updateIfNeededRecursive();
             scrollContentsIfNeededRecursive();
 
-            ASSERT(lifecycle().state() >= DocumentLifecycle::CompositingClean);
+            DCHECK(lifecycle().state() >= DocumentLifecycle::CompositingClean);
 
             if (targetState >= DocumentLifecycle::PrePaintClean) {
                 if (!RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
                     invalidateTreeIfNeededRecursive();
 
                 if (view.compositor()->inCompositingMode())
                     scrollingCoordinator()->updateAfterCompositingChangeIfNeeded();
 
                 updateCompositedSelectionIfNeeded();
             }
         }
 
         if (targetState >= DocumentLifecycle::PrePaintClean) {
             if (RuntimeEnabledFeatures::slimmingPaintV2Enabled())
                 updatePaintProperties();
         }
 
         if (targetState == DocumentLifecycle::PaintClean) {
             if (!m_frame->document()->printing())
                 synchronizedPaint();
 
             if (RuntimeEnabledFeatures::slimmingPaintV2Enabled())
                 pushPaintArtifactToCompositor();
 
-            ASSERT(!view.hasPendingSelection());
-            ASSERT((m_frame->document()->printing() && lifecycle().state() == DocumentLifecycle::PaintInvalidationClean)
+            DCHECK(!view.hasPendingSelection());
+            DCHECK((m_frame->document()->printing() && lifecycle().state() == DocumentLifecycle::PaintInvalidationClean)
                 || lifecycle().state() == DocumentLifecycle::PaintClean);
         }
+
+        forAllNonThrottledFrameViews([](FrameView& frameView) {
+            frameView.checkDoesNotNeedLayout();
+            frameView.m_allowsLayoutInvalidationAfterLayoutClean = true;
+        });
     }
 
     updateViewportIntersectionsForSubtree(targetState);
 }
 
 void FrameView::updatePaintProperties()
 {
     TRACE_EVENT0("blink", "FrameView::updatePaintProperties");
 
     ASSERT(RuntimeEnabledFeatures::slimmingPaintV2Enabled());
@@ skipping 104 matching lines @@
     // Mac actually tests for intersection with the dirty region and tries not to
     // update layout for frames that are outside the dirty region.  Not only does this seem
     // pointless (since those frames will have set a zero timer to layout anyway), but
     // it is also incorrect, since if two frames overlap, the first could be excluded from the dirty
     // region but then become included later by the second frame adding rects to the dirty region
     // when it lays out.
 
     m_frame->document()->updateStyleAndLayoutTree();
 
     CHECK(!shouldThrottleRendering());
+    CHECK(m_frame->document()->isActive());
+    CHECK(!m_nestedLayoutCount);

[szager1, 2016/07/07 18:55:29]

Nice!

 
     if (needsLayout())
         layout();
 
     checkDoesNotNeedLayout();
 
     // WebView plugins need to update regardless of whether the LayoutEmbeddedObject
     // that owns them needed layout.
     // TODO(leviw): This currently runs the entire lifecycle on plugin WebViews. We
     // should have a way to only run these other Documents to the same lifecycle stage
     // as this frame.
-    {
-        TemporaryChange<bool> t(m_inPluginUpdate, true);
-        const ChildrenWidgetSet* viewChildren = children();
-        for (const Member<Widget>& child : *viewChildren) {
-            if ((*child).isPluginContainer())
-                toPluginView(child.get())->updateAllLifecyclePhases();
-        }
-        checkDoesNotNeedLayout();
+    const ChildrenWidgetSet* viewChildren = children();
+    for (const Member<Widget>& child : *viewChildren) {
+        if ((*child).isPluginContainer())
+            toPluginView(child.get())->updateAllLifecyclePhases();
     }
+    checkDoesNotNeedLayout();
 
     // FIXME: Calling layout() shouldn't trigger script execution or have any
     // observable effects on the frame tree but we're not quite there yet.
     HeapVector<Member<FrameView>> frameViews;
     for (Frame* child = m_frame->tree().firstChild(); child; child = child->tree().nextSibling()) {
         if (!child->isLocalFrame())
             continue;
         if (FrameView* view = toLocalFrame(child)->view())
             frameViews.append(view);
     }
@@ skipping 1518 matching lines @@
     return m_subtreeThrottled || (m_hiddenForThrottling && m_crossOriginForThrottling);
 }
 
 LayoutBox& FrameView::boxForScrollControlPaintInvalidation() const
 {
     ASSERT(!layoutViewItem().isNull());
     return *layoutView();
 }
 
 } // namespace blink