Source/core/frame/FrameView.cpp - Issue 927773002: Keep track of multiple layout roots

Unified Diff: Source/core/frame/FrameView.cpp

Issue 927773002: Keep track of multiple layout roots (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Remove bool Created 5 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: Source/core/frame/FrameView.cpp

diff --git a/Source/core/frame/FrameView.cpp b/Source/core/frame/FrameView.cpp

index a338e756e020c1eb3beb88cabe31f2c009c53052..096223dfca53078c9a751ab23ccf6fcc7bcef558 100644

--- a/Source/core/frame/FrameView.cpp

+++ b/Source/core/frame/FrameView.cpp

@@ -108,7 +108,6 @@ FrameView::FrameView(LocalFrame* frame)

, m_canHaveScrollbars(true)

, m_slowRepaintObjectCount(0)

, m_hasPendingLayout(false)

- , m_layoutSubtreeRoot(0)

, m_inSynchronousPostLayout(false)

, m_postLayoutTasksTimer(this, &FrameView::postLayoutTimerFired)

, m_updateWidgetsTimer(this, &FrameView::updateWidgetsTimerFired)

@@ -227,7 +226,6 @@ DEFINE_TRACE(FrameView)

void FrameView::reset()

{

m_hasPendingLayout = false;

- m_layoutSubtreeRoot = nullptr;

m_doFullPaintInvalidation = false;

m_layoutSchedulingEnabled = true;

m_inPerformLayout = false;

@@ -757,9 +755,32 @@ bool FrameView::isEnclosedInCompositingLayer() const

return frameOwnerRenderer && frameOwnerRenderer->enclosingLayer()->enclosingLayerForPaintInvalidationCrossingFrameBoundaries();

}

-LayoutObject* FrameView::layoutRoot(bool onlyDuringLayout) const

+static inline void countObjectsNeedingLayoutInRoot(const LayoutObject* root, unsigned& needsLayoutObjects, unsigned& totalObjects)

dsinclair 2015/02/27 16:12:09 Can the root be null? If not, can we make this Lay

{

- return onlyDuringLayout && layoutPending() ? nullptr : m_layoutSubtreeRoot;

+ for (const LayoutObject* o = root; o; o = o->nextInPreOrder(root)) {

+ ++totalObjects;

+ if (o->needsLayout())

+ ++needsLayoutObjects;

+ }

+void FrameView::countObjectsNeedingLayout(unsigned& needsLayoutObjects, unsigned& totalObjects, bool& isSubtree)

+ needsLayoutObjects = 0;

+ totalObjects = 0;

+ if (isSubtreeLayout()) {

+ isSubtree = true;

+ for (auto& subtreeRoot : m_layoutSubtreeRoots)

+ countObjectsNeedingLayoutInRoot(subtreeRoot, needsLayoutObjects, totalObjects);

+ } else {

+ isSubtree = false;

Julien - ping for review 2015/02/27 16:38:09 IMHO that would be more readable if it was outside

+ countObjectsNeedingLayoutInRoot(layoutView(), needsLayoutObjects, totalObjects);

+ }

+bool FrameView::isLayoutRoot(const LayoutObject& object) const

+ return m_layoutSubtreeRoots.contains(const_cast<LayoutObject*>(&object));

}

inline void FrameView::forceLayoutParentViewIfNeeded()

@@ -828,8 +849,43 @@ void FrameView::lineLayoutTime(double ms)

m_lineLayoutMs += ms;

}

-void FrameView::performLayout(LayoutObject* rootForThisLayout, bool inSubtreeLayout)

+static void gatherDebugLayoutRects(LayoutObject& layoutRoot)

+ bool isTracing;

+ TRACE_EVENT_CATEGORY_GROUP_ENABLED(TRACE_DISABLED_BY_DEFAULT("blink.debug.layout"), &isTracing);

+ if (!isTracing)

+ return;

+ if (!layoutRoot.enclosingLayer()->hasCompositedLayerMapping())

+ return;

+ // For access to compositedLayerMapping().

+ DisableCompositingQueryAsserts disabler;

+ GraphicsLayer* graphicsLayer = layoutRoot.enclosingLayer()->compositedLayerMapping()->mainGraphicsLayer();

+ if (!graphicsLayer)

+ return;

+ GraphicsLayerDebugInfo& debugInfo = graphicsLayer->debugInfo();

+ debugInfo.currentLayoutRects().clear();

+ for (LayoutObject* renderer = &layoutRoot; renderer; renderer = renderer->nextInPreOrder()) {

+ if (renderer->layoutDidGetCalledSinceLastFrame()) {

+ FloatQuad quad = renderer->localToAbsoluteQuad(FloatQuad(renderer->previousPaintInvalidationRect()));

+ LayoutRect rect = LayoutRect(quad.enclosingBoundingBox());

+ debugInfo.currentLayoutRects().append(rect);

+ }

+static inline void layoutFromRootObject(LayoutObject& root)

{

+ LayoutState layoutState(root);

+ root.layout();

+ gatherDebugLayoutRects(root);

+void FrameView::performLayout(bool inSubtreeLayout)

+ ASSERT(inSubtreeLayout || m_layoutSubtreeRoots.isEmpty());

m_lineLayoutMs = 0;

TRACE_EVENT0("blink,benchmark", "FrameView::performLayout");

double start = WTF::currentTimeMS();

@@ -845,12 +901,24 @@ void FrameView::performLayout(LayoutObject* rootForThisLayout, bool inSubtreeLay

// FIXME: The 300 other lines in layout() probably belong in other helper functions

// so that a single human could understand what layout() is actually doing.

- LayoutState layoutState(*rootForThisLayout);

forceLayoutParentViewIfNeeded();

- rootForThisLayout->layout();

- gatherDebugLayoutRects(rootForThisLayout);

+ if (inSubtreeLayout) {

+ while (m_layoutSubtreeRoots.size()) {

+ LayoutObject& root = *m_layoutSubtreeRoots.takeAny();

+ if (!root.needsLayout())

+ continue;

+ layoutFromRootObject(root);

Julien - ping for review 2015/02/27 16:38:09 We could unify the 2 code paths by inserting the R

leviw_travelin_and_unemployed 2015/02/27 20:09:28 That's my ultimate goal :)

+ // We need to ensure that we mark up all renderers up to the LayoutView

+ // for paint invalidation. This simplifies our code as we just always

+ // do a full tree walk.

+ if (LayoutObject* container = root.container())

+ container->setMayNeedPaintInvalidation();

+ }

+ } else {

+ layoutFromRootObject(*layoutView());

+ }

ResourceLoadPriorityOptimizer::resourceLoadPriorityOptimizer()->updateAllImageResourcePriorities();

@@ -926,7 +994,9 @@ void FrameView::layout()

Document* document = m_frame->document();

bool inSubtreeLayout = isSubtreeLayout();

- LayoutObject* rootForThisLayout = inSubtreeLayout ? m_layoutSubtreeRoot : document->layoutView();

+ // FIXME: The notion of a single root for layout is no longer applicable. Remove or update this code. crbug.com/460596

+ LayoutObject* rootForThisLayout = inSubtreeLayout ? *(m_layoutSubtreeRoots.begin()) : layoutView();

if (!rootForThisLayout) {

// FIXME: Do we need to set m_size here?

ASSERT_NOT_REACHED();

@@ -934,13 +1004,12 @@ void FrameView::layout()

}

FontCachePurgePreventer fontCachePurgePreventer;

- Layer* layer;

{

TemporaryChange<bool> changeSchedulingEnabled(m_layoutSchedulingEnabled, false);

m_nestedLayoutCount++;

if (!inSubtreeLayout) {

- Document* document = m_frame->document();

+ clearLayoutSubtreeRootsAndMarkContainingBlocks();

Node* body = document->body();

if (body && body->renderer()) {

if (isHTMLFrameSetElement(*body)) {

@@ -969,7 +1038,7 @@ void FrameView::layout()

m_firstLayout = false;

m_firstLayoutCallbackPending = true;

m_lastViewportSize = layoutSize(IncludeScrollbars);

- m_lastZoomFactor = rootForThisLayout->style()->zoom();

+ m_lastZoomFactor = layoutView()->style()->zoom();

// Set the initial vMode to AlwaysOn if we're auto.

if (vMode == ScrollbarAuto)

@@ -1005,43 +1074,36 @@ void FrameView::layout()

m_doFullPaintInvalidation |= layoutView()->shouldDoFullPaintInvalidationForNextLayout();

}

- layer = rootForThisLayout->enclosingLayer();

- performLayout(rootForThisLayout, inSubtreeLayout);

+ performLayout(inSubtreeLayout);

- m_layoutSubtreeRoot = nullptr;

- // We need to ensure that we mark up all renderers up to the LayoutView

- // for paint invalidation. This simplifies our code as we just always

- // do a full tree walk.

- if (LayoutObject* container = rootForThisLayout->container())

- container->setMayNeedPaintInvalidation();

+ ASSERT(m_layoutSubtreeRoots.isEmpty());

} // Reset m_layoutSchedulingEnabled to its previous value.

- if (!inSubtreeLayout && !toLayoutView(rootForThisLayout)->document().printing())

+ if (!inSubtreeLayout && !document->printing())

adjustViewSize();

- layer->updateLayerPositionsAfterLayout();

+ // FIXME: Could find the common ancestor layer of all dirty subtrees and mark from there.

dsinclair 2015/02/27 16:12:09 crbug link?

+ layoutView()->enclosingLayer()->updateLayerPositionsAfterLayout();

layoutView()->compositor()->didLayout();

m_layoutCount++;

- if (AXObjectCache* cache = rootForThisLayout->document().axObjectCache()) {

- const KURL& url = rootForThisLayout->document().url();

+ if (AXObjectCache* cache = document->axObjectCache()) {

+ const KURL& url = document->url();

if (url.isValid() && !url.isAboutBlankURL())

- cache->handleLayoutComplete(rootForThisLayout);

+ cache->handleLayoutComplete(document);

}

updateAnnotatedRegions();

- ASSERT(!rootForThisLayout->needsLayout());

if (document->hasListenerType(Document::OVERFLOWCHANGED_LISTENER))

updateOverflowStatus(layoutSize().width() < contentsWidth(), layoutSize().height() < contentsHeight());

scheduleOrPerformPostLayoutTasks();

+ // FIXME: The notion of a single root for layout is no longer applicable. Remove or update this code. crbug.com/460596

TRACE_EVENT_END1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "Layout", "endData", InspectorLayoutEvent::endData(rootForThisLayout));

- InspectorInstrumentation::didLayout(m_frame.get(), rootForThisLayout);

+ InspectorInstrumentation::didLayout(m_frame.get());

m_nestedLayoutCount--;

if (m_nestedLayoutCount)

@@ -1049,7 +1111,7 @@ void FrameView::layout()

#if ENABLE(ASSERT)

// Post-layout assert that nobody was re-marked as needing layout during layout.

- document->layoutView()->assertSubtreeIsLaidOut();

+ layoutView()->assertSubtreeIsLaidOut();

#endif

// FIXME: It should be not possible to remove the FrameView from the frame/page during layout

@@ -1103,32 +1165,6 @@ DocumentLifecycle& FrameView::lifecycle() const

return m_frame->document()->lifecycle();

}

-void FrameView::gatherDebugLayoutRects(LayoutObject* layoutRoot)

- bool isTracing;

- TRACE_EVENT_CATEGORY_GROUP_ENABLED(TRACE_DISABLED_BY_DEFAULT("blink.debug.layout"), &isTracing);

- if (!isTracing)

- return;

- if (!layoutRoot->enclosingLayer()->hasCompositedLayerMapping())

- return;

- // For access to compositedLayerMapping().

- DisableCompositingQueryAsserts disabler;

- GraphicsLayer* graphicsLayer = layoutRoot->enclosingLayer()->compositedLayerMapping()->mainGraphicsLayer();

- if (!graphicsLayer)

- return;

- GraphicsLayerDebugInfo& debugInfo = graphicsLayer->debugInfo();

- debugInfo.currentLayoutRects().clear();

- for (LayoutObject* renderer = layoutRoot; renderer; renderer = renderer->nextInPreOrder()) {

- if (renderer->layoutDidGetCalledSinceLastFrame()) {

- FloatQuad quad = renderer->localToAbsoluteQuad(FloatQuad(renderer->previousPaintInvalidationRect()));

- LayoutRect rect = LayoutRect(quad.enclosingBoundingBox());

- debugInfo.currentLayoutRects().append(rect);

- }

LayoutBox* FrameView::embeddedContentBox() const

{

LayoutView* layoutView = this->layoutView();

@@ -1803,14 +1839,22 @@ void FrameView::handleLoadCompleted()

maintainScrollPositionAtAnchor(0);

}

+void FrameView::clearLayoutSubtreeRoot(const LayoutObject* root)

dsinclair 2015/02/27 16:12:09 Can this be a reference?

+ m_layoutSubtreeRoots.remove(const_cast<LayoutObject*>(root));

+void FrameView::clearLayoutSubtreeRootsAndMarkContainingBlocks()

+ for (auto& iter : m_layoutSubtreeRoots)

+ iter->markContainingBlocksForLayout(false);

+ m_layoutSubtreeRoots.clear();

void FrameView::scheduleRelayout()

{

ASSERT(m_frame->view() == this);

- if (isSubtreeLayout()) {

- m_layoutSubtreeRoot->markContainingBlocksForLayout(false);

- m_layoutSubtreeRoot = nullptr;

- }

if (!m_layoutSchedulingEnabled)

return;

if (!needsLayout())

@@ -1819,6 +1863,8 @@ void FrameView::scheduleRelayout()

return;

TRACE_EVENT_INSTANT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "InvalidateLayout", "data", InspectorInvalidateLayoutEvent::data(m_frame.get()));

+ clearLayoutSubtreeRootsAndMarkContainingBlocks();

if (m_hasPendingLayout)

return;

m_hasPendingLayout = true;

@@ -1827,15 +1873,6 @@ void FrameView::scheduleRelayout()

lifecycle().ensureStateAtMost(DocumentLifecycle::StyleClean);

}

-static bool isObjectAncestorContainerOf(LayoutObject* ancestor, LayoutObject* descendant)

- for (LayoutObject* r = descendant; r; r = r->container()) {

- if (r == ancestor)

- return true;

- }

- return false;

void FrameView::scheduleRelayoutOfSubtree(LayoutObject* relayoutRoot)

{

ASSERT(m_frame->view() == this);

@@ -1851,28 +1888,11 @@ void FrameView::scheduleRelayoutOfSubtree(LayoutObject* relayoutRoot)

return;

}

- if (layoutPending() || !m_layoutSchedulingEnabled) {

- if (m_layoutSubtreeRoot != relayoutRoot) {

- if (isObjectAncestorContainerOf(m_layoutSubtreeRoot, relayoutRoot)) {

- // Keep the current root

- relayoutRoot->markContainingBlocksForLayout(false, m_layoutSubtreeRoot);

- ASSERT(!m_layoutSubtreeRoot->container() || !m_layoutSubtreeRoot->container()->needsLayout());

- } else if (isSubtreeLayout() && isObjectAncestorContainerOf(relayoutRoot, m_layoutSubtreeRoot)) {

- // Re-root at relayoutRoot

- m_layoutSubtreeRoot->markContainingBlocksForLayout(false, relayoutRoot);

- m_layoutSubtreeRoot = relayoutRoot;

- ASSERT(!m_layoutSubtreeRoot->container() || !m_layoutSubtreeRoot->container()->needsLayout());

- } else {

- // Just do a full relayout

- if (isSubtreeLayout())

- m_layoutSubtreeRoot->markContainingBlocksForLayout(false);

- m_layoutSubtreeRoot = nullptr;

- relayoutRoot->markContainingBlocksForLayout(false);

- }

- } else if (m_layoutSchedulingEnabled) {

- m_layoutSubtreeRoot = relayoutRoot;

- ASSERT(!m_layoutSubtreeRoot->container() || !m_layoutSubtreeRoot->container()->needsLayout());

+ if (relayoutRoot == layoutView)

+ clearLayoutSubtreeRootsAndMarkContainingBlocks();

+ else

+ m_layoutSubtreeRoots.add(relayoutRoot);

+ if (m_layoutSchedulingEnabled) {

m_hasPendingLayout = true;

page()->animator().scheduleVisualUpdate(m_frame.get());

« Source/core/frame/FrameView.h ('K') | « Source/core/frame/FrameView.h ('k') | Source/core/frame/LocalFrame.h » ('j') | no next file with comments »