Attachment 372300 Details for Bug 198945 – Patch

[patch] Patch

bug-198945-20190617173431.patch (text/plain), 3.37 KB, created by Brady Eidson on 2019-06-17 17:34:31 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Brady Eidson

Created: 2019-06-17 17:34:31 PDT

Size: 3.37 KB

patch

obsolete

>Subversion Revision: 246526
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index ce34490edf06eec0e15f426e0fa45d76739d363a..9d3ad34607a7145e3f2eaed40b4cece5618a9bff 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,23 @@
>+2019-06-17  Brady Eidson  <beidson@apple.com>
>+
>+        Handle NSProgress calling our cancellation handler on background threads (and calling it more than once).
>+        <rdar://problem/51392926> and https://bugs.webkit.org/show_bug.cgi?id=198945
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        If you have a download in progress and quickly tap the button to cancel it twice, then:
>+            - NSProgress calls our cancellation handler on a non-main thread, which we can't handle.
>+            - They do it more than once, which is also bad.
>+        
>+        Let's work around that.
>+
>+        * NetworkProcess/Downloads/Download.cpp:
>+        (WebKit::Download::cancel):
>+
>+        * NetworkProcess/Downloads/cocoa/WKDownloadProgress.mm:
>+        (-[WKDownloadProgress initWithDownloadTask:download:URL:sandboxExtension:]): Move the work to the main thread,
>+          and correctly only do it once.
>+
> 2019-06-17  Alex Christensen  <achristensen@webkit.org>
> 
>         Protect StorageManager::m_localStorageNamespaces with a Lock
>diff --git a/Source/WebKit/NetworkProcess/Downloads/Download.cpp b/Source/WebKit/NetworkProcess/Downloads/Download.cpp
>index e210d0dcabf084455dd46a27c0fbcf2da3bd5247..216c30c7ba7c342e60ed16b8c2a40292a9842e5c 100644
>--- a/Source/WebKit/NetworkProcess/Downloads/Download.cpp
>+++ b/Source/WebKit/NetworkProcess/Downloads/Download.cpp
>@@ -86,6 +86,8 @@ Download::~Download()
> 
> void Download::cancel()
> {
>+    RELEASE_ASSERT(isMainThread());
>+
>     m_wasCanceled = true;
>     if (m_download) {
>         m_download->cancel();
>diff --git a/Source/WebKit/NetworkProcess/Downloads/cocoa/WKDownloadProgress.mm b/Source/WebKit/NetworkProcess/Downloads/cocoa/WKDownloadProgress.mm
>index 81b5937161895bb1d57bb870ca2ea421ef073fca..ea79d3ad199581aff6a49b453fbc87d03260bd72 100644
>--- a/Source/WebKit/NetworkProcess/Downloads/cocoa/WKDownloadProgress.mm
>+++ b/Source/WebKit/NetworkProcess/Downloads/cocoa/WKDownloadProgress.mm
>@@ -29,6 +29,7 @@
> #import "Download.h"
> #import <pal/spi/cocoa/NSProgressSPI.h>
> #import <wtf/BlockPtr.h>
>+#import <wtf/MainThread.h>
> #import <wtf/WeakObjCPtr.h>
> 
> static void* WKDownloadProgressBytesExpectedToReceiveCountContext = &WKDownloadProgressBytesExpectedToReceiveCountContext;
>@@ -65,13 +66,27 @@ - (instancetype)initWithDownloadTask:(NSURLSessionDownloadTask *)task download:(
>     m_sandboxExtension = sandboxExtension;
> 
>     self.cancellable = YES;
>-    self.cancellationHandler = makeBlockPtr([weakSelf = WeakObjCPtr<WKDownloadProgress> { self }] {
>+
>+    Function<void()> cancelFunction = [weakSelf = WeakObjCPtr<WKDownloadProgress> { self }] {
>         auto strongSelf = weakSelf.get();
>         if (!strongSelf)
>             return;
> 
>         if (auto* download = strongSelf.get()->m_download)
>             download->cancel();
>+    };
>+
>+    self.cancellationHandler = makeBlockPtr([cancelFunction = WTFMove(cancelFunction)]() mutable {
>+        if (!cancelFunction)
>+            return;
>+
>+        if (!isMainThread()) {
>+            callOnMainThread(WTFMove(cancelFunction));
>+            return;
>+        }
>+
>+        cancelFunction();
>+        cancelFunction = nullptr;
>     }).get();
> 
>     return self;

Actions: View | Formatted Diff | Diff

Attachments on bug 198945: 372300 | 372366