REGRESSION (r251623): When downloading a QuickLook file, the web view navigates to...
authoraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Nov 2019 16:32:36 +0000 (16:32 +0000)
committeraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Nov 2019 16:32:36 +0000 (16:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=203790
<rdar://problem/56795440>

Reviewed by Alex Christensen.

Source/WebCore:

When a PreviewConverter fails updating due to a call to PreviewConverter::failedUpdating(),
we should not dispatch previewConverterDidFailUpdating. LegacyPreviewLoader handles that
callback by calling ResourceLoader::didFail() with a WebKitErrorCannotShowURL error code,
but when downloading or cancelling we must fail with a
WebKitErrorFrameLoadInterruptedByPolicyChange error code instead.

This patch teaches PreviewConverter to distinguish between internal updating errors and
external calls to failedUpdating(), only dispatching previewConverterDidFailUpdating in the
former case.

Updated QuickLook API tests.

* platform/PreviewConverter.cpp:
(WebCore::PreviewConverter::updateMainResource):
(WebCore::PreviewConverter::failedUpdating):
(WebCore::PreviewConverter::didFailUpdating):
* platform/PreviewConverter.h:

Tools:

Tested that "download" and "cancel" policy decisions for QuickLook files result in a
provisional navigation failure with error code WebKitErrorFrameLoadInterruptedByPolicyChange.

* TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm:
(-[QuickLookDelegate navigationError]):
(-[QuickLookDelegate _webView:didFailNavigation:withError:userInfo:]):
(-[QuickLookDelegate webView:didFailProvisionalNavigation:withError:]):
(TEST):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/PreviewConverter.cpp
Source/WebCore/platform/PreviewConverter.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm

index 3f2e311..385c9f5 100644 (file)
@@ -1,3 +1,29 @@
+2019-11-04  Andy Estes  <aestes@apple.com>
+
+        REGRESSION (r251623): When downloading a QuickLook file, the web view navigates to a "The URL can't be shown" error page instead of staying on the current page
+        https://bugs.webkit.org/show_bug.cgi?id=203790
+        <rdar://problem/56795440>
+
+        Reviewed by Alex Christensen.
+
+        When a PreviewConverter fails updating due to a call to PreviewConverter::failedUpdating(),
+        we should not dispatch previewConverterDidFailUpdating. LegacyPreviewLoader handles that
+        callback by calling ResourceLoader::didFail() with a WebKitErrorCannotShowURL error code,
+        but when downloading or cancelling we must fail with a
+        WebKitErrorFrameLoadInterruptedByPolicyChange error code instead.
+
+        This patch teaches PreviewConverter to distinguish between internal updating errors and
+        external calls to failedUpdating(), only dispatching previewConverterDidFailUpdating in the
+        former case.
+
+        Updated QuickLook API tests.
+
+        * platform/PreviewConverter.cpp:
+        (WebCore::PreviewConverter::updateMainResource):
+        (WebCore::PreviewConverter::failedUpdating):
+        (WebCore::PreviewConverter::didFailUpdating):
+        * platform/PreviewConverter.h:
+
 2019-11-04  youenn fablet  <youenn@apple.com>
 
         RealtimeOutgoingAudioSource/RealtimeOutgoingVideoSource should unobserve before destruction time
index fdbf738..f8e9136 100644 (file)
@@ -78,7 +78,7 @@ void PreviewConverter::updateMainResource()
 
     auto provider = m_provider.get();
     if (!provider) {
-        failedUpdating();
+        didFailUpdating();
         return;
     }
 
@@ -86,7 +86,7 @@ void PreviewConverter::updateMainResource()
         if (buffer)
             appendFromBuffer(*buffer);
         else
-            failedUpdating();
+            didFailUpdating();
     });
 }
 
@@ -124,10 +124,6 @@ void PreviewConverter::failedUpdating()
 
     m_state = State::FailedUpdating;
     platformFailedAppending();
-
-    iterateClients([&](auto& client) {
-        client.previewConverterDidFailUpdating(*this);
-    });
 }
 
 bool PreviewConverter::hasClient(PreviewConverterClient& client) const
@@ -195,6 +191,15 @@ void PreviewConverter::didFailConvertingWithError(const ResourceError& error)
     });
 }
 
+void PreviewConverter::didFailUpdating()
+{
+    failedUpdating();
+
+    iterateClients([&](auto& client) {
+        client.previewConverterDidFailUpdating(*this);
+    });
+}
+
 void PreviewConverter::replayToClient(PreviewConverterClient& client)
 {
     if (!hasClient(client))
index 70df224..2a37292 100644 (file)
@@ -97,6 +97,7 @@ private:
     void appendFromBuffer(const SharedBuffer&);
     void didAddClient(PreviewConverterClient&);
     void didFailConvertingWithError(const ResourceError&);
+    void didFailUpdating();
     void replayToClient(PreviewConverterClient&);
 
     void platformAppend(const SharedBufferDataView&);
index c662dea..48117b5 100644 (file)
@@ -1,3 +1,20 @@
+2019-11-04  Andy Estes  <aestes@apple.com>
+
+        REGRESSION (r251623): When downloading a QuickLook file, the web view navigates to a "The URL can't be shown" error page instead of staying on the current page
+        https://bugs.webkit.org/show_bug.cgi?id=203790
+        <rdar://problem/56795440>
+
+        Reviewed by Alex Christensen.
+
+        Tested that "download" and "cancel" policy decisions for QuickLook files result in a
+        provisional navigation failure with error code WebKitErrorFrameLoadInterruptedByPolicyChange.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm:
+        (-[QuickLookDelegate navigationError]):
+        (-[QuickLookDelegate _webView:didFailNavigation:withError:userInfo:]):
+        (-[QuickLookDelegate webView:didFailProvisionalNavigation:withError:]):
+        (TEST):
+
 2019-11-04  Peng Liu  <peng.liu6@apple.com>
 
         Cannot run WebKit layout tests on iOS devices
index b20787b..2a78d02 100644 (file)
@@ -63,6 +63,7 @@ static NSURL * const pagesDocumentURL = [[NSBundle.mainBundle URLForResource:@"p
 @property (nonatomic, readonly) BOOL didFinishNavigation;
 @property (nonatomic, readonly) BOOL didFinishQuickLookLoad;
 @property (nonatomic, readonly) BOOL didStartQuickLookLoad;
+@property (nonatomic, readonly, nullable) NSError *navigationError;
 
 @end
 
@@ -71,6 +72,7 @@ static NSURL * const pagesDocumentURL = [[NSBundle.mainBundle URLForResource:@"p
     NSUInteger _downloadFileSize;
     NSUInteger _expectedFileSize;
     RetainPtr<NSData> _expectedFileData;
+    RetainPtr<NSError> _navigationError;
     RetainPtr<NSString> _expectedFileName;
     RetainPtr<NSString> _expectedFileType;
     RetainPtr<NSString> _expectedMIMEType;
@@ -113,6 +115,11 @@ static void readFile(NSURL *fileURL, NSUInteger& fileSize, RetainPtr<NSString>&
     return self;
 }
 
+- (nullable NSError *)navigationError
+{
+    return _navigationError.get();
+}
+
 - (void)webView:(WKWebView *)webView didStartProvisionalNavigation:(WKNavigation *)navigation
 {
     _didFailNavigation = NO;
@@ -159,7 +166,9 @@ static void readFile(NSURL *fileURL, NSUInteger& fileSize, RetainPtr<NSString>&
 {
     EXPECT_FALSE(_didFailNavigation);
     EXPECT_FALSE(_didFinishNavigation);
+    EXPECT_NULL(_navigationError);
     _didFailNavigation = YES;
+    _navigationError = error;
     isDone = true;
 }
 
@@ -167,7 +176,9 @@ static void readFile(NSURL *fileURL, NSUInteger& fileSize, RetainPtr<NSString>&
 {
     EXPECT_FALSE(_didFailNavigation);
     EXPECT_FALSE(_didFinishNavigation);
+    EXPECT_NULL(_navigationError);
     _didFailNavigation = YES;
+    _navigationError = error;
     isDone = true;
 }
 
@@ -327,6 +338,7 @@ TEST(QuickLook, CancelResponseBeforeLoadingPreview)
 {
     auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL responsePolicy:WKNavigationResponsePolicyCancel]);
     runTestDecideBeforeLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_EQ(WebKitErrorFrameLoadInterruptedByPolicyChange, [delegate navigationError].code);
     EXPECT_FALSE([delegate didFinishNavigation]);
     EXPECT_FALSE([delegate didFinishQuickLookLoad]);
     EXPECT_FALSE([delegate didStartQuickLookLoad]);
@@ -337,6 +349,7 @@ TEST(QuickLook, CancelResponseAfterLoadingPreview)
 {
     auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL previewMIMEType:pagesDocumentPreviewMIMEType responsePolicy:WKNavigationResponsePolicyCancel]);
     runTestDecideAfterLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_EQ(WebKitErrorFrameLoadInterruptedByPolicyChange, [delegate navigationError].code);
     EXPECT_FALSE([delegate didFinishNavigation]);
     EXPECT_TRUE([delegate didFailNavigation]);
     EXPECT_TRUE([delegate didFinishQuickLookLoad]);
@@ -347,6 +360,7 @@ TEST(QuickLook, DownloadResponseBeforeLoadingPreview)
 {
     auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL responsePolicy:_WKNavigationResponsePolicyBecomeDownload]);
     runTestDecideBeforeLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_EQ(WebKitErrorFrameLoadInterruptedByPolicyChange, [delegate navigationError].code);
     EXPECT_FALSE([delegate didFinishNavigation]);
     EXPECT_FALSE([delegate didFinishQuickLookLoad]);
     EXPECT_FALSE([delegate didStartQuickLookLoad]);
@@ -360,6 +374,7 @@ TEST(QuickLook, DownloadResponseAfterLoadingPreview)
 {
     auto delegate = adoptNS([[QuickLookDelegate alloc] initWithExpectedFileURL:pagesDocumentURL previewMIMEType:pagesDocumentPreviewMIMEType responsePolicy:_WKNavigationResponsePolicyBecomeDownload]);
     runTestDecideAfterLoading(delegate.get(), [NSURLRequest requestWithURL:pagesDocumentURL]);
+    EXPECT_EQ(WebKitErrorFrameLoadInterruptedByPolicyChange, [delegate navigationError].code);
     EXPECT_FALSE([delegate didFinishNavigation]);
     EXPECT_TRUE([delegate didFailNavigation]);
     EXPECT_TRUE([delegate didFinishQuickLookLoad]);