From d0306ff74e5a3dfde81de3a6f688a178b22bd38c Mon Sep 17 00:00:00 2001 From: "simon.fraser@apple.com" Date: Wed, 13 Nov 2013 20:03:53 +0000 Subject: ASSERTION FAILED: m_repaintRect == renderer().clippedOverflowRectForRepaint(renderer().containerForRepaint()) after r135816 https://bugs.webkit.org/show_bug.cgi?id=103432 Reviewed by Dave Hyatt. RenderLayer caches repaint rects in m_repaintRect, and on updating layer positions after scrolling, asserts that the cached rect is correct. However, this assertion would sometimes fail if we were scrolling as a result of doing adjustViewSize() in the middle of layout, because we haven't updated layer positions post-layout yet. Fix by having the poorly named FrameView::repaintFixedElementsAfterScrolling() skip the layer updating if this FrameView is inside of adjusetViewSize() in layout. In order to know if we're inside view size adjusting, add a LayoutPhase member to FrameView, replacing two existing bools that track laying out state. Investigative work showed that there are many, many ways to re-enter FrameView::layout(), which makes it hard (but desirable) to more assertions about state changes, but indicated that saving and restoring the state (via TemporaryChange) was a good idea. * page/FrameView.cpp: (WebCore::FrameView::FrameView): (WebCore::FrameView::reset): (WebCore::FrameView::updateCompositingLayersAfterStyleChange): (WebCore::FrameView::layout): (WebCore::FrameView::repaintFixedElementsAfterScrolling): * page/FrameView.h: Change-Id: If79914ee00b9250831c935bca8e7dcccf485ecf8 git-svn-id: http://svn.webkit.org/repository/webkit/trunk@159218 268f45cc-cd09-0410-ab3c-d52691b4dbfc Reviewed-by: Bartosz Brachaczek Reviewed-by: Allan Sandfeld Jensen --- Source/WebCore/page/FrameView.cpp | 43 +++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 11 deletions(-) (limited to 'Source/WebCore/page/FrameView.cpp') diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp index 4823b48d0..56698c73b 100644 --- a/Source/WebCore/page/FrameView.cpp +++ b/Source/WebCore/page/FrameView.cpp @@ -174,6 +174,7 @@ FrameView::FrameView(Frame* frame) , m_canHaveScrollbars(true) , m_layoutTimer(this, &FrameView::layoutTimerFired) , m_layoutRoot(0) + , m_layoutPhase(OutsideLayout) , m_inSynchronousPostLayout(false) , m_postLayoutTasksTimer(this, &FrameView::postLayoutTimerFired) , m_isTransparent(false) @@ -275,8 +276,7 @@ void FrameView::reset() m_delayedLayout = false; m_doFullRepaint = true; m_layoutSchedulingEnabled = true; - m_inLayout = false; - m_doingPreLayoutStyleUpdate = false; + m_layoutPhase = OutsideLayout; m_inSynchronousPostLayout = false; m_layoutCount = 0; m_nestedLayoutCount = 0; @@ -768,7 +768,7 @@ void FrameView::updateCompositingLayersAfterStyleChange() return; // If we expect to update compositing after an incipient layout, don't do so here. - if (m_doingPreLayoutStyleUpdate || layoutPending() || renderView->needsLayout()) + if (inPreLayoutStyleUpdate() || layoutPending() || renderView->needsLayout()) return; // This call will make sure the cached hasAcceleratedCompositing is updated from the pref @@ -1136,9 +1136,13 @@ inline void FrameView::forceLayoutParentViewIfNeeded() void FrameView::layout(bool allowSubtree) { - if (m_inLayout) + if (isInLayout()) return; + // 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. + TemporaryChange layoutPhaseRestorer(m_layoutPhase, InPreLayout); + // Protect the view from being deleted during layout (in recalcStyle) RefPtr protector(this); @@ -1188,11 +1192,12 @@ void FrameView::layout(bool allowSubtree) if (!m_nestedLayoutCount && !m_inSynchronousPostLayout && m_postLayoutTasksTimer.isActive() && !inChildFrameLayoutWithFrameFlattening) { // This is a new top-level layout. If there are any remaining tasks from the previous // layout, finish them now. - m_inSynchronousPostLayout = true; + TemporaryChange inSynchronousPostLayoutChange(m_inSynchronousPostLayout, true); performPostLayoutTasks(); - m_inSynchronousPostLayout = false; } + m_layoutPhase = InPreLayoutStyleUpdate; + // Viewport-dependent media queries may cause us to need completely different style information. if (!document->styleResolverIfExists() || document->styleResolverIfExists()->affectedByViewportChange()) { document->styleResolverChanged(DeferRecalcStyle); @@ -1208,8 +1213,8 @@ void FrameView::layout(bool allowSubtree) // Always ensure our style info is up-to-date. This can happen in situations where // the layout beats any sort of style recalc update that needs to occur. - TemporaryChange changeDoingPreLayoutStyleUpdate(m_doingPreLayoutStyleUpdate, true); document->updateStyleIfNeeded(); + m_layoutPhase = InPreLayout; subtree = m_layoutRoot; @@ -1275,6 +1280,7 @@ void FrameView::layout(bool allowSubtree) m_lastViewportSize = fixedLayoutSize(); else m_lastViewportSize = visibleContentRect(IncludeScrollbars).size(); + m_lastZoomFactor = root->style()->zoom(); // Set the initial vMode to AlwaysOn if we're auto. @@ -1305,6 +1311,8 @@ void FrameView::layout(bool allowSubtree) rootRenderer->setChildNeedsLayout(true); } } + + m_layoutPhase = InPreLayout; } layer = root->enclosingLayer(); @@ -1320,9 +1328,14 @@ void FrameView::layout(bool allowSubtree) } LayoutStateDisabler layoutStateDisabler(disableLayoutState ? root->view() : 0); - m_inLayout = true; + ASSERT(m_layoutPhase == InPreLayout); + m_layoutPhase = InLayout; + beginDeferredRepaints(); forceLayoutParentViewIfNeeded(); + + ASSERT(m_layoutPhase == InLayout); + root->layout(); #if ENABLE(TEXT_AUTOSIZING) bool autosized = document->textAutosizer()->processSubtree(root); @@ -1330,7 +1343,8 @@ void FrameView::layout(bool allowSubtree) root->layout(); #endif endDeferredRepaints(); - m_inLayout = false; + + ASSERT(m_layoutPhase == InLayout); if (subtree) root->view()->popLayoutState(root); @@ -1338,11 +1352,15 @@ void FrameView::layout(bool allowSubtree) m_layoutRoot = 0; } // Reset m_layoutSchedulingEnabled to its previous value. + m_layoutPhase = InViewSizeAdjust; + bool neededFullRepaint = m_doFullRepaint; if (!subtree && !toRenderView(root)->printing()) adjustViewSize(); + m_layoutPhase = InPostLayout; + m_doFullRepaint = neededFullRepaint; // Now update the positions of all layers. @@ -1383,10 +1401,9 @@ void FrameView::layout(bool allowSubtree) if (RenderView* renderView = this->renderView()) renderView->updateWidgetPositions(); } else { - m_inSynchronousPostLayout = true; + TemporaryChange inSynchronousPostLayoutChange(m_inSynchronousPostLayout, true); // Calls resumeScheduledEvents() performPostLayoutTasks(); - m_inSynchronousPostLayout = false; } } @@ -2039,6 +2056,10 @@ void FrameView::scrollPositionChanged() // FIXME: this function is misnamed; its primary purpose is to update RenderLayer positions. void FrameView::repaintFixedElementsAfterScrolling() { + // If we're scrolling as a result of updating the view size after layout, we'll update widgets and layer positions soon anyway. + if (m_layoutPhase == InViewSizeAdjust) + return; + // For fixed position elements, update widget positions and compositing layers after scrolling, // but only if we're not inside of layout. if (m_nestedLayoutCount <= 1 && hasViewportConstrainedObjects()) { -- cgit v1.2.1