Delay scrollContents until the next paint

Source/core/frame/FrameView.cpp

Index: Source/core/frame/FrameView.cpp
diff --git a/Source/core/frame/FrameView.cpp b/Source/core/frame/FrameView.cpp
index 55a282301363347d3cfa190398315f57102a9a30..0e8f9071926834bbea5375a2f688d82931ef08a1 100644
--- a/Source/core/frame/FrameView.cpp
+++ b/Source/core/frame/FrameView.cpp
@@ -1261,10 +1261,6 @@ bool FrameView::useSlowRepaintsIfNotOverlapped() const
 
 bool FrameView::shouldAttemptToScrollUsingFastPath() const
 {
-    // FIXME: useSlowRepaints reads compositing state in parent frames. Compositing state on the parent
-    // frames is not necessarily up to date.
-    // https://code.google.com/p/chromium/issues/detail?id=343766
-    DisableCompositingQueryAsserts disabler;
     return !useSlowRepaints();
 }
 
@@ -1392,12 +1388,6 @@ bool FrameView::scrollContentsFastPath(const IntSize& scrollDelta, const IntRect
         return true;
     }
 
-    // https://code.google.com/p/chromium/issues/detail?id=343767
-    DisableCompositingQueryAsserts disabler;
-    const bool isCompositedContentLayer = contentsInCompositedLayer();
-
-    // Get the rects of the fixed objects visible in the rectToScroll
-    Region regionToUpdate;
     ViewportConstrainedObjectSet::const_iterator end = m_viewportConstrainedObjects->end();
     for (ViewportConstrainedObjectSet::const_iterator it = m_viewportConstrainedObjects->begin(); it != end; ++it) {
         RenderObject* renderer = *it;
@@ -1408,77 +1398,20 @@ bool FrameView::scrollContentsFastPath(const IntSize& scrollDelta, const IntRect
         ASSERT(renderer->hasLayer());
         RenderLayer* layer = toRenderBoxModelObject(renderer)->layer();
 
-        // Layers that paint into their ancestor or into a grouped backing will still need
-        // to apply a repaint invalidation. If the layer paints into its own backing, then
-        // it does not need repainting just to scroll.
-        if (layer->compositingState() == PaintsIntoOwnBacking)
-            continue;
-
-        if (layer->viewportConstrainedNotCompositedReason() == RenderLayer::NotCompositedForBoundsOutOfView
-            || layer->viewportConstrainedNotCompositedReason() == RenderLayer::NotCompositedForNoVisibleContent) {
-            // Don't invalidate for invisible fixed layers.
-            continue;
-        }
-
         if (layer->hasAncestorWithFilterOutsets()) {
             // If the fixed layer has a blur/drop-shadow filter applied on at least one of its parents, we cannot
             // scroll using the fast path, otherwise the outsets of the filter will be moved around the page.
             return false;
         }
-
-        IntRect updateRect = pixelSnappedIntRect(layer->repainter().repaintRectIncludingNonCompositingDescendants());
-
-        RenderLayer* enclosingCompositingLayer = layer->enclosingCompositingLayer(ExcludeSelf);
-        if (enclosingCompositingLayer && !enclosingCompositingLayer->renderer()->isRenderView()) {
-            // If the fixed-position layer is contained by a composited layer that is not its containing block,
-            // then we have to invlidate that enclosing layer, not the RenderView.
-            updateRect.moveBy(scrollPosition());
-            IntRect previousRect = updateRect;
-            previousRect.move(scrollDelta);
-            updateRect.unite(previousRect);
-            enclosingCompositingLayer->repainter().setBackingNeedsRepaintInRect(updateRect);
-        } else {
-            // Coalesce the repaints that will be issued to the renderView.
-            updateRect = contentsToRootView(updateRect);
-            if (!isCompositedContentLayer && clipsRepaints())
-                updateRect.intersect(rectToScroll);
-            if (!updateRect.isEmpty())
-                regionToUpdate.unite(updateRect);
-        }
     }
 
-    // 1) scroll
     hostWindow()->scroll(scrollDelta, rectToScroll, clipRect);
 
-    // 2) update the area of fixed objects that has been invalidated
-    Vector<IntRect> subRectsToUpdate = regionToUpdate.rects();
-    size_t viewportConstrainedObjectsCount = subRectsToUpdate.size();
-    for (size_t i = 0; i < viewportConstrainedObjectsCount; ++i) {
-        IntRect updateRect = subRectsToUpdate[i];
-        IntRect scrolledRect = updateRect;
-        scrolledRect.move(scrollDelta);
-        updateRect.unite(scrolledRect);
-        if (isCompositedContentLayer) {
-            updateRect = rootViewToContents(updateRect);
-            ASSERT(renderView());
-            renderView()->layer()->repainter().setBackingNeedsRepaintInRect(updateRect);
-            continue;
-        }
-        if (clipsRepaints())
-            updateRect.intersect(rectToScroll);
-        hostWindow()->invalidateContentsAndRootView(updateRect);
-    }
-
     return true;
 }
 
 void FrameView::scrollContentsSlowPath(const IntRect& updateRect)
 {
-    // FIXME: This is called when JS calls scrollTo, at which point there's no guarantee that
-    // compositing state is up to date.
-    // https://code.google.com/p/chromium/issues/detail?id=343767
-    DisableCompositingQueryAsserts disabler;
-
     if (contentsInCompositedLayer()) {
         IntRect updateRect = visibleContentRect();
         ASSERT(renderView());
@@ -1665,6 +1598,92 @@ void FrameView::didScrollTimerFired(Timer<FrameView>*)
     }
 }
 
+void FrameView::invalidateFixedElementsBeforeScrolling(const IntSize& scrollDelta, const IntRect& rectToScroll)
+{
+    // Invalidate the fixed position layer before updating the compositing state. If the fixed
+    // position layer gains its own backing after scrolling, the backing it formerly painted into
+    // will not get invalidated after scrolling, because at that point it already forgot about
+    // the previous compositing state.
+    // TODO: Add a way to read the stale compositing state to get rid of this disabler.
+    DisableCompositingQueryAsserts disabler;
+    if (!shouldAttemptToScrollUsingFastPath())
+        return;
+
+    if (!hasViewportConstrainedObjects())
+        return;
+
+    const bool isCompositedContentLayer = contentsInCompositedLayer();
+
+    Region regionToUpdate;
+    ViewportConstrainedObjectSet::const_iterator end = m_viewportConstrainedObjects->end();
+    for (ViewportConstrainedObjectSet::const_iterator it = m_viewportConstrainedObjects->begin(); it != end; ++it) {
+        RenderObject* renderer = *it;
+        if (!renderer->style()->hasViewportConstrainedPosition())
+            continue;
+
+        // Fixed items should always have layers.
+        ASSERT(renderer->hasLayer());
+        RenderLayer* layer = toRenderBoxModelObject(renderer)->layer();
+
+        // Layers that paint into their ancestor or into a grouped backing will still need
+        // to apply a repaint invalidation. If the layer paints into its own backing, then
+        // it does not need repainting just to scroll.
+        if (layer->compositingState() == PaintsIntoOwnBacking)
+            continue;
+
+        if (layer->viewportConstrainedNotCompositedReason() == RenderLayer::NotCompositedForBoundsOutOfView
+            || layer->viewportConstrainedNotCompositedReason() == RenderLayer::NotCompositedForNoVisibleContent) {
+            // Don't invalidate for invisible fixed layers.
+            continue;
+        }
+
+        if (layer->hasAncestorWithFilterOutsets()) {
+            // If the fixed layer has a blur/drop-shadow filter applied on at least one of its parents, we cannot
+            // scroll using the fast path, otherwise the outsets of the filter will be moved around the page.
+            return;
+        }
+
+        IntRect updateRect = pixelSnappedIntRect(layer->repainter().repaintRectIncludingNonCompositingDescendants());
+
+        RenderLayer* enclosingCompositingLayer = layer->enclosingCompositingLayer(ExcludeSelf);
+        if (enclosingCompositingLayer && !enclosingCompositingLayer->renderer()->isRenderView()) {
+            // If the fixed-position layer is contained by a composited layer that is not its containing block,
+            // then we have to invlidate that enclosing layer, not the RenderView.
+            updateRect.moveBy(scrollPosition());
+            IntRect previousRect = updateRect;
+            previousRect.move(scrollDelta);
+            updateRect.unite(previousRect);
+            enclosingCompositingLayer->repainter().setBackingNeedsRepaintInRect(updateRect);
+        } else {
+            // Coalesce the repaints that will be issued to the renderView.
+            updateRect = contentsToRootView(updateRect);
+            if (!isCompositedContentLayer && clipsRepaints())
+                updateRect.intersect(rectToScroll);
+            if (!updateRect.isEmpty())
+                regionToUpdate.unite(updateRect);
+        }
+    }
+
+    // update the area of fixed objects that has been invalidated
+    Vector<IntRect> subRectsToUpdate = regionToUpdate.rects();
+    size_t viewportConstrainedObjectsCount = subRectsToUpdate.size();
+    for (size_t i = 0; i < viewportConstrainedObjectsCount; ++i) {
+        IntRect updateRect = subRectsToUpdate[i];
+        IntRect scrolledRect = updateRect;
+        scrolledRect.move(scrollDelta);
+        updateRect.unite(scrolledRect);
+        if (isCompositedContentLayer) {
+            updateRect = rootViewToContents(updateRect);
+            ASSERT(renderView());
+            renderView()->layer()->repainter().setBackingNeedsRepaintInRect(updateRect);
+            continue;
+        }
+        if (clipsRepaints())
+            updateRect.intersect(rectToScroll);
+        hostWindow()->invalidateContentsAndRootView(updateRect);
+    }
+}
+
 void FrameView::repaintFixedElementsAfterScrolling()
 {
     RefPtr<FrameView> protect(this);
@@ -2752,6 +2771,7 @@ void FrameView::updateLayoutAndStyleForPainting()
     updateLayoutAndStyleIfNeededRecursive();
     if (RenderView* view = renderView())
         view->compositor()->updateCompositingLayers();
+    scrollContentsIfNeeded();
 }
 
 void FrameView::updateLayoutAndStyleIfNeededRecursive()