Modified: trunk/Source/WebCore/ChangeLog (223648 => 223649)
--- trunk/Source/WebCore/ChangeLog 2017-10-19 02:14:55 UTC (rev 223648)
+++ trunk/Source/WebCore/ChangeLog 2017-10-19 02:22:45 UTC (rev 223649)
@@ -1,3 +1,18 @@
+2017-10-18 Zalan Bujtas <[email protected]>
+
+ [FrameView::layout cleanup] Group related pre-layout code to improve readability
+ https://bugs.webkit.org/show_bug.cgi?id=178496
+ <rdar://problem/35065718>
+
+ Reviewed by Simon Fraser.
+
+ Early returns/asserts/member variable resets etc.
+
+ Covered by existing tests.
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::layout):
+
2017-10-17 Jiewen Tan <[email protected]>
Replace some stack raw pointers with RefPtrs within WebCore/html
Modified: trunk/Source/WebCore/page/FrameView.cpp (223648 => 223649)
--- trunk/Source/WebCore/page/FrameView.cpp 2017-10-19 02:14:55 UTC (rev 223648)
+++ trunk/Source/WebCore/page/FrameView.cpp 2017-10-19 02:22:45 UTC (rev 223649)
@@ -1349,59 +1349,51 @@
LOG(Layout, "FrameView %p (%dx%d) layout, main frameview %d, allowSubtreeLayout=%d", this, size().width(), size().height(), frame().isMainFrame(), allowSubtreeLayout);
if (isInRenderTreeLayout()) {
- LOG(Layout, " in layout, bailing");
+ LOG(Layout, " in render tree layout, bailing");
return;
}
if (layoutDisallowed()) {
- LOG(Layout, " layout is disallowed, bailing");
+ LOG(Layout, " is disallowed, bailing");
return;
}
+ ASSERT(!isPainting());
+ if (isPainting()) {
+ LOG(Layout, " in painting, bailing");
+ return;
+ }
+ ASSERT(frame().view() == this);
+ ASSERT(frame().document());
+ ASSERT(frame().document()->pageCacheState() == Document::NotInPageCache);
// Protect the view from being deleted during layout (in recalcStyle).
Ref<FrameView> protectedThis(*this);
+ TraceScope tracingScope(LayoutStart, LayoutEnd);
+ InspectorInstrumentationCookie cookie = InspectorInstrumentation::willLayout(frame());
+ AnimationUpdateBlock animationUpdateBlock(&frame().animation());
// Many of the tasks performed during layout can cause this function to be re-entered,
// so save the layout phase now and restore it on exit.
SetForScope<LayoutPhase> layoutPhaseRestorer(m_layoutPhase, InPreLayout);
-
// Every scroll that happens during layout is programmatic.
SetForScope<bool> changeInProgrammaticScroll(m_inProgrammaticScroll, true);
-
- TraceScope tracingScope(LayoutStart, LayoutEnd);
- if (handleLayoutWithFrameFlatteningIfNeeded(allowSubtreeLayout))
- return;
-
+ m_layoutTimer.stop();
+ m_delayedLayout = false;
+ m_setNeedsLayoutWasDeferred = false;
+ if (!allowSubtreeLayout && m_subtreeLayoutRoot)
+ convertSubtreeLayoutToFullLayout();
#if PLATFORM(IOS)
if (updateFixedPositionLayoutRect())
allowSubtreeLayout = false;
#endif
- m_layoutTimer.stop();
- m_delayedLayout = false;
- m_setNeedsLayoutWasDeferred = false;
-
- // we shouldn't enter layout() while painting
- ASSERT(!isPainting());
- if (isPainting())
+ if (handleLayoutWithFrameFlatteningIfNeeded(allowSubtreeLayout))
return;
- InspectorInstrumentationCookie cookie = InspectorInstrumentation::willLayout(frame());
- AnimationUpdateBlock animationUpdateBlock(&frame().animation());
-
- if (!allowSubtreeLayout && m_subtreeLayoutRoot)
- convertSubtreeLayoutToFullLayout();
-
- ASSERT(frame().view() == this);
- ASSERT(frame().document());
-
Document& document = *frame().document();
- ASSERT(document.pageCacheState() == Document::NotInPageCache);
RenderElement* layoutRoot = nullptr;
- RenderLayer* layer = nullptr;
bool isSubtreeLayout = false;
-
{
SetForScope<bool> changeSchedulingEnabled(m_layoutSchedulingEnabled, false);
@@ -1493,27 +1485,22 @@
}
ASSERT(allowSubtreeLayout || !isSubtreeLayout);
- layer = layoutRoot->enclosingLayer();
- SubtreeLayoutStateMaintainer subtreeLayoutStateMaintainer(m_subtreeLayoutRoot);
-
- RenderView::RepaintRegionAccumulator repaintRegionAccumulator(&layoutRoot->view());
-
ASSERT(m_layoutPhase == InPreLayout);
- m_layoutPhase = InRenderTreeLayout;
forceLayoutParentViewIfNeeded();
- ASSERT(m_layoutPhase == InRenderTreeLayout);
+ SubtreeLayoutStateMaintainer subtreeLayoutStateMaintainer(m_subtreeLayoutRoot);
+ RenderView::RepaintRegionAccumulator repaintRegionAccumulator(&layoutRoot->view());
#ifndef NDEBUG
RenderTreeNeedsLayoutChecker checker(*layoutRoot);
#endif
+ m_layoutPhase = InRenderTreeLayout;
layoutRoot->layout();
+ ASSERT(m_layoutPhase == InRenderTreeLayout);
#if ENABLE(TEXT_AUTOSIZING)
applyTextSizingIfNeeded(*layoutRoot);
#endif
-
- ASSERT(m_layoutPhase == InRenderTreeLayout);
m_subtreeLayoutRoot = nullptr;
// Close block here to end the scope of changeSchedulingEnabled and SubtreeLayoutStateMaintainer.
}
@@ -1535,9 +1522,9 @@
layoutRoot->view().releaseProtectedRenderWidgets();
ASSERT(!layoutRoot->needsLayout());
+ auto* layoutRootEnclosingLayer = layoutRoot->enclosingLayer();
+ layoutRootEnclosingLayer->updateLayerPositionsAfterLayout(renderView()->layer(), updateLayerPositionFlags(layoutRootEnclosingLayer, isSubtreeLayout, m_needsFullRepaint));
- layer->updateLayerPositionsAfterLayout(renderView()->layer(), updateLayerPositionFlags(layer, isSubtreeLayout, m_needsFullRepaint));
-
updateCompositingLayersAfterLayout();
m_layoutPhase = InPostLayerPositionsUpdatedAfterLayout;
