Title: [246568] trunk/Source/WebKit
- Revision
- 246568
- Author
- beid...@apple.com
- Date
- 2019-06-18 14:30:18 -0700 (Tue, 18 Jun 2019)
Log Message
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 Alex Christensen.
If you have a download in progress and quickly tap the button to cancel it multiple times, 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.
- They might even do it multiple times concurrently (on different background dispatch queues)
Let's work around these.
* NetworkProcess/Downloads/Download.cpp:
(WebKit::Download::cancel): Double check we're on the main thread, and handle being called twice.
* NetworkProcess/Downloads/cocoa/WKDownloadProgress.mm:
(-[WKDownloadProgress performCancel]): Actually cancel the WebKit::Download if we still have one.
(-[WKDownloadProgress progressCancelled]): Called when NSProgress calls the cancellation handler, no matter
which thread it does it on. By leveraging std::call_once we handle multiple calls as well as being called
concurrently from different threads. call_once punts the *actual* cancel operation off to the main thread.
(-[WKDownloadProgress initWithDownloadTask:download:URL:sandboxExtension:]): The cancellation handler is
now simply calling 'progressCancelled' on self, assuming the weak pointer for self is still valid.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (246567 => 246568)
--- trunk/Source/WebKit/ChangeLog 2019-06-18 21:21:48 UTC (rev 246567)
+++ trunk/Source/WebKit/ChangeLog 2019-06-18 21:30:18 UTC (rev 246568)
@@ -1,3 +1,28 @@
+2019-06-18 Brady Eidson <beid...@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 Alex Christensen.
+
+ If you have a download in progress and quickly tap the button to cancel it multiple times, 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.
+ - They might even do it multiple times concurrently (on different background dispatch queues)
+
+ Let's work around these.
+
+ * NetworkProcess/Downloads/Download.cpp:
+ (WebKit::Download::cancel): Double check we're on the main thread, and handle being called twice.
+
+ * NetworkProcess/Downloads/cocoa/WKDownloadProgress.mm:
+ (-[WKDownloadProgress performCancel]): Actually cancel the WebKit::Download if we still have one.
+ (-[WKDownloadProgress progressCancelled]): Called when NSProgress calls the cancellation handler, no matter
+ which thread it does it on. By leveraging std::call_once we handle multiple calls as well as being called
+ concurrently from different threads. call_once punts the *actual* cancel operation off to the main thread.
+ (-[WKDownloadProgress initWithDownloadTask:download:URL:sandboxExtension:]): The cancellation handler is
+ now simply calling 'progressCancelled' on self, assuming the weak pointer for self is still valid.
+
2019-06-18 Daniel Bates <daba...@apple.com>
[iOS] Pressing key while holding Command should not insert character
Modified: trunk/Source/WebKit/NetworkProcess/Downloads/Download.cpp (246567 => 246568)
--- trunk/Source/WebKit/NetworkProcess/Downloads/Download.cpp 2019-06-18 21:21:48 UTC (rev 246567)
+++ trunk/Source/WebKit/NetworkProcess/Downloads/Download.cpp 2019-06-18 21:30:18 UTC (rev 246568)
@@ -86,7 +86,12 @@
void Download::cancel()
{
+ RELEASE_ASSERT(isMainThread());
+
+ if (m_wasCanceled)
+ return;
m_wasCanceled = true;
+
if (m_download) {
m_download->cancel();
didCancel({ });
Modified: trunk/Source/WebKit/NetworkProcess/Downloads/cocoa/WKDownloadProgress.mm (246567 => 246568)
--- trunk/Source/WebKit/NetworkProcess/Downloads/cocoa/WKDownloadProgress.mm 2019-06-18 21:21:48 UTC (rev 246567)
+++ trunk/Source/WebKit/NetworkProcess/Downloads/cocoa/WKDownloadProgress.mm 2019-06-18 21:30:18 UTC (rev 246568)
@@ -41,8 +41,27 @@
RetainPtr<NSURLSessionDownloadTask> m_task;
WebKit::Download* m_download;
RefPtr<WebKit::SandboxExtension> m_sandboxExtension;
+ std::once_flag m_progressCancelOnce;
}
+- (void)performCancel
+{
+ if (m_download)
+ m_download->cancel();
+}
+
+- (void)progressCancelled
+{
+ std::call_once(m_progressCancelOnce, [self] {
+ if (!isMainThread()) {
+ [self performSelectorOnMainThread:@selector(performCancel) withObject:nil waitUntilDone:NO];
+ return;
+ }
+
+ [self performCancel];
+ });
+}
+
- (instancetype)initWithDownloadTask:(NSURLSessionDownloadTask *)task download:(WebKit::Download*)download URL:(NSURL *)fileURL sandboxExtension:(RefPtr<WebKit::SandboxExtension>)sandboxExtension
{
if (!(self = [self initWithParent:nil userInfo:nil]))
@@ -66,12 +85,7 @@
self.cancellable = YES;
self.cancellationHandler = makeBlockPtr([weakSelf = WeakObjCPtr<WKDownloadProgress> { self }] {
- auto strongSelf = weakSelf.get();
- if (!strongSelf)
- return;
-
- if (auto* download = strongSelf.get()->m_download)
- download->cancel();
+ [weakSelf.get() progressCancelled];
}).get();
return self;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
