Title: [289062] trunk/Source
Revision
289062
Author
commit-qu...@webkit.org
Date
2022-02-03 11:18:09 -0800 (Thu, 03 Feb 2022)

Log Message

Crash at com.apple.WebKit: WebKit::RemoteLayerBackingStore::display
https://bugs.webkit.org/show_bug.cgi?id=236003
Source/WebCore:

Patch by Kimmo Kinnunen <kkinnu...@apple.com> on 2022-02-03
Reviewed by Simon Fraser.

* platform/graphics/GraphicsLayerContentsDisplayDelegate.h:
Add a comment not to call PlatformCALayer::setBackingStoreAttached()

* platform/graphics/cocoa/WebProcessGraphicsContextGLCocoa.mm:
Clear the PlatformCALayer contents in more explicit way.

Source/WebKit:

<rdar://87617695>

Patch by Kimmo Kinnunen <kkinnu...@apple.com> on 2022-02-03
Reviewed by Simon Fraser.

Using UI-side compositing, following chain would be the cause a crash:
PlatformCALayerRemote::recursiveBuildTransaction
RemoteLayerBackingStore::display
PlatformCALayer::platformCALayerLayerDisplay
WebProcessGraphicsContextGLCocoa.mm DisplayBufferDisplayDelegate::display
PlatformCALayer::setContents(nullptr)

The nullptr contents happens when the WebGL context does not have
display buffer yet or when the display buffer creation would fail.

Setting empty layer contents would destroy the RemoteLayerBackingStore,
even though the callstack is in process of displaying the layer to the
backing store.

When setting PlatformCALayer contents "empty", clear the backing store instead
of removing the backing store.

Tested by LayoutTests/webgl (under ASAN, ios simulator)

* Shared/RemoteLayerTree/RemoteLayerBackingStore.h:
* WebProcess/GPU/graphics/cocoa/RemoteGraphicsContextGLProxyCocoa.mm:
* WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp:
(WebKit::PlatformCALayerRemote::setContents):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (289061 => 289062)


--- trunk/Source/WebCore/ChangeLog	2022-02-03 19:01:24 UTC (rev 289061)
+++ trunk/Source/WebCore/ChangeLog	2022-02-03 19:18:09 UTC (rev 289062)
@@ -1,3 +1,16 @@
+2022-02-03  Kimmo Kinnunen  <kkinnu...@apple.com>
+
+        Crash at com.apple.WebKit: WebKit::RemoteLayerBackingStore::display
+        https://bugs.webkit.org/show_bug.cgi?id=236003
+
+        Reviewed by Simon Fraser.
+
+        * platform/graphics/GraphicsLayerContentsDisplayDelegate.h:
+        Add a comment not to call PlatformCALayer::setBackingStoreAttached()
+
+        * platform/graphics/cocoa/WebProcessGraphicsContextGLCocoa.mm:
+        Clear the PlatformCALayer contents in more explicit way.
+
 2022-02-03  Chris Dumez  <cdu...@apple.com>
 
         Move and rename ServiceWorkerThreadProxy::setupPageForServiceWorker()

Modified: trunk/Source/WebCore/platform/graphics/GraphicsLayerContentsDisplayDelegate.h (289061 => 289062)


--- trunk/Source/WebCore/platform/graphics/GraphicsLayerContentsDisplayDelegate.h	2022-02-03 19:01:24 UTC (rev 289061)
+++ trunk/Source/WebCore/platform/graphics/GraphicsLayerContentsDisplayDelegate.h	2022-02-03 19:18:09 UTC (rev 289062)
@@ -45,6 +45,7 @@
 
 #if USE(CA)
     virtual void prepareToDelegateDisplay(PlatformCALayer&);
+    // Must not detach the platform layer backing store.
     virtual void display(PlatformCALayer&) = 0;
     virtual GraphicsLayer::CompositingCoordinatesOrientation orientation() const;
 #else

Modified: trunk/Source/WebCore/platform/graphics/cocoa/WebProcessGraphicsContextGLCocoa.mm (289061 => 289062)


--- trunk/Source/WebCore/platform/graphics/cocoa/WebProcessGraphicsContextGLCocoa.mm	2022-02-03 19:01:24 UTC (rev 289061)
+++ trunk/Source/WebCore/platform/graphics/cocoa/WebProcessGraphicsContextGLCocoa.mm	2022-02-03 19:18:09 UTC (rev 289062)
@@ -58,7 +58,7 @@
         if (m_displayBuffer)
             layer.setContents(*m_displayBuffer);
         else
-            layer.setContents(nullptr);
+            layer.clearContents();
     }
 
     GraphicsLayer::CompositingCoordinatesOrientation orientation() const final

Modified: trunk/Source/WebKit/ChangeLog (289061 => 289062)


--- trunk/Source/WebKit/ChangeLog	2022-02-03 19:01:24 UTC (rev 289061)
+++ trunk/Source/WebKit/ChangeLog	2022-02-03 19:18:09 UTC (rev 289062)
@@ -1,3 +1,35 @@
+2022-02-03  Kimmo Kinnunen  <kkinnu...@apple.com>
+
+        Crash at com.apple.WebKit: WebKit::RemoteLayerBackingStore::display
+        https://bugs.webkit.org/show_bug.cgi?id=236003
+        <rdar://87617695>
+
+        Reviewed by Simon Fraser.
+
+        Using UI-side compositing, following chain would be the cause a crash:
+        PlatformCALayerRemote::recursiveBuildTransaction
+        RemoteLayerBackingStore::display
+        PlatformCALayer::platformCALayerLayerDisplay
+        WebProcessGraphicsContextGLCocoa.mm DisplayBufferDisplayDelegate::display
+        PlatformCALayer::setContents(nullptr)
+
+        The nullptr contents happens when the WebGL context does not have
+        display buffer yet or when the display buffer creation would fail.
+
+        Setting empty layer contents would destroy the RemoteLayerBackingStore,
+        even though the callstack is in process of displaying the layer to the
+        backing store.
+
+        When setting PlatformCALayer contents "empty", clear the backing store instead
+        of removing the backing store.
+
+        Tested by LayoutTests/webgl (under ASAN, ios simulator)
+
+        * Shared/RemoteLayerTree/RemoteLayerBackingStore.h:
+        * WebProcess/GPU/graphics/cocoa/RemoteGraphicsContextGLProxyCocoa.mm:
+        * WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp:
+        (WebKit::PlatformCALayerRemote::setContents):
+
 2022-02-03  Chris Dumez  <cdu...@apple.com>
 
         Move and rename ServiceWorkerThreadProxy::setupPageForServiceWorker()

Modified: trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.h (289061 => 289062)


--- trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.h	2022-02-03 19:01:24 UTC (rev 289061)
+++ trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.h	2022-02-03 19:18:09 UTC (rev 289062)
@@ -100,9 +100,10 @@
 
     MonotonicTime lastDisplayTime() const { return m_lastDisplayTime; }
 
+    void clearBackingStore();
+
 private:
     void drawInContext(WebCore::GraphicsContext&);
-    void clearBackingStore();
     void swapToValidFrontBuffer();
 
     bool supportsPartialRepaint();

Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/RemoteGraphicsContextGLProxyCocoa.mm (289061 => 289062)


--- trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/RemoteGraphicsContextGLProxyCocoa.mm	2022-02-03 19:01:24 UTC (rev 289061)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/RemoteGraphicsContextGLProxyCocoa.mm	2022-02-03 19:18:09 UTC (rev 289062)
@@ -60,7 +60,7 @@
         if (m_displayBuffer)
             layer.setContents(m_displayBuffer);
         else
-            layer.setContents(nullptr);
+            layer.clearContents();
     }
 
     WebCore::GraphicsLayer::CompositingCoordinatesOrientation orientation() const final

Modified: trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp (289061 => 289062)


--- trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp	2022-02-03 19:01:24 UTC (rev 289061)
+++ trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp	2022-02-03 19:18:09 UTC (rev 289062)
@@ -683,8 +683,10 @@
 
 void PlatformCALayerRemote::setContents(CFTypeRef value)
 {
+    if (!m_properties.backingStore)
+        return;
     if (!value)
-        m_properties.backingStore = nullptr;
+        m_properties.backingStore->clearBackingStore();
 }
 
 #if HAVE(IOSURFACE)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to