REGRESSION(AppleWebKit/605.1.15): WebDownloadDelegate delegate methods called on...
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Apr 2019 21:52:51 +0000 (21:52 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Apr 2019 21:52:51 +0000 (21:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190918
<rdar://problem/45603890>

Reviewed by Darin Adler.

Source/WebKitLegacy/mac:

Since we not doing networking on the main thread but WebView is to be used on the main thread,
we need to hop delegate calls to the main thread similarly to how we do it in the non-download
delegate calls in WebCoreResourceHandleAsOperationQueueDelegate.

* Misc/WebDownload.mm:
(-[WebDownloadInternal downloadDidBegin:]):
(-[WebDownloadInternal download:willSendRequest:redirectResponse:]):
(-[WebDownloadInternal download:didReceiveAuthenticationChallenge:]):
(-[WebDownloadInternal download:didReceiveResponse:]):
(-[WebDownloadInternal download:didReceiveDataOfLength:]):
(-[WebDownloadInternal download:shouldDecodeSourceDataOfMIMEType:]):
(-[WebDownloadInternal download:decideDestinationWithSuggestedFilename:]):
(-[WebDownloadInternal download:didCreateDestination:]):
(-[WebDownloadInternal downloadDidFinish:]):
(-[WebDownloadInternal download:didFailWithError:]):

Tools:

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKitLegacy/mac/DownloadThread.mm: Added.
(-[DownloadThreadChecker webView:decidePolicyForMIMEType:request:frame:decisionListener:]):
(-[DownloadThreadChecker downloadDidBegin:]):
(-[DownloadThreadChecker download:shouldDecodeSourceDataOfMIMEType:]):
(-[DownloadThreadChecker download:decideDestinationWithSuggestedFilename:]):
(-[DownloadThreadChecker download:didCreateDestination:]):
(-[DownloadThreadChecker downloadDidFinish:]):
(TestWebKitAPI::TEST):

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

Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/Misc/WebDownload.mm
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKitLegacy/mac/DownloadThread.mm [new file with mode: 0644]

index a3c88f3..118637b 100644 (file)
@@ -1,3 +1,27 @@
+2019-04-05  Alex Christensen  <achristensen@webkit.org>
+
+        REGRESSION(AppleWebKit/605.1.15): WebDownloadDelegate delegate methods called on non-main thread
+        https://bugs.webkit.org/show_bug.cgi?id=190918
+        <rdar://problem/45603890>
+
+        Reviewed by Darin Adler.
+
+        Since we not doing networking on the main thread but WebView is to be used on the main thread,
+        we need to hop delegate calls to the main thread similarly to how we do it in the non-download
+        delegate calls in WebCoreResourceHandleAsOperationQueueDelegate.
+
+        * Misc/WebDownload.mm:
+        (-[WebDownloadInternal downloadDidBegin:]):
+        (-[WebDownloadInternal download:willSendRequest:redirectResponse:]):
+        (-[WebDownloadInternal download:didReceiveAuthenticationChallenge:]):
+        (-[WebDownloadInternal download:didReceiveResponse:]):
+        (-[WebDownloadInternal download:didReceiveDataOfLength:]):
+        (-[WebDownloadInternal download:shouldDecodeSourceDataOfMIMEType:]):
+        (-[WebDownloadInternal download:decideDestinationWithSuggestedFilename:]):
+        (-[WebDownloadInternal download:didCreateDestination:]):
+        (-[WebDownloadInternal downloadDidFinish:]):
+        (-[WebDownloadInternal download:didFailWithError:]):
+
 2019-04-05  Eric Carlson  <eric.carlson@apple.com>
 
         Remove AUDIO_TOOLBOX_AUDIO_SESSION
index df54d11..0bbeb15 100644 (file)
@@ -39,6 +39,7 @@
 #import <WebKitLegacy/WebPanelAuthenticationHandler.h>
 #import <pal/spi/cocoa/NSURLDownloadSPI.h>
 #import <wtf/Assertions.h>
+#import <wtf/MainThread.h>
 
 using namespace WebCore;
 
@@ -82,12 +83,23 @@ using namespace WebCore;
 
 - (void)downloadDidBegin:(NSURLDownload *)download
 {
-    [realDelegate downloadDidBegin:download];
+    callOnMainThread([realDelegate = retainPtr(realDelegate), download = retainPtr(download)] {
+        [realDelegate downloadDidBegin:download.get()];
+    });
 }
 
 - (NSURLRequest *)download:(NSURLDownload *)download willSendRequest:(NSURLRequest *)request redirectResponse:(NSURLResponse *)redirectResponse
 {
-    return [realDelegate download:download willSendRequest:request redirectResponse:redirectResponse];
+    RetainPtr<NSURLRequest> returnValue;
+    auto work = [&] {
+        ASSERT(isMainThread());
+        returnValue = [realDelegate download:download willSendRequest:request redirectResponse:redirectResponse];
+    };
+    if (isMainThread())
+        work();
+    else
+        dispatch_sync(dispatch_get_main_queue(), work);
+    return returnValue.autorelease();
 }
 
 - (void)download:(NSURLDownload *)download didReceiveAuthenticationChallenge:(NSURLAuthenticationChallenge *)challenge
@@ -103,51 +115,74 @@ using namespace WebCore;
     }
 
     if ([realDelegate respondsToSelector:@selector(download:didReceiveAuthenticationChallenge:)]) {
-        [realDelegate download:download didReceiveAuthenticationChallenge:challenge];
+        callOnMainThread([realDelegate = retainPtr(realDelegate), download = retainPtr(download), challenge = retainPtr(challenge)] {
+            [realDelegate download:download.get() didReceiveAuthenticationChallenge:challenge.get()];
+        });
     } else {
-        NSWindow *window = nil;
-        if ([realDelegate respondsToSelector:@selector(downloadWindowForAuthenticationSheet:)]) {
-            window = [realDelegate downloadWindowForAuthenticationSheet:(WebDownload *)download];
-        }
+        callOnMainThread([realDelegate = retainPtr(realDelegate), download = retainPtr(download), challenge = retainPtr(challenge)] {
+            NSWindow *window = nil;
+            if ([realDelegate respondsToSelector:@selector(downloadWindowForAuthenticationSheet:)])
+                window = [realDelegate downloadWindowForAuthenticationSheet:(WebDownload *)download.get()];
 
-        [[WebPanelAuthenticationHandler sharedHandler] startAuthentication:challenge window:window];
+            [[WebPanelAuthenticationHandler sharedHandler] startAuthentication:challenge.get() window:window];
+        });
     }
 #endif
 }
 
 - (void)download:(NSURLDownload *)download didReceiveResponse:(NSURLResponse *)response
 {
-    [realDelegate download:download didReceiveResponse:response];
+    callOnMainThread([realDelegate = retainPtr(realDelegate), download = retainPtr(download), response = retainPtr(response)] {
+        [realDelegate download:download.get() didReceiveResponse:response.get()];
+    });
 }
 
 - (void)download:(NSURLDownload *)download didReceiveDataOfLength:(NSUInteger)length
 {
-    [realDelegate download:download didReceiveDataOfLength:length];
+    callOnMainThread([realDelegate = retainPtr(realDelegate), download = retainPtr(download), length] {
+        [realDelegate download:download.get() didReceiveDataOfLength:length];
+    });
 }
 
 - (BOOL)download:(NSURLDownload *)download shouldDecodeSourceDataOfMIMEType:(NSString *)encodingType
 {
-    return [realDelegate download:download shouldDecodeSourceDataOfMIMEType:encodingType];
+    BOOL returnValue = NO;
+    auto work = [&] {
+        returnValue = [realDelegate download:download shouldDecodeSourceDataOfMIMEType:encodingType];
+    };
+    if (isMainThread())
+        work();
+    else
+        dispatch_sync(dispatch_get_main_queue(), work);
+    return returnValue;
 }
 
 - (void)download:(NSURLDownload *)download decideDestinationWithSuggestedFilename:(NSString *)filename
 {
-    [realDelegate download:download decideDestinationWithSuggestedFilename:filename];
+    callOnMainThread([realDelegate = retainPtr(realDelegate), download = retainPtr(download), filename = retainPtr(filename)] {
+        [realDelegate download:download.get() decideDestinationWithSuggestedFilename:filename.get()];
+    });
 }
 
 - (void)download:(NSURLDownload *)download didCreateDestination:(NSString *)path
 {
-    [realDelegate download:download didCreateDestination:path];
+    callOnMainThread([realDelegate = retainPtr(realDelegate), download = retainPtr(download), path = retainPtr(path)] {
+        [realDelegate download:download.get() didCreateDestination:path.get()];
+    });
 }
 
 - (void)downloadDidFinish:(NSURLDownload *)download
 {
-    [realDelegate downloadDidFinish:download];
+    callOnMainThread([realDelegate = retainPtr(realDelegate), download = retainPtr(download)] {
+        [realDelegate downloadDidFinish:download.get()];
+    });
 }
 
 - (void)download:(NSURLDownload *)download didFailWithError:(NSError *)error
 {
-    [realDelegate download:download didFailWithError:error];
+    callOnMainThread([realDelegate = retainPtr(realDelegate), download = retainPtr(download), error = retainPtr(error)] {
+        [realDelegate download:download.get() didFailWithError:error.get()];
+    });
 }
 
 @end
index 9c7a379..b66c5cc 100644 (file)
                5CCB10E4213457E000AC5AF0 /* ShouldGoToBackForwardListItem.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CCB10DF2134579D00AC5AF0 /* ShouldGoToBackForwardListItem.mm */; };
                5CE354D91E70DA5C00BEFE3B /* WKContentExtensionStore.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CE354D81E70D9C300BEFE3B /* WKContentExtensionStore.mm */; };
                5CEAB5E11FA939F400A77FAA /* _WKInputDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CEAB5DF1FA937CB00A77FAA /* _WKInputDelegate.mm */; };
+               5CF540E92257E67C00E6BC0E /* DownloadThread.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CF540E82257E64B00E6BC0E /* DownloadThread.mm */; };
                5E4B1D2E1D404C6100053621 /* WKScrollViewDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5E4B1D2C1D404C6100053621 /* WKScrollViewDelegate.mm */; };
                631EFFF61E7B5E8D00D2EBB8 /* Geolocation.mm in Sources */ = {isa = PBXBuildFile; fileRef = 631EFFF51E7B5E8D00D2EBB8 /* Geolocation.mm */; };
                634910E01E9D3FF300880309 /* CoreLocation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 634910DF1E9D3FF300880309 /* CoreLocation.framework */; };
                5CCB10E02134579D00AC5AF0 /* ResponsivenessTimer.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ResponsivenessTimer.mm; sourceTree = "<group>"; };
                5CE354D81E70D9C300BEFE3B /* WKContentExtensionStore.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKContentExtensionStore.mm; sourceTree = "<group>"; };
                5CEAB5DF1FA937CB00A77FAA /* _WKInputDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = _WKInputDelegate.mm; sourceTree = "<group>"; };
+               5CF540E82257E64B00E6BC0E /* DownloadThread.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DownloadThread.mm; sourceTree = "<group>"; };
                5E4B1D2C1D404C6100053621 /* WKScrollViewDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WKScrollViewDelegate.mm; path = ../ios/WKScrollViewDelegate.mm; sourceTree = "<group>"; };
                631EFFF51E7B5E8D00D2EBB8 /* Geolocation.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = Geolocation.mm; sourceTree = "<group>"; };
                634910DF1E9D3FF300880309 /* CoreLocation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CoreLocation.framework; path = System/Library/Frameworks/CoreLocation.framework; sourceTree = SDKROOT; };
                        children = (
                                9BD5111B1FE8E11600D2B630 /* AccessingPastedImage.mm */,
                                6B306105218A372900F5A802 /* ClosingWebView.mm */,
+                               5CF540E82257E64B00E6BC0E /* DownloadThread.mm */,
                                5C6E27A6224EEBEA00128736 /* URLCanonicalization.mm */,
                        );
                        path = mac;
                                7CCE7F231A411AF600447C4C /* Download.mm in Sources */,
                                7CCE7EEE1A411AE600447C4C /* DownloadDecideDestinationCrash.cpp in Sources */,
                                637281A721AE1386009E0DE6 /* DownloadProgress.mm in Sources */,
+                               5CF540E92257E67C00E6BC0E /* DownloadThread.mm in Sources */,
                                F4D4F3B61E4E2BCB00BB2767 /* DragAndDropSimulatorIOS.mm in Sources */,
                                F46128B7211C8ED500D9FADB /* DragAndDropSimulatorMac.mm in Sources */,
                                F46128D7211E489C00D9FADB /* DragAndDropTests.mm in Sources */,
diff --git a/Tools/TestWebKitAPI/Tests/WebKitLegacy/mac/DownloadThread.mm b/Tools/TestWebKitAPI/Tests/WebKitLegacy/mac/DownloadThread.mm
new file mode 100644 (file)
index 0000000..b65e54b
--- /dev/null
@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2019 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import "config.h"
+
+#import "PlatformUtilities.h"
+#import <WebKit/WebView.h>
+#import <wtf/RetainPtr.h>
+
+#if PLATFORM(MAC)
+
+static bool done;
+static RetainPtr<NSString> destination;
+
+@interface DownloadThreadChecker : NSObject <WebDownloadDelegate, WebPolicyDelegate>
+@end
+
+@implementation DownloadThreadChecker
+
+- (void)webView:(WebView *)sender decidePolicyForMIMEType:(NSString *)type request:(NSURLRequest *)request frame:(WebFrame *)frame decisionListener:(id<WebPolicyDecisionListener>)listener
+{
+    [listener download];
+}
+
+- (void)downloadDidBegin:(NSURLDownload *)download
+{
+    EXPECT_TRUE([NSThread isMainThread]);
+}
+
+- (BOOL)download:(NSURLDownload *)download shouldDecodeSourceDataOfMIMEType:(NSString *)encodingType
+{
+    EXPECT_TRUE([NSThread isMainThread]);
+    return YES;
+}
+
+- (void)download:(NSURLDownload *)download decideDestinationWithSuggestedFilename:(NSString *)filename
+{
+    EXPECT_TRUE([NSThread isMainThread]);
+}
+
+- (void)download:(NSURLDownload *)download didCreateDestination:(NSString *)path
+{
+    destination = path;
+    EXPECT_TRUE([NSThread isMainThread]);
+}
+
+- (void)downloadDidFinish:(NSURLDownload *)download
+{
+    EXPECT_TRUE([NSThread isMainThread]);
+    done = true;
+}
+
+@end
+
+namespace TestWebKitAPI {
+
+TEST(WebKitLegacy, DownloadThread)
+{
+    auto delegate = adoptNS([DownloadThreadChecker new]);
+    auto webView = adoptNS([[WebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400) frameName:nil groupName:nil]);
+    [webView setPolicyDelegate:delegate.get()];
+    [webView setDownloadDelegate:delegate.get()];
+
+    [[webView mainFrame] loadRequest:[NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]]];
+    Util::run(&done);
+
+    EXPECT_TRUE([[NSFileManager defaultManager] fileExistsAtPath:destination.get()]);
+    [[NSFileManager defaultManager] removeItemAtPath:destination.get() error:nil];
+    EXPECT_FALSE([[NSFileManager defaultManager] fileExistsAtPath:destination.get()]);
+}
+
+} // namespace TestWebKitAPI
+
+#endif // PLATFORM(MAC)