Title: [280183] trunk/Source/WebKit
Revision
280183
Author
timothy_hor...@apple.com
Date
2021-07-22 11:04:04 -0700 (Thu, 22 Jul 2021)

Log Message

REGRESSION (r279992): Crashes under RemoteLayerBackingStore::applyBackingStoreToLayer() in macCatalyst
https://bugs.webkit.org/show_bug.cgi?id=228181
rdar://80923581

Reviewed by Dan Bates.

* Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:
(WebKit::RemoteLayerBackingStore::applyBackingStoreToLayer):
r279992 reorganized this code to determine the contents object and then
set it on the layer, instead of setting it directly; this means that the
lifetime of the contents object must be extended.

Interestingly, the common case (the CAMachPort case), as well as the
case I was actually adding in r279992 both were safe, because of the use
of autorelease. (macCatalyst uses IOSurface as layer contents directly,
without CAMachPort, so uses the one path that r279992 broke).

It is unnecessary to use autorelease; instead just store the contents
object in a RetainPtr until it is set.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (280182 => 280183)


--- trunk/Source/WebKit/ChangeLog	2021-07-22 17:53:43 UTC (rev 280182)
+++ trunk/Source/WebKit/ChangeLog	2021-07-22 18:04:04 UTC (rev 280183)
@@ -1,3 +1,25 @@
+2021-07-22  Tim Horton  <timothy_hor...@apple.com>
+
+        REGRESSION (r279992): Crashes under RemoteLayerBackingStore::applyBackingStoreToLayer() in macCatalyst
+        https://bugs.webkit.org/show_bug.cgi?id=228181
+        rdar://80923581
+
+        Reviewed by Dan Bates.
+
+        * Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:
+        (WebKit::RemoteLayerBackingStore::applyBackingStoreToLayer):
+        r279992 reorganized this code to determine the contents object and then
+        set it on the layer, instead of setting it directly; this means that the
+        lifetime of the contents object must be extended.
+
+        Interestingly, the common case (the CAMachPort case), as well as the
+        case I was actually adding in r279992 both were safe, because of the use
+        of autorelease. (macCatalyst uses IOSurface as layer contents directly,
+        without CAMachPort, so uses the one path that r279992 broke).
+
+        It is unnecessary to use autorelease; instead just store the contents
+        object in a RetainPtr until it is set.
+
 2021-07-22  Megan Gardner  <megan_gard...@apple.com>
 
         Avoid Quick Note overlay when scrolling to show a highlight

Modified: trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm (280182 => 280183)


--- trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm	2021-07-22 17:53:43 UTC (rev 280182)
+++ trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm	2021-07-22 18:04:04 UTC (rev 280183)
@@ -426,12 +426,12 @@
     ASSERT(m_bufferHandle);
     layer.contentsOpaque = m_isOpaque;
 
-    id contents = nil;
+    RetainPtr<id> contents;
     WTF::switchOn(*m_bufferHandle,
         [&] (ShareableBitmap::Handle& handle) {
             ASSERT(m_type == Type::Bitmap);
             auto bitmap = ShareableBitmap::create(handle);
-            contents = bitmap->makeCGImageCopy().bridgingAutorelease();
+            contents = bitmap->makeCGImageCopy();
         },
         [&] (MachSendRight& machSendRight) {
             ASSERT(m_type == Type::IOSurface);
@@ -442,7 +442,7 @@
                 break;
             }
             case RemoteLayerBackingStore::LayerContentsType::CAMachPort:
-                contents = adoptCF(CAMachPortCreate(machSendRight.leakSendRight())).bridgingAutorelease();
+                contents = adoptCF(CAMachPortCreate(machSendRight.leakSendRight()));
                 break;
             }
         }
@@ -460,12 +460,12 @@
             return;
         [layer setValue:@1 forKeyPath:WKCGDisplayListEnabledKey];
         auto data = ""
-        [(WKCompositingLayer *)layer _setWKContents:contents withDisplayList:data.get()];
+        [(WKCompositingLayer *)layer _setWKContents:contents.get() withDisplayList:data.get()];
         return;
     }
 #endif
 
-    layer.contents = contents;
+    layer.contents = contents.get();
 }
 
 Vector<std::unique_ptr<WebCore::ThreadSafeImageBufferFlusher>> RemoteLayerBackingStore::takePendingFlushers()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to