Title: [278494] branches/safari-611-branch/Source/WebKit
Revision
278494
Author
[email protected]
Date
2021-06-04 13:24:34 -0700 (Fri, 04 Jun 2021)

Log Message

Cherry-pick r278441. rdar://problem/78875140

    Fix thread safety issues in [WKShareSheet presentWithParameters]
    https://bugs.webkit.org/show_bug.cgi?id=226611
    <rdar://77736001>

    Reviewed by Ryosuke Niwa.

    The code in [WKShareSheet presentWithParameters] was passing WebCore::RawFile objects to another thread.
    This wasn't safe since WebCore::RawFile contains a WTF::String and a SharedBuffer, both of which are not
    thread safe. Fix this by creating an isolated copy of the fileName String and converting the SharedBuffer
    to a NSData before passing them to the background thread.

    Also rewrite the code from ObjC to C++ because I felt it was clearer and less error-prone.

    * UIProcess/Cocoa/WKShareSheet.mm:
    (appendFilesAsShareableURLs):
    (-[WKShareSheet presentWithParameters:inRect:completionHandler:]):

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@278441 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Diff

Modified: branches/safari-611-branch/Source/WebKit/ChangeLog (278493 => 278494)


--- branches/safari-611-branch/Source/WebKit/ChangeLog	2021-06-04 20:24:32 UTC (rev 278493)
+++ branches/safari-611-branch/Source/WebKit/ChangeLog	2021-06-04 20:24:34 UTC (rev 278494)
@@ -1,5 +1,47 @@
 2021-06-04  Alan Coon  <[email protected]>
 
+        Cherry-pick r278441. rdar://problem/78875140
+
+    Fix thread safety issues in [WKShareSheet presentWithParameters]
+    https://bugs.webkit.org/show_bug.cgi?id=226611
+    <rdar://77736001>
+    
+    Reviewed by Ryosuke Niwa.
+    
+    The code in [WKShareSheet presentWithParameters] was passing WebCore::RawFile objects to another thread.
+    This wasn't safe since WebCore::RawFile contains a WTF::String and a SharedBuffer, both of which are not
+    thread safe. Fix this by creating an isolated copy of the fileName String and converting the SharedBuffer
+    to a NSData before passing them to the background thread.
+    
+    Also rewrite the code from ObjC to C++ because I felt it was clearer and less error-prone.
+    
+    * UIProcess/Cocoa/WKShareSheet.mm:
+    (appendFilesAsShareableURLs):
+    (-[WKShareSheet presentWithParameters:inRect:completionHandler:]):
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@278441 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-06-03  Chris Dumez  <[email protected]>
+
+            Fix thread safety issues in [WKShareSheet presentWithParameters]
+            https://bugs.webkit.org/show_bug.cgi?id=226611
+            <rdar://77736001>
+
+            Reviewed by Ryosuke Niwa.
+
+            The code in [WKShareSheet presentWithParameters] was passing WebCore::RawFile objects to another thread.
+            This wasn't safe since WebCore::RawFile contains a WTF::String and a SharedBuffer, both of which are not
+            thread safe. Fix this by creating an isolated copy of the fileName String and converting the SharedBuffer
+            to a NSData before passing them to the background thread.
+
+            Also rewrite the code from ObjC to C++ because I felt it was clearer and less error-prone.
+
+            * UIProcess/Cocoa/WKShareSheet.mm:
+            (appendFilesAsShareableURLs):
+            (-[WKShareSheet presentWithParameters:inRect:completionHandler:]):
+
+2021-06-04  Alan Coon  <[email protected]>
+
         Cherry-pick r277873. rdar://problem/78875406
 
     [Cocoa] REGRESSION(Sandbox): Font smoothing within Safari doesn't seem to respect AppleFontSmoothing

Modified: branches/safari-611-branch/Source/WebKit/UIProcess/Cocoa/WKShareSheet.mm (278493 => 278494)


--- branches/safari-611-branch/Source/WebKit/UIProcess/Cocoa/WKShareSheet.mm	2021-06-04 20:24:32 UTC (rev 278493)
+++ branches/safari-611-branch/Source/WebKit/UIProcess/Cocoa/WKShareSheet.mm	2021-06-04 20:24:34 UTC (rev 278494)
@@ -37,6 +37,7 @@
 #import <wtf/Scope.h>
 #import <wtf/UUID.h>
 #import <wtf/WeakObjCPtr.h>
+#import <wtf/WorkQueue.h>
 
 #if PLATFORM(IOS_FAMILY)
 #import "UIKitSPI.h"
@@ -89,6 +90,32 @@
     return self;
 }
 
+static void appendFilesAsShareableURLs(RetainPtr<NSMutableArray>&& shareDataArray, const Vector<WebCore::RawFile>& files, NSURL* temporaryDirectory, CompletionHandler<void(RetainPtr<NSMutableArray>&&)>&& completionHandler)
+{
+    struct FileWriteTask {
+        String fileName;
+        RetainPtr<NSData> fileData;
+    };
+    Vector<FileWriteTask> fileWriteTasks;
+    for (auto& file : files)
+        fileWriteTasks.append({ file.fileName.isolatedCopy(), file.fileData->createNSData() });
+
+    auto queue = WorkQueue::create("com.apple.WebKit.WKShareSheet.ShareableFileWriter");
+    queue->dispatch([shareDataArray = WTFMove(shareDataArray), fileWriteTasks = WTFMove(fileWriteTasks), temporaryDirectory = retainPtr(temporaryDirectory), completionHandler = WTFMove(completionHandler)]() mutable {
+        for (auto& fileWriteTask : fileWriteTasks) {
+            NSURL *fileURL = [WKShareSheet writeFileToShareableURL:WebCore::ResourceResponseBase::sanitizeSuggestedFilename(fileWriteTask.fileName) data:fileWriteTask.fileData.get() temporaryDirectory:temporaryDirectory.get()];
+            if (!fileURL) {
+                shareDataArray = nullptr;
+                break;
+            }
+            [shareDataArray addObject:fileURL];
+        }
+        RunLoop::main().dispatch([completionHandler = WTFMove(completionHandler), shareDataArray = WTFMove(shareDataArray)]() mutable {
+            completionHandler(WTFMove(shareDataArray));
+        });
+    });
+}
+
 - (void)presentWithParameters:(const WebCore::ShareDataWithParsedURL &)data inRect:(WTF::Optional<WebCore::FloatRect>)rect completionHandler:(WTF::CompletionHandler<void(bool)>&&)completionHandler
 {
     auto shareDataArray = adoptNS([[NSMutableArray alloc] init]);
@@ -116,35 +143,14 @@
         return;
     }
     
-    if (data.files.size()) {
+    if (!data.files.isEmpty()) {
         _temporaryFileShareDirectory = [WKShareSheet createTemporarySharingDirectory];
-        
-        auto fileWriteGroup = adoptOSObject(dispatch_group_create());
-        auto queue = adoptOSObject(dispatch_queue_create("com.apple.WebKit.WKShareSheet.ShareableFileWriter", DISPATCH_QUEUE_SERIAL));
-        
-        __block bool successful = true;
-        
-        int index = 0;
-        for (auto file : data.files) {
-            dispatch_group_async(fileWriteGroup.get(), queue.get(), ^{
-                if (!successful)
-                    return;
-                NSURL *fileURL = [WKShareSheet writeFileToShareableURL:WebCore::ResourceResponseBase::sanitizeSuggestedFilename(file.fileName) data:file.fileData->createNSData().get() temporaryDirectory:_temporaryFileShareDirectory.get()];
-                if (!fileURL) {
-                    successful = false;
-                    return;
-                }
-                [shareDataArray addObject:fileURL];
-            });
-            index++;
-        }
-        
-        dispatch_group_notify(fileWriteGroup.get(), dispatch_get_main_queue(), ^{
-            if (!successful) {
-                [self dismiss];
+        appendFilesAsShareableURLs(WTFMove(shareDataArray), data.files, _temporaryFileShareDirectory.get(), [retainedSelf = retainPtr(self), rect = WTFMove(rect)](RetainPtr<NSMutableArray>&& shareDataArray) mutable {
+            if (!shareDataArray) {
+                [retainedSelf dismiss];
                 return;
             }
-            [self presentWithShareDataArray:shareDataArray.get() inRect:rect];
+            [retainedSelf presentWithShareDataArray:shareDataArray.get() inRect:rect];
         });
         return;
     }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to