Title: [286408] trunk/Source/WebKit
Revision
286408
Author
j...@apple.com
Date
2021-12-01 18:12:37 -0800 (Wed, 01 Dec 2021)

Log Message

Avoid allocating and copy memory from a SharedMemory into a SharedBuffer
https://bugs.webkit.org/show_bug.cgi?id=233401
rdar://85637721

Reviewed by Eric Carlson.

We can directly wrap a SharedMemory into a SharedBuffer using
SharedMemory::createSharedBuffer instead.
No change in observable behaviour other than total memory size reduction.

Fly-by fix: check for null when mapping a SharedMemory to an IPC handle
in a couple of spots.

* Shared/ShareableResource.h: Make ShareableResource use thread-safe refcount.
A ShareableResource once created is immutable and the SharedMemory object
itself has thread-safe refounting. Deleting the object outside the main thread
is safe.
Make members const.
* Shared/ShareableResource.cpp:
(WebKit::ShareableResource::wrapInSharedBuffer): Refactor method, using DataSegment::Provider
instead of platfom specific code.
* Shared/WebCoreArgumentCoders.cpp:
(IPC::decodeSharedBuffer):
* UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:
(WebKit::WebPasteboardProxy::setPasteboardBufferForType):
* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::saveImageToLibrary):
* UIProcess/mac/WebPageProxyMac.mm:
(WebKit::WebPageProxy::dataSelectionForPasteboard):
(WebKit::WebPageProxy::setPromisedDataForImage):
* WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:
(WebKit::WebPlatformStrategies::bufferForType):
Hardening code, checking for nullability.
(WebKit::WebPlatformStrategies::readBufferFromPasteboard):
Hardening code, checking for nullability.
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::restoreAppHighlightsAndScrollToIndex):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (286407 => 286408)


--- trunk/Source/WebKit/ChangeLog	2021-12-02 02:10:30 UTC (rev 286407)
+++ trunk/Source/WebKit/ChangeLog	2021-12-02 02:12:37 UTC (rev 286408)
@@ -1,3 +1,43 @@
+2021-12-01  Jean-Yves Avenard  <j...@apple.com>
+
+        Avoid allocating and copy memory from a SharedMemory into a SharedBuffer
+        https://bugs.webkit.org/show_bug.cgi?id=233401
+        rdar://85637721
+
+        Reviewed by Eric Carlson.
+
+        We can directly wrap a SharedMemory into a SharedBuffer using
+        SharedMemory::createSharedBuffer instead.
+        No change in observable behaviour other than total memory size reduction.
+
+        Fly-by fix: check for null when mapping a SharedMemory to an IPC handle
+        in a couple of spots.
+
+        * Shared/ShareableResource.h: Make ShareableResource use thread-safe refcount.
+        A ShareableResource once created is immutable and the SharedMemory object
+        itself has thread-safe refounting. Deleting the object outside the main thread
+        is safe.
+        Make members const.
+        * Shared/ShareableResource.cpp:
+        (WebKit::ShareableResource::wrapInSharedBuffer): Refactor method, using DataSegment::Provider
+        instead of platfom specific code.
+        * Shared/WebCoreArgumentCoders.cpp:
+        (IPC::decodeSharedBuffer):
+        * UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:
+        (WebKit::WebPasteboardProxy::setPasteboardBufferForType):
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::WebPageProxy::saveImageToLibrary):
+        * UIProcess/mac/WebPageProxyMac.mm:
+        (WebKit::WebPageProxy::dataSelectionForPasteboard):
+        (WebKit::WebPageProxy::setPromisedDataForImage):
+        * WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:
+        (WebKit::WebPlatformStrategies::bufferForType):
+        Hardening code, checking for nullability.
+        (WebKit::WebPlatformStrategies::readBufferFromPasteboard):
+        Hardening code, checking for nullability.
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::restoreAppHighlightsAndScrollToIndex):
+
 2021-12-01  Chris Fleizach  <cfleiz...@apple.com>
 
         AX: Support accessibility attributes for <model>

Modified: trunk/Source/WebKit/Shared/ShareableResource.cpp (286407 => 286408)


--- trunk/Source/WebKit/Shared/ShareableResource.cpp	2021-12-02 02:10:30 UTC (rev 286407)
+++ trunk/Source/WebKit/Shared/ShareableResource.cpp	2021-12-02 02:12:37 UTC (rev 286408)
@@ -55,46 +55,12 @@
     return true;
 }
 
-#if USE(CF)
-static void shareableResourceDeallocate(void *ptr, void *info)
-{
-    static_cast<ShareableResource*>(info)->deref(); // Balanced by ref() in createShareableResourceDeallocator()
-}
-    
-static RetainPtr<CFAllocatorRef> createShareableResourceDeallocator(ShareableResource* resource)
-{
-    CFAllocatorContext context = { 0,
-        resource,
-        NULL, // retain
-        NULL, // release
-        NULL, // copyDescription
-        NULL, // allocate
-        NULL, // reallocate
-        shareableResourceDeallocate,
-        NULL, // preferredSize
-    };
-
-    return adoptCF(CFAllocatorCreate(kCFAllocatorDefault, &context));
-}
-#endif
-
 RefPtr<SharedBuffer> ShareableResource::wrapInSharedBuffer()
 {
-    ref(); // Balanced by deref when SharedBuffer is deallocated.
-
-#if USE(CF)
-    auto deallocator = createShareableResourceDeallocator(this);
-    auto cfData = adoptCF(CFDataCreateWithBytesNoCopy(kCFAllocatorDefault, data(), static_cast<CFIndex>(size()), deallocator.get()));
-    return SharedBuffer::create(cfData.get());
-#elif USE(GLIB)
-    GRefPtr<GBytes> bytes = adoptGRef(g_bytes_new_with_free_func(data(), size(), [](void* data) {
-        static_cast<ShareableResource*>(data)->deref();
-    }, this));
-    return SharedBuffer::create(bytes.get());
-#else
-    ASSERT_NOT_REACHED();
-    return nullptr;
-#endif
+    return SharedBuffer::create(SharedBuffer::DataSegment::Provider {
+        [self = Ref { *this }]() { return self->data(); },
+        [self = Ref { *this }]() { return self->size(); }
+    });
 }
 
 RefPtr<SharedBuffer> ShareableResource::Handle::tryWrapInSharedBuffer() const
@@ -138,9 +104,7 @@
 {
 }
 
-ShareableResource::~ShareableResource()
-{
-}
+ShareableResource::~ShareableResource() = default;
 
 bool ShareableResource::createHandle(Handle& handle)
 {
@@ -162,7 +126,7 @@
 {
     return m_size;
 }
-    
+
 } // namespace WebKit
 
 #endif // ENABLE(SHAREABLE_RESOURCE)

Modified: trunk/Source/WebKit/Shared/ShareableResource.h (286407 => 286408)


--- trunk/Source/WebKit/Shared/ShareableResource.h	2021-12-02 02:10:30 UTC (rev 286407)
+++ trunk/Source/WebKit/Shared/ShareableResource.h	2021-12-02 02:12:37 UTC (rev 286408)
@@ -38,7 +38,7 @@
 
 namespace WebKit {
     
-class ShareableResource : public RefCounted<ShareableResource> {
+class ShareableResource : public ThreadSafeRefCounted<ShareableResource> {
 public:
 
     class Handle {
@@ -84,8 +84,8 @@
 
     Ref<SharedMemory> m_sharedMemory;
 
-    unsigned m_offset;
-    unsigned m_size;    
+    const unsigned m_offset;
+    const unsigned m_size;
 };
 
 } // namespace WebKit

Modified: trunk/Source/WebKit/Shared/WebCoreArgumentCoders.cpp (286407 => 286408)


--- trunk/Source/WebKit/Shared/WebCoreArgumentCoders.cpp	2021-12-02 02:10:30 UTC (rev 286407)
+++ trunk/Source/WebKit/Shared/WebCoreArgumentCoders.cpp	2021-12-02 02:12:37 UTC (rev 286408)
@@ -234,7 +234,7 @@
     if (sharedMemoryBuffer->size() < bufferSize)
         return false;
 
-    buffer = SharedBuffer::create(static_cast<unsigned char*>(sharedMemoryBuffer->data()), bufferSize);
+    buffer = sharedMemoryBuffer->createSharedBuffer(bufferSize);
 #endif
 
     return true;

Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebPasteboardProxyCocoa.mm (286407 => 286408)


--- trunk/Source/WebKit/UIProcess/Cocoa/WebPasteboardProxyCocoa.mm	2021-12-02 02:10:30 UTC (rev 286407)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebPasteboardProxyCocoa.mm	2021-12-02 02:12:37 UTC (rev 286408)
@@ -419,7 +419,7 @@
         auto sharedMemoryBuffer = SharedMemory::map(ipcHandle.handle, SharedMemory::Protection::ReadOnly);
         if (!sharedMemoryBuffer)
             return completionHandler(0);
-        auto buffer = SharedBuffer::create(static_cast<unsigned char *>(sharedMemoryBuffer->data()), static_cast<size_t>(ipcHandle.dataSize));
+        auto buffer = sharedMemoryBuffer->createSharedBuffer(ipcHandle.dataSize);
         auto newChangeCount = PlatformPasteboard(pasteboardName).setBufferForType(buffer.ptr(), pasteboardType);
         didModifyContentsOfPasteboard(connection, pasteboardName, previousChangeCount, newChangeCount);
         completionHandler(newChangeCount);

Modified: trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm (286407 => 286408)


--- trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm	2021-12-02 02:10:30 UTC (rev 286407)
+++ trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm	2021-12-02 02:12:37 UTC (rev 286408)
@@ -588,7 +588,7 @@
     if (!sharedMemoryBuffer)
         return;
 
-    auto buffer = SharedBuffer::create(static_cast<unsigned char*>(sharedMemoryBuffer->data()), static_cast<size_t>(imageHandle.dataSize));
+    auto buffer = sharedMemoryBuffer->createSharedBuffer(imageHandle.dataSize);
     pageClient().saveImageToLibrary(WTFMove(buffer));
 }
 

Modified: trunk/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm (286407 => 286408)


--- trunk/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm	2021-12-02 02:10:30 UTC (rev 286407)
+++ trunk/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm	2021-12-02 02:12:37 UTC (rev 286408)
@@ -240,7 +240,7 @@
     auto sharedMemoryBuffer = SharedMemory::map(ipcHandle.handle, SharedMemory::Protection::ReadOnly);
     if (!sharedMemoryBuffer)
         return nullptr;
-    return SharedBuffer::create(static_cast<unsigned char *>(sharedMemoryBuffer->data()), ipcHandle.dataSize);
+    return sharedMemoryBuffer->createSharedBuffer(ipcHandle.dataSize);
 }
 
 bool WebPageProxy::readSelectionFromPasteboard(const String& pasteboardName)
@@ -275,15 +275,14 @@
     auto sharedMemoryImage = SharedMemory::map(imageHandle.handle, SharedMemory::Protection::ReadOnly);
     if (!sharedMemoryImage)
         return;
+    auto imageBuffer = sharedMemoryImage->createSharedBuffer(imageHandle.dataSize);
 
-    auto imageBuffer = SharedBuffer::create(static_cast<unsigned char*>(sharedMemoryImage->data()), static_cast<size_t>(imageHandle.dataSize));
     RefPtr<SharedBuffer> archiveBuffer;
-
     if (!archiveHandle.handle.isNull()) {
         auto sharedMemoryArchive = SharedMemory::map(archiveHandle.handle, SharedMemory::Protection::ReadOnly);
         if (!sharedMemoryArchive)
             return;
-        archiveBuffer = SharedBuffer::create(static_cast<unsigned char*>(sharedMemoryArchive->data()), static_cast<size_t>(archiveHandle.dataSize));
+        archiveBuffer = sharedMemoryArchive->createSharedBuffer(archiveHandle.dataSize);
     }
     pageClient().setPromisedDataForImage(pasteboardName, WTFMove(imageBuffer), ResourceResponseBase::sanitizeSuggestedFilename(filename), extension, title, url, visibleURL, WTFMove(archiveBuffer), originIdentifier);
 }

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp (286407 => 286408)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp	2021-12-02 02:10:30 UTC (rev 286407)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp	2021-12-02 02:12:37 UTC (rev 286408)
@@ -145,7 +145,9 @@
     if (ipcHandle.handle.isNull())
         return nullptr;
     RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::map(ipcHandle.handle, SharedMemory::Protection::ReadOnly);
-    return SharedBuffer::create(static_cast<unsigned char *>(sharedMemoryBuffer->data()), ipcHandle.dataSize);
+    if (!sharedMemoryBuffer)
+        return nullptr;
+    return sharedMemoryBuffer->createSharedBuffer(ipcHandle.dataSize);
 }
 
 void WebPlatformStrategies::getPathnamesForType(Vector<String>& pathnames, const String& pasteboardType, const String& pasteboardName, const PasteboardContext* context)
@@ -412,7 +414,9 @@
         return nullptr;
 
     RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::map(ipcHandle.handle, SharedMemory::Protection::ReadOnly);
-    return SharedBuffer::create(static_cast<unsigned char *>(sharedMemoryBuffer->data()), ipcHandle.dataSize);
+    if (!sharedMemoryBuffer)
+        return nullptr;
+    return sharedMemoryBuffer->createSharedBuffer(ipcHandle.dataSize);
 }
 
 URL WebPlatformStrategies::readURLFromPasteboard(size_t index, const String& pasteboardName, String& title, const PasteboardContext* context)

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (286407 => 286408)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-12-02 02:10:30 UTC (rev 286407)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-12-02 02:12:37 UTC (rev 286408)
@@ -7738,7 +7738,7 @@
         if (!sharedMemory)
             continue;
 
-        document->appHighlightStorage().restoreAndScrollToAppHighlight(SharedBuffer::create(static_cast<const uint8_t*>(sharedMemory->data()), ipcHandle.dataSize), i == index ? ScrollToHighlight::Yes : ScrollToHighlight::No);
+        document->appHighlightStorage().restoreAndScrollToAppHighlight(sharedMemory->createSharedBuffer(ipcHandle.dataSize), i == index ? ScrollToHighlight::Yes : ScrollToHighlight::No);
         i++;
     }
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to