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<LayoutPhase>)
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 <b.brachaczek@gmail.com>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@digia.com>

2 files changed, 47 insertions, 15 deletions

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<LayoutPhase> layoutPhaseRestorer(m_layoutPhase, InPreLayout);
+
     // Protect the view from being deleted during layout (in recalcStyle)
     RefPtr<FrameView> 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<bool> 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<bool> 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<bool> 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()) {
diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
index 37d1897bd..24f0c2a27 100644
--- a/Source/WebCore/page/FrameView.h
+++ b/Source/WebCore/page/FrameView.h
@@ -105,7 +105,7 @@ public:
     void scheduleRelayoutOfSubtree(RenderObject*);
     void unscheduleRelayout();
     bool layoutPending() const;
-    bool isInLayout() const { return m_inLayout; }
+    bool isInLayout() const { return m_layoutPhase == InLayout; }
 
     RenderObject* layoutRoot(bool onlyDuringLayout = false) const;
     void clearLayoutRoot() { m_layoutRoot = 0; }
@@ -452,6 +452,18 @@ private:
     void reset();
     void init();
 
+    enum LayoutPhase {
+        OutsideLayout,
+        InPreLayout,
+        InPreLayoutStyleUpdate,
+        InLayout,
+        InViewSizeAdjust,
+        InPostLayout,
+    };
+    LayoutPhase layoutPhase() const { return m_layoutPhase; }
+
+    bool inPreLayoutStyleUpdate() const { return m_layoutPhase == InPreLayoutStyleUpdate; }
+
     virtual bool isFrameView() const OVERRIDE { return true; }
 
     friend class RenderWidget;
@@ -567,10 +579,9 @@ private:
     Timer<FrameView> m_layoutTimer;
     bool m_delayedLayout;
     RenderObject* m_layoutRoot;
-    
+
+    LayoutPhase m_layoutPhase;
     bool m_layoutSchedulingEnabled;
-    bool m_inLayout;
-    bool m_doingPreLayoutStyleUpdate;
     bool m_inSynchronousPostLayout;
     int m_layoutCount;
     unsigned m_nestedLayoutCount;