Title: [185118] trunk/Source/WebCore
Revision
185118
Author
cdu...@apple.com
Date
2015-06-02 13:20:41 -0700 (Tue, 02 Jun 2015)

Log Message

Calling FrameView::viewportContentsChanged() after style recalcs is too expensive
https://bugs.webkit.org/show_bug.cgi?id=145554
<rdar://problem/21189478>

Reviewed by Darin Adler and Simon Fraser.

Only call FrameView::viewportContentsChanged() after a style recalc if
composited layers have been updated (and there is no pending layout).

We already viewportContentsChanged() after layout so we only need to
call viewportContentsChanged() after a style recalc if it did not cause
a layout but may have caused an element to become visible. In
particular, this can happen in the case of composited animations (e.g.
using -webkit-transform to move an element inside the viewport).
Therefore, we now only call viewportContentsChanged() after a style
recalc if it caused composited layers to be updated. This avoids a lot
of unnecessary calls to viewportContentsChanged(), which is expensive.

No new tests, already covered by:
fast/images/animated-gif-webkit-transform.html

* dom/Document.cpp:
(WebCore::Document::recalcStyle):
* page/FrameView.cpp:
(WebCore::FrameView::updateCompositingLayersAfterStyleChange):
* page/FrameView.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::didRecalcStyleWithNoPendingLayout):
(WebCore::RenderLayerCompositor::updateCompositingLayers):
* rendering/RenderLayerCompositor.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (185117 => 185118)


--- trunk/Source/WebCore/ChangeLog	2015-06-02 20:19:08 UTC (rev 185117)
+++ trunk/Source/WebCore/ChangeLog	2015-06-02 20:20:41 UTC (rev 185118)
@@ -1,3 +1,36 @@
+2015-06-02  Chris Dumez  <cdu...@apple.com>
+
+        Calling FrameView::viewportContentsChanged() after style recalcs is too expensive
+        https://bugs.webkit.org/show_bug.cgi?id=145554
+        <rdar://problem/21189478>
+
+        Reviewed by Darin Adler and Simon Fraser.
+
+        Only call FrameView::viewportContentsChanged() after a style recalc if
+        composited layers have been updated (and there is no pending layout).
+
+        We already viewportContentsChanged() after layout so we only need to
+        call viewportContentsChanged() after a style recalc if it did not cause
+        a layout but may have caused an element to become visible. In
+        particular, this can happen in the case of composited animations (e.g.
+        using -webkit-transform to move an element inside the viewport).
+        Therefore, we now only call viewportContentsChanged() after a style
+        recalc if it caused composited layers to be updated. This avoids a lot
+        of unnecessary calls to viewportContentsChanged(), which is expensive.
+
+        No new tests, already covered by:
+        fast/images/animated-gif-webkit-transform.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::recalcStyle):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::updateCompositingLayersAfterStyleChange):
+        * page/FrameView.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::didRecalcStyleWithNoPendingLayout):
+        (WebCore::RenderLayerCompositor::updateCompositingLayers):
+        * rendering/RenderLayerCompositor.h:
+
 2015-06-02  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         Remove use of CTFontSetRenderingParameters()

Modified: trunk/Source/WebCore/dom/Document.cpp (185117 => 185118)


--- trunk/Source/WebCore/dom/Document.cpp	2015-06-02 20:19:08 UTC (rev 185117)
+++ trunk/Source/WebCore/dom/Document.cpp	2015-06-02 20:20:41 UTC (rev 185118)
@@ -1767,6 +1767,7 @@
     // i.e. updating the flag here would be too late.
 
     m_inStyleRecalc = true;
+    bool updatedCompositingLayers = false;
     {
         Style::PostResolutionCallbackDisabler disabler(*this);
         WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
@@ -1781,7 +1782,7 @@
 
         Style::resolveTree(*this, change);
 
-        frameView.updateCompositingLayersAfterStyleChange();
+        updatedCompositingLayers = frameView.updateCompositingLayersAfterStyleChange();
 
         clearNeedsStyleRecalc();
         clearChildNeedsStyleRecalc();
@@ -1807,7 +1808,7 @@
     // Some animated images may now be inside the viewport due to style recalc,
     // resume them if necessary if there is no layout pending. Otherwise, we'll
     // check if they need to be resumed after layout.
-    if (!frameView.needsLayout())
+    if (updatedCompositingLayers && !frameView.needsLayout())
         frameView.viewportContentsChanged();
 
     // As a result of the style recalculation, the currently hovered element might have been

Modified: trunk/Source/WebCore/page/FrameView.cpp (185117 => 185118)


--- trunk/Source/WebCore/page/FrameView.cpp	2015-06-02 20:19:08 UTC (rev 185117)
+++ trunk/Source/WebCore/page/FrameView.cpp	2015-06-02 20:20:41 UTC (rev 185118)
@@ -758,17 +758,17 @@
     renderView->compositor().willRecalcStyle();
 }
 
-void FrameView::updateCompositingLayersAfterStyleChange()
+bool FrameView::updateCompositingLayersAfterStyleChange()
 {
     RenderView* renderView = this->renderView();
     if (!renderView)
-        return;
+        return false;
 
     // If we expect to update compositing after an incipient layout, don't do so here.
     if (inPreLayoutStyleUpdate() || layoutPending() || renderView->needsLayout())
-        return;
+        return false;
 
-    renderView->compositor().didRecalcStyleWithNoPendingLayout();
+    return renderView->compositor().didRecalcStyleWithNoPendingLayout();
 }
 
 void FrameView::updateCompositingLayersAfterLayout()

Modified: trunk/Source/WebCore/page/FrameView.h (185117 => 185118)


--- trunk/Source/WebCore/page/FrameView.h	2015-06-02 20:19:08 UTC (rev 185117)
+++ trunk/Source/WebCore/page/FrameView.h	2015-06-02 20:20:41 UTC (rev 185118)
@@ -149,7 +149,7 @@
 #endif
 
     void willRecalcStyle();
-    void updateCompositingLayersAfterStyleChange();
+    bool updateCompositingLayersAfterStyleChange();
     void updateCompositingLayersAfterLayout();
     bool flushCompositingStateForThisFrame(Frame* rootFrameForFlush);
 

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (185117 => 185118)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2015-06-02 20:19:08 UTC (rev 185117)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2015-06-02 20:20:41 UTC (rev 185118)
@@ -384,13 +384,13 @@
     m_layerNeedsCompositingUpdate = false;
 }
 
-void RenderLayerCompositor::didRecalcStyleWithNoPendingLayout()
+bool RenderLayerCompositor::didRecalcStyleWithNoPendingLayout()
 {
     if (!m_layerNeedsCompositingUpdate)
-        return;
+        return false;
     
     cacheAcceleratedCompositingFlags();
-    updateCompositingLayers(CompositingUpdateAfterStyleChange);
+    return updateCompositingLayers(CompositingUpdateAfterStyleChange);
 }
 
 void RenderLayerCompositor::customPositionForVisibleRectComputation(const GraphicsLayer* graphicsLayer, FloatPoint& position) const
@@ -663,7 +663,7 @@
     m_updateCompositingLayersTimer.stop();
 }
 
-void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType updateType, RenderLayer* updateRoot)
+bool RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType updateType, RenderLayer* updateRoot)
 {
     LOG(Compositing, "RenderLayerCompositor %p updateCompositingLayers %d %p", this, updateType, updateRoot);
 
@@ -673,17 +673,17 @@
     
     // Compositing layers will be updated in Document::setVisualUpdatesAllowed(bool) if suppressed here.
     if (!m_renderView.document().visualUpdatesAllowed())
-        return;
+        return false;
 
     // Avoid updating the layers with old values. Compositing layers will be updated after the layout is finished.
     if (m_renderView.needsLayout())
-        return;
+        return false;
 
     if ((m_forceCompositingMode || m_renderView.frame().mainFrame().pageOverlayController().overlayCount()) && !m_compositing)
         enableCompositingMode(true);
 
     if (!m_reevaluateCompositingAfterLayout && !m_compositing)
-        return;
+        return false;
 
     ++m_compositingUpdateCount;
 
@@ -711,7 +711,7 @@
     }
 
     if (!checkForHierarchyUpdate && !needGeometryUpdate)
-        return;
+        return false;
 
     bool needHierarchyUpdate = m_compositingLayersNeedRebuild;
     bool isFullUpdate = !updateRoot;
@@ -800,6 +800,8 @@
 
     // Inform the inspector that the layer tree has changed.
     InspectorInstrumentation::layerTreeDidChange(page());
+
+    return true;
 }
 
 void RenderLayerCompositor::appendDocumentOverlayLayers(Vector<GraphicsLayer*>& childList)

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.h (185117 => 185118)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2015-06-02 20:19:08 UTC (rev 185117)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2015-06-02 20:20:41 UTC (rev 185118)
@@ -121,8 +121,10 @@
     bool compositingLayersNeedRebuild() const { return m_compositingLayersNeedRebuild; }
     
     void willRecalcStyle();
-    void didRecalcStyleWithNoPendingLayout();
 
+    // Returns true if the composited layers were actually updated.
+    bool didRecalcStyleWithNoPendingLayout();
+
     // GraphicsLayers buffer state, which gets pushed to the underlying platform layers
     // at specific times.
     void scheduleLayerFlush(bool canThrottle);
@@ -139,7 +141,7 @@
     void didChangeVisibleRect();
     
     // Rebuild the tree of compositing layers
-    void updateCompositingLayers(CompositingUpdateType, RenderLayer* updateRoot = nullptr);
+    bool updateCompositingLayers(CompositingUpdateType, RenderLayer* updateRoot = nullptr);
     // This is only used when state changes and we do not exepect a style update or layout to happen soon (e.g. when
     // we discover that an iframe is overlapped during painting).
     void scheduleCompositingLayerUpdate();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to