Changeset 286097 in webkit

Timestamp:

Nov 20, 2021, 3:21:55 PM (5 years ago)

Author:

Jean-Yves Avenard

Message:

Remove unnecessary flattening of SharedBuffer when sending them over IPC
​https://bugs.webkit.org/show_bug.cgi?id=233363
rdar://85600684

Reviewed by Sam Weinig.

In several places, a SharedMemory was allocated followed by a copy. To perform
this copy, the SharedBuffer was flattened which could involve a memory allocation
followed by a copy of all segments into the final vector.
We instead make use of SharedBuffer::copyBuffer convenience method which will simply
copy the individual segment into the newly allocated SharedMemory.

No change in user observable behaviour.

Fly-by fix: SharedMemory::allocate is fallible; few instances didn't check the
returned value. So test that it succeeded and prevent a crash (null deref)

• Shared/WebHitTestResultData.cpp:

(WebKit::WebHitTestResultData::WebHitTestResultData):

• UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:

(WebKit::WebPasteboardProxy::getPasteboardBufferForType):
(WebKit::WebPasteboardProxy::readBufferFromPasteboard):

• WebProcess/Plugins/PDF/PDFPlugin.mm:

(WebKit::PDFPlugin::writeItemsToPasteboard):

• WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:

(WebKit::WebPlatformStrategies::setBufferForType):

• WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:

(WebKit::WebDragClient::declareAndWriteDragImage):

• WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::performActionOnElement):

• WebProcess/WebPage/mac/WebPageMac.mm:

(WebKit::WebPage::getDataSelectionForPasteboard):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Shared/WebHitTestResultData.cpp (modified) (1 diff)
• UIProcess/Cocoa/WebPasteboardProxyCocoa.mm (modified) (2 diffs)
• WebProcess/Plugins/PDF/PDFPlugin.mm (modified) (1 diff)
• WebProcess/WebCoreSupport/WebPlatformStrategies.cpp (modified) (1 diff)
• WebProcess/WebCoreSupport/mac/WebDragClientMac.mm (modified) (2 diffs)
• WebProcess/WebPage/ios/WebPageIOS.mm (modified) (1 diff)
• WebProcess/WebPage/mac/WebPageMac.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-11-20  Jean-Yves Avenard  <jya@apple.com>
+
+        Remove unnecessary flattening of SharedBuffer when sending them over IPC
+        https://bugs.webkit.org/show_bug.cgi?id=233363
+        rdar://85600684
+
+        Reviewed by Sam Weinig.
+
+        In several places, a SharedMemory was allocated followed by a copy. To perform
+        this copy, the SharedBuffer was flattened which could involve a memory allocation
+        followed by a copy of all segments into the final vector.
+        We instead make use of SharedBuffer::copyBuffer convenience method which will simply
+        copy the individual segment into the newly allocated SharedMemory.
+
+        No change in user observable behaviour.
+
+        Fly-by fix: SharedMemory::allocate is fallible; few instances didn't check the
+        returned value. So test that it succeeded and prevent a crash (null deref)
+
+        * Shared/WebHitTestResultData.cpp:
+        (WebKit::WebHitTestResultData::WebHitTestResultData):
+        * UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:
+        (WebKit::WebPasteboardProxy::getPasteboardBufferForType):
+        (WebKit::WebPasteboardProxy::readBufferFromPasteboard):
+        * WebProcess/Plugins/PDF/PDFPlugin.mm:
+        (WebKit::PDFPlugin::writeItemsToPasteboard):
+        * WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:
+        (WebKit::WebPlatformStrategies::setBufferForType):
+        * WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:
+        (WebKit::WebDragClient::declareAndWriteDragImage):
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::performActionOnElement):
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::getDataSelectionForPasteboard):
+
 2021-11-20  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebKit/Shared/WebHitTestResultData.cpp

@@ -88 +88 @@
         RefPtr<SharedBuffer> buffer = image->data();
         if (buffer) {
-            imageSharedMemory = WebKit::SharedMemory::allocate(buffer->size());
-            memcpy(imageSharedMemory->data(), buffer->data(), buffer->size());
+            imageSharedMemory = WebKit::SharedMemory::copyBuffer(*buffer);
             imageSize = buffer->size();
         }

trunk/Source/WebKit/UIProcess/Cocoa/WebPasteboardProxyCocoa.mm

@@ -234 +234 @@
         if (!size)
             return completionHandler({ });
-        RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::allocate(size);
+        auto sharedMemoryBuffer = SharedMemory::copyBuffer(*buffer);
         if (!sharedMemoryBuffer)
             return completionHandler({ });
-        memcpy(sharedMemoryBuffer->data(), buffer->data(), size);
         SharedMemory::Handle handle;
         if (!sharedMemoryBuffer->createHandle(handle, SharedMemory::Protection::ReadOnly))
@@ -557 +556 @@
         if (!size)
             return completionHandler({ });
-        RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::allocate(size);
+        auto sharedMemoryBuffer = SharedMemory::copyBuffer(*buffer);
         if (!sharedMemoryBuffer)
             return completionHandler({ });
-        memcpy(sharedMemoryBuffer->data(), buffer->data(), size);
         SharedMemory::Handle handle;
         if (!sharedMemoryBuffer->createHandle(handle, SharedMemory::Protection::ReadOnly))

trunk/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm

@@ -2664 +2664 @@
         } else {
             auto buffer = SharedBuffer::create(data);
+            auto sharedMemory = SharedMemory::copyBuffer(buffer.get());
+            if (!sharedMemory)
+                continue;
             SharedMemory::Handle handle;
-            auto sharedMemory = SharedMemory::allocate(buffer->size());
-            memcpy(sharedMemory->data(), buffer->data(), buffer->size());
             sharedMemory->createHandle(handle, SharedMemory::Protection::ReadOnly);
             webProcess.parentProcessConnection()->sendSync(Messages::WebPasteboardProxy::SetPasteboardBufferForType(pasteboardName, type, SharedMemory::IPCHandle { WTFMove(handle), buffer->size() }, pageIdentifier), Messages::WebPasteboardProxy::SetPasteboardBufferForType::Reply(newChangeCount), 0);

trunk/Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp

@@ -212 +212 @@
     SharedMemory::Handle handle;
     if (buffer && buffer->size()) {
-        RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::allocate(buffer->size());
+        auto sharedMemoryBuffer = SharedMemory::copyBuffer(*buffer);
         // FIXME: Null check prevents crashing, but it is not great that we will have empty pasteboard content for this type,
         // because we've already set the types.
-        if (sharedMemoryBuffer) {
-            memcpy(sharedMemoryBuffer->data(), buffer->data(), buffer->size());
+        if (sharedMemoryBuffer)
             sharedMemoryBuffer->createHandle(handle, SharedMemory::Protection::ReadOnly);
-        }
     }
     int64_t newChangeCount { 0 };

trunk/Source/WebKit/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm

@@ -153 +153 @@
     RefPtr<SharedBuffer> imageBuffer = image->image()->data();
     size_t imageSize = imageBuffer->size();
-    SharedMemory::Handle imageHandle;
-    
-    RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::allocate(imageBuffer->size());
+
+    auto sharedMemoryBuffer = SharedMemory::copyBuffer(*imageBuffer);
     if (!sharedMemoryBuffer)
         return;
-    memcpy(sharedMemoryBuffer->data(), imageBuffer->data(), imageSize);
+    SharedMemory::Handle imageHandle;
     sharedMemoryBuffer->createHandle(imageHandle, SharedMemory::Protection::ReadOnly);
     
@@ -166 +165 @@
     if (data) {
         auto archiveBuffer = SharedBuffer::create((__bridge NSData *)data.get());
-        RefPtr<SharedMemory> archiveSharedMemoryBuffer = SharedMemory::allocate(archiveBuffer->size());
+        auto archiveSharedMemoryBuffer = SharedMemory::copyBuffer(archiveBuffer.get());
         if (!archiveSharedMemoryBuffer)
             return;
         archiveSize = archiveBuffer->size();
-        memcpy(archiveSharedMemoryBuffer->data(), archiveBuffer->data(), archiveSize);
         archiveSharedMemoryBuffer->createHandle(archiveHandle, SharedMemory::Protection::ReadOnly);
     }

trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

@@ -3221 +3221 @@
         if (!buffer)
             return;
-        uint64_t bufferSize = buffer->size();
-        RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::allocate(bufferSize);
-        memcpy(sharedMemoryBuffer->data(), buffer->data(), bufferSize);
+        auto sharedMemoryBuffer = SharedMemory::copyBuffer(*buffer);
+        if (!sharedMemoryBuffer)
+            return;
         SharedMemory::Handle handle;
         sharedMemoryBuffer->createHandle(handle, SharedMemory::Protection::ReadOnly);
-        send(Messages::WebPageProxy::SaveImageToLibrary(SharedMemory::IPCHandle { WTFMove(handle), bufferSize }));
+        send(Messages::WebPageProxy::SaveImageToLibrary(SharedMemory::IPCHandle { WTFMove(handle), buffer->size() }));
     }
 }

trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm

@@ -570 +570 @@
     if (!buffer)
         return completionHandler({ });
-    uint64_t size = buffer->size();
-    RefPtr<SharedMemory> sharedMemoryBuffer = SharedMemory::allocate(size);
-    memcpy(sharedMemoryBuffer->data(), buffer->data(), size);
+    auto sharedMemoryBuffer = SharedMemory::copyBuffer(*buffer);
+    if (!sharedMemoryBuffer)
+        return completionHandler({ });
     SharedMemory::Handle handle;
     sharedMemoryBuffer->createHandle(handle, SharedMemory::Protection::ReadOnly);
-    completionHandler(SharedMemory::IPCHandle { WTFMove(handle), size });
+    completionHandler(SharedMemory::IPCHandle { WTFMove(handle), buffer->size() });
 }