Changeset 224796 in webkit

Timestamp:

Nov 13, 2017 6:01:40 PM (6 years ago)

Author:

Simon Fraser

Message:

When navigating back to a page, compositing layers may not use accelerated drawing
​https://bugs.webkit.org/show_bug.cgi?id=178749
rdar://problem/35158946

Reviewed by Dean Jackson.
Source/WebCore:

There were two issues with setting GraphicsLayerCA's "acceleratesDrawing" state which
occurred on back navigation, related to the ordering of style recalcs and layout.

First, at style recalc time, we created a RenderLayerCompositor but hadn't yet called
its cacheAcceleratedCompositingFlags(), so any layers created during style update
didn't get accelerated drawing. Fix by making cacheAcceleratedCompositingFlags() internal
to RenderLayerCompositor and calling it from willRecalcStyle() and updateCompositingLayers().

We have to be careful to avoid m_forceCompositingMode flip-flopping between cacheAcceleratedCompositingFlags()
and cacheAcceleratedCompositingFlagsAfterLayout(), since, for subframes, it depends on layout.

Secondly, GraphicsLayerCA::commitLayerChangesBeforeSublayers() needs to updateAcceleratesDrawing()
before updating tiles, so that new tiles fetch the right acceleratesDrawing state from
the TileController.

Test: compositing/accelerated-layers-after-back.html

• page/FrameView.cpp:

(WebCore::FrameView::updateCompositingLayersAfterLayout):

• platform/graphics/ca/GraphicsLayerCA.cpp:

(WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers):

• rendering/RenderLayerCompositor.cpp:

(WebCore::RenderLayerCompositor::willRecalcStyle):
(WebCore::RenderLayerCompositor::didRecalcStyleWithNoPendingLayout):
(WebCore::RenderLayerCompositor::updateCompositingLayers):

• rendering/RenderLayerCompositor.h:

Source/WebKit:

Avoid assertions when a test enables accelerated drawing (which we can't support
in the iOS simulator).

• Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:

(WebKit::RemoteLayerBackingStore::ensureBackingStore):

Tools:

Fix WTR and DRT to parse "useAcceleratedDrawing" out of "webkit-test-runner" options
and use it to set the state of the web view.

• DumpRenderTree/TestOptions.h:
• DumpRenderTree/TestOptions.mm:

(TestOptions::TestOptions):

• DumpRenderTree/mac/DumpRenderTree.mm:

(setWebPreferencesForTestOptions):

• WebKitTestRunner/TestController.cpp:

(WTR::TestController::resetPreferencesToConsistentValues):
(WTR::updateTestOptionsFromTestHeader):

• WebKitTestRunner/TestOptions.h:

(WTR::TestOptions::hasSameInitializationOptions const):

LayoutTests:

Moved iframes/resources/page-cache-helper.html up to compositing/resources/go-back.html.

• compositing/accelerated-layers-after-back-expected.txt: Added.
• compositing/accelerated-layers-after-back.html: Added.
• compositing/iframes/page-cache-layer-tree.html:
• compositing/page-cache-back-crash.html:
• compositing/resources/go-back.html: Renamed from LayoutTests/compositing/iframes/resources/page-cache-helper.html.
• platform/ios/compositing/accelerated-layers-after-back-expected.txt: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/compositing/accelerated-layers-after-back-expected.txt (added)
• LayoutTests/compositing/accelerated-layers-after-back.html (added)
• LayoutTests/compositing/iframes/page-cache-layer-tree.html (modified) (1 diff)
• LayoutTests/compositing/page-cache-back-crash.html (modified) (1 diff)
• LayoutTests/compositing/resources/go-back.html (moved) (moved from trunk/LayoutTests/compositing/iframes/resources/page-cache-helper.html)
• LayoutTests/compositing/tiling/tiled-drawing-async-frame-scrolling.html (modified) (1 diff)
• LayoutTests/platform/ios/compositing/accelerated-layers-after-back-expected.txt (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/FrameView.cpp (modified) (1 diff)
• Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderLayerCompositor.cpp (modified) (7 diffs)
• Source/WebCore/rendering/RenderLayerCompositor.h (modified) (2 diffs)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/DumpRenderTree/TestOptions.h (modified) (1 diff)
• Tools/DumpRenderTree/TestOptions.mm (modified) (1 diff)
• Tools/DumpRenderTree/mac/DumpRenderTree.mm (modified) (1 diff)
• Tools/WebKitTestRunner/TestController.cpp (modified) (2 diffs)
• Tools/WebKitTestRunner/TestOptions.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-11-13  Simon Fraser  <simon.fraser@apple.com>
+
+        When navigating back to a page, compositing layers may not use accelerated drawing
+        https://bugs.webkit.org/show_bug.cgi?id=178749
+        rdar://problem/35158946
+
+        Reviewed by Dean Jackson.
+        
+        Moved iframes/resources/page-cache-helper.html up to compositing/resources/go-back.html.
+
+        * compositing/accelerated-layers-after-back-expected.txt: Added.
+        * compositing/accelerated-layers-after-back.html: Added.
+        * compositing/iframes/page-cache-layer-tree.html:
+        * compositing/page-cache-back-crash.html:
+        * compositing/resources/go-back.html: Renamed from LayoutTests/compositing/iframes/resources/page-cache-helper.html.
+        * platform/ios/compositing/accelerated-layers-after-back-expected.txt: Added.
+
 2017-11-13  Ryan Haddad  <ryanhaddad@apple.com>
 

trunk/LayoutTests/compositing/iframes/page-cache-layer-tree.html

@@ -74 +74 @@
     window.addEventListener("load", function(event) {
         setTimeout(function() {
-            window.location.href = "resources/page-cache-helper.html";
+            window.location.href = "../resources/go-back.html";
         }, 0);
     }, false);

trunk/LayoutTests/compositing/page-cache-back-crash.html

@@ -20 +20 @@
     setTimeout(function() {
         // Navigate to a helper page that will immediately navigate back here after loading.
-        window.location.href = "iframes/resources/page-cache-helper.html";
+        window.location.href = "resources/go-back.html";
     }, 0);
 });

trunk/LayoutTests/compositing/tiling/tiled-drawing-async-frame-scrolling.html

@@ -10 +10 @@
             testRunner.dumpAsText();
             testRunner.waitUntilDone();
-        }
+        }
 
         function doTest()

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-11-13  Simon Fraser  <simon.fraser@apple.com>
+
+        When navigating back to a page, compositing layers may not use accelerated drawing
+        https://bugs.webkit.org/show_bug.cgi?id=178749
+        rdar://problem/35158946
+
+        Reviewed by Dean Jackson.
+
+        There were two issues with setting GraphicsLayerCA's "acceleratesDrawing" state which
+        occurred on back navigation, related to the ordering of style recalcs and layout.
+
+        First, at style recalc time, we created a RenderLayerCompositor but hadn't yet called
+        its cacheAcceleratedCompositingFlags(), so any layers created during style update
+        didn't get accelerated drawing. Fix by making cacheAcceleratedCompositingFlags() internal
+        to RenderLayerCompositor and calling it from willRecalcStyle() and updateCompositingLayers().
+
+        We have to be careful to avoid m_forceCompositingMode flip-flopping between cacheAcceleratedCompositingFlags()
+        and cacheAcceleratedCompositingFlagsAfterLayout(), since, for subframes, it depends on layout.
+
+        Secondly, GraphicsLayerCA::commitLayerChangesBeforeSublayers() needs to updateAcceleratesDrawing()
+        before updating tiles, so that new tiles fetch the right acceleratesDrawing state from
+        the TileController.
+
+        Test: compositing/accelerated-layers-after-back.html
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::updateCompositingLayersAfterLayout):
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::willRecalcStyle):
+        (WebCore::RenderLayerCompositor::didRecalcStyleWithNoPendingLayout):
+        (WebCore::RenderLayerCompositor::updateCompositingLayers):
+        * rendering/RenderLayerCompositor.h:
+
 2017-11-13  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -800 +800 @@
         return;
 
-    // This call will make sure the cached hasAcceleratedCompositing is updated from the pref
-    renderView->compositor().cacheAcceleratedCompositingFlags();
     renderView->compositor().updateCompositingLayers(CompositingUpdateType::AfterLayout);
 }

trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

@@ -1759 +1759 @@
         updateCoverage(commitState);
 
+    if (m_uncommittedChanges & AcceleratesDrawingChanged) // Needs to happen before TilingAreaChanged.
+        updateAcceleratesDrawing();
+
     if (m_uncommittedChanges & TilingAreaChanged) // Needs to happen after CoverageRectChanged, ContentsScaleChanged
         updateTiles();
@@ -1781 +1784 @@
         updateContentsNeedsDisplay();
     
-    if (m_uncommittedChanges & AcceleratesDrawingChanged)
-        updateAcceleratesDrawing();
-
     if (m_uncommittedChanges & SupportsSubpixelAntialiasedTextChanged)
         updateSupportsSubpixelAntialiasedText();

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -300 +300 @@
 void RenderLayerCompositor::cacheAcceleratedCompositingFlags()
 {
-    bool hasAcceleratedCompositing = false;
-    bool showDebugBorders = false;
-    bool showRepaintCounter = false;
-    bool forceCompositingMode = false;
-    bool acceleratedDrawingEnabled = false;
-    bool displayListDrawingEnabled = false;
-
     auto& settings = m_renderView.settings();
-    hasAcceleratedCompositing = settings.acceleratedCompositingEnabled();
+    bool hasAcceleratedCompositing = settings.acceleratedCompositingEnabled();
 
     // We allow the chrome to override the settings, in case the page is rendered
@@ -317 +310 @@
     }
 
-    showDebugBorders = settings.showDebugBorders();
-    showRepaintCounter = settings.showRepaintCounter();
-    forceCompositingMode = settings.forceCompositingMode() && hasAcceleratedCompositing;
-
-    if (forceCompositingMode && !isMainFrameCompositor())
-        forceCompositingMode = requiresCompositingForScrollableFrame();
-
-    acceleratedDrawingEnabled = settings.acceleratedDrawingEnabled();
-    displayListDrawingEnabled = settings.displayListDrawingEnabled();
+    bool showDebugBorders = settings.showDebugBorders();
+    bool showRepaintCounter = settings.showRepaintCounter();
+    bool acceleratedDrawingEnabled = settings.acceleratedDrawingEnabled();
+    bool displayListDrawingEnabled = settings.displayListDrawingEnabled();
+
+    // forceCompositingMode for subframes can only be computed after layout.
+    bool forceCompositingMode = m_forceCompositingMode;
+    if (isMainFrameCompositor())
+        forceCompositingMode = m_renderView.settings().forceCompositingMode() && hasAcceleratedCompositing; 
     
     if (hasAcceleratedCompositing != m_hasAcceleratedCompositing || showDebugBorders != m_showDebugBorders || showRepaintCounter != m_showRepaintCounter || forceCompositingMode != m_forceCompositingMode)
@@ -332 +325 @@
     bool debugBordersChanged = m_showDebugBorders != showDebugBorders;
     m_hasAcceleratedCompositing = hasAcceleratedCompositing;
+    m_forceCompositingMode = forceCompositingMode;
     m_showDebugBorders = showDebugBorders;
     m_showRepaintCounter = showRepaintCounter;
-    m_forceCompositingMode = forceCompositingMode;
     m_acceleratedDrawingEnabled = acceleratedDrawingEnabled;
     m_displayListDrawingEnabled = displayListDrawingEnabled;
@@ -350 +343 @@
 }
 
+void RenderLayerCompositor::cacheAcceleratedCompositingFlagsAfterLayout()
+{
+    cacheAcceleratedCompositingFlags();
+
+    if (isMainFrameCompositor())
+        return;
+
+    bool forceCompositingMode = m_hasAcceleratedCompositing && m_renderView.settings().forceCompositingMode() && requiresCompositingForScrollableFrame();
+    if (forceCompositingMode != m_forceCompositingMode) {
+        m_forceCompositingMode = forceCompositingMode;
+        setCompositingLayersNeedRebuild();
+    }
+}
+
 bool RenderLayerCompositor::canRender3DTransforms() const
 {
@@ -364 +371 @@
 {
     m_layerNeedsCompositingUpdate = false;
+    cacheAcceleratedCompositingFlags();
 }
 
@@ -371 +379 @@
         return false;
     
-    cacheAcceleratedCompositingFlags();
     return updateCompositingLayers(CompositingUpdateType::AfterStyleChange);
 }
@@ -631 +638 @@
 {
     LOG_WITH_STREAM(Compositing, stream << "RenderLayerCompositor " << this << " updateCompositingLayers " << updateType << " root " << updateRoot);
+
+    if (updateType == CompositingUpdateType::AfterStyleChange || updateType == CompositingUpdateType::AfterLayout)
+        cacheAcceleratedCompositingFlagsAfterLayout(); // Some flags (e.g. forceCompositingMode) depend on layout.
 
     m_updateCompositingLayersTimer.stop();

trunk/Source/WebCore/rendering/RenderLayerCompositor.h

@@ -111 +111 @@
     bool canRender3DTransforms() const;
 
-    // Copy the accelerated compositing related flags from Settings
-    void cacheAcceleratedCompositingFlags();
-
     // Called when the layer hierarchy needs to be updated (compositing layers have been
     // created, destroyed or re-parented).
@@ -346 +343 @@
     // GraphicsLayerUpdaterClient implementation
     void flushLayersSoon(GraphicsLayerUpdater&) override;
+
+    // Copy the accelerated compositing related flags from Settings
+    void cacheAcceleratedCompositingFlags();
+    void cacheAcceleratedCompositingFlagsAfterLayout();
 
     // Whether the given RL needs a compositing layer.

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2017-11-13  Simon Fraser  <simon.fraser@apple.com>
+
+        When navigating back to a page, compositing layers may not use accelerated drawing
+        https://bugs.webkit.org/show_bug.cgi?id=178749
+        rdar://problem/35158946
+
+        Reviewed by Dean Jackson.
+
+        Avoid assertions when a test enables accelerated drawing (which we can't support
+        in the iOS simulator).
+
+        * Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:
+        (WebKit::RemoteLayerBackingStore::ensureBackingStore):
+
 2017-11-13  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm

@@ -73 +73 @@
 void RemoteLayerBackingStore::ensureBackingStore(FloatSize size, float scale, bool acceleratesDrawing, bool deepColor, bool isOpaque)
 {
+#if !USE(IOSURFACE)
+    acceleratesDrawing = false;
+#endif
     if (m_size == size && m_scale == scale && m_deepColor == deepColor && m_acceleratesDrawing == acceleratesDrawing && m_isOpaque == isOpaque)
         return;

trunk/Tools/ChangeLog

@@ +1 @@
+2017-11-13  Simon Fraser  <simon.fraser@apple.com>
+
+        When navigating back to a page, compositing layers may not use accelerated drawing
+        https://bugs.webkit.org/show_bug.cgi?id=178749
+        rdar://problem/35158946
+
+        Reviewed by Dean Jackson.
+
+        Fix WTR and DRT to parse "useAcceleratedDrawing" out of "webkit-test-runner" options
+        and use it to set the state of the web view.
+
+        * DumpRenderTree/TestOptions.h:
+        * DumpRenderTree/TestOptions.mm:
+        (TestOptions::TestOptions):
+        * DumpRenderTree/mac/DumpRenderTree.mm:
+        (setWebPreferencesForTestOptions):
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::resetPreferencesToConsistentValues):
+        (WTR::updateTestOptionsFromTestHeader):
+        * WebKitTestRunner/TestOptions.h:
+        (WTR::TestOptions::hasSameInitializationOptions const):
+
 2017-11-13  Basuke Suzuki  <Basuke.Suzuki@sony.com>
 

trunk/Tools/DumpRenderTree/TestOptions.h

@@ -31 +31 @@
 struct TestOptions {
     bool enableAttachmentElement { false };
+    bool useAcceleratedDrawing { false };
     bool enableIntersectionObserver { false };
     bool enableMenuItemElement { false };

trunk/Tools/DumpRenderTree/TestOptions.mm

@@ -81 +81 @@
         if (key == "enableAttachmentElement")
             this->enableAttachmentElement = parseBooleanTestHeaderValue(value);
+        if (key == "useAcceleratedDrawing")
+            this->useAcceleratedDrawing = parseBooleanTestHeaderValue(value);
         else if (key == "enableIntersectionObserver")
             this->enableIntersectionObserver = parseBooleanTestHeaderValue(value);

trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm

@@ -983 +983 @@
 
     preferences.attachmentElementEnabled = options.enableAttachmentElement;
+    preferences.acceleratedDrawingEnabled = options.useAcceleratedDrawing;
     preferences.intersectionObserverEnabled = options.enableIntersectionObserver;
     preferences.menuItemElementEnabled = options.enableMenuItemElement;

trunk/Tools/WebKitTestRunner/TestController.cpp

@@ -722 +722 @@
     WKPreferencesSetHiddenPageCSSAnimationSuspensionEnabled(preferences, false);
 
-    WKPreferencesSetAcceleratedDrawingEnabled(preferences, m_shouldUseAcceleratedDrawing);
+    WKPreferencesSetAcceleratedDrawingEnabled(preferences, m_shouldUseAcceleratedDrawing || options.useAcceleratedDrawing);
     // FIXME: We should be testing the default.
     WKPreferencesSetStorageBlockingPolicy(preferences, kWKAllowAllStorage);
@@ -1026 +1026 @@
         if (key == "useThreadedScrolling")
             testOptions.useThreadedScrolling = parseBooleanTestHeaderValue(value);
+        if (key == "useAcceleratedDrawing")
+            testOptions.useAcceleratedDrawing = parseBooleanTestHeaderValue(value);
         if (key == "useFlexibleViewport")
             testOptions.useFlexibleViewport = parseBooleanTestHeaderValue(value);

trunk/Tools/WebKitTestRunner/TestOptions.h

@@ -34 +34 @@
 struct TestOptions {
     bool useThreadedScrolling { false };
+    bool useAcceleratedDrawing { false };
     bool useRemoteLayerTree { false };
     bool shouldShowWebView { false };
@@ -67 +68 @@
     {
         if (useThreadedScrolling != options.useThreadedScrolling
+            || useAcceleratedDrawing != options.useAcceleratedDrawing
             || overrideLanguages != options.overrideLanguages
             || useMockScrollbars != options.useMockScrollbars