[iOS] Crash in WebResourceLoaderQuickLookDelegate when the client cancels the navigat...
authoraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Oct 2016 01:02:27 +0000 (01:02 +0000)
committeraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Oct 2016 01:02:27 +0000 (01:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162950
<rdar://problem/23759114>

Reviewed by Brady Eidson.

Source/WebCore:

When we receive data from QLPreviewConverter for the first time, we call
ResourceLoader::didReceiveResponse() with the preview NSURLResponse from QuickLook. If the
client decides to cancel this navigation in decidePolicyForResponse(),
WebResourceLoaderQuickLookDelegate will end up with a null _resourceLoader after
didReceiveResponse() returns. This change adds null checks in the methods that use
_resourceLoader after calling -_sendDidReceiveResponseIfNecessary.

New API test: QuickLook.CancelNavigationAfterResponse

* platform/network/ios/QuickLook.mm:
(-[WebResourceLoaderQuickLookDelegate connection:didReceiveDataArray:]): Changed to only
call ResourceLoader::didReceiveDataArray() if _resourceLoader is non-null.
(-[WebResourceLoaderQuickLookDelegate connection:didReceiveData:lengthReceived:]): Ditto for
ResourceLoader::didReceiveData().
(-[WebResourceLoaderQuickLookDelegate connection:didFailWithError:]): Ditto for
ResourceLoader::didFail().

Tools:

Added a new API test.

* TestWebKitAPI/Tests/WebKit2Cocoa/QuickLook.mm: Sorted imports and removed redundant
initialization of static bools.
(runTest): Factored out the common test logic between QuickLook.NavigationDelegate and
QuickLook.CancelNavigationAfterResponse.
(TEST): Added QuickLook.CancelNavigationAfterResponse.
(-[QuickLookDecidePolicyDelegate
webView:decidePolicyForNavigationResponse:decisionHandler:]): Canceled the navigation.
(-[QuickLookDecidePolicyDelegate webView:didFailProvisionalNavigation:withError:]): Set
isDone to true.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/network/ios/QuickLook.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/QuickLook.mm

index d2d1ad7..7658d6b 100644 (file)
@@ -1,3 +1,28 @@
+2016-10-04  Andy Estes  <aestes@apple.com>
+
+        [iOS] Crash in WebResourceLoaderQuickLookDelegate when the client cancels the navigation to a QuickLook resource
+        https://bugs.webkit.org/show_bug.cgi?id=162950
+        <rdar://problem/23759114>
+
+        Reviewed by Brady Eidson.
+
+        When we receive data from QLPreviewConverter for the first time, we call
+        ResourceLoader::didReceiveResponse() with the preview NSURLResponse from QuickLook. If the
+        client decides to cancel this navigation in decidePolicyForResponse(),
+        WebResourceLoaderQuickLookDelegate will end up with a null _resourceLoader after
+        didReceiveResponse() returns. This change adds null checks in the methods that use
+        _resourceLoader after calling -_sendDidReceiveResponseIfNecessary.
+
+        New API test: QuickLook.CancelNavigationAfterResponse
+
+        * platform/network/ios/QuickLook.mm:
+        (-[WebResourceLoaderQuickLookDelegate connection:didReceiveDataArray:]): Changed to only
+        call ResourceLoader::didReceiveDataArray() if _resourceLoader is non-null.
+        (-[WebResourceLoaderQuickLookDelegate connection:didReceiveData:lengthReceived:]): Ditto for
+        ResourceLoader::didReceiveData().
+        (-[WebResourceLoaderQuickLookDelegate connection:didFailWithError:]): Ditto for
+        ResourceLoader::didFail().
+
 2016-10-04  Chris Dumez  <cdumez@apple.com>
 
         Add support for KeyboardEvent.isComposing attribute
index 13d1ddc..a68bde1 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -309,7 +309,8 @@ const char* WebCore::QLPreviewProtocol()
     if (_hasFailed)
         return;
 
-    _resourceLoader->didReceiveDataArray(reinterpret_cast<CFArrayRef>(dataArray));
+    if (_resourceLoader)
+        _resourceLoader->didReceiveDataArray(reinterpret_cast<CFArrayRef>(dataArray));
 }
 #endif
 
@@ -327,7 +328,9 @@ const char* WebCore::QLPreviewProtocol()
     // ResourceHandleMac.cpp added for a different bug.
     if (![data length])
         return;
-    _resourceLoader->didReceiveData(reinterpret_cast<const char*>([data bytes]), [data length], lengthReceived, DataPayloadBytes);
+
+    if (_resourceLoader)
+        _resourceLoader->didReceiveData(reinterpret_cast<const char*>([data bytes]), [data length], lengthReceived, DataPayloadBytes);
 }
 
 - (void)connectionDidFinishLoading:(NSURLConnection *)connection
@@ -348,7 +351,8 @@ const char* WebCore::QLPreviewProtocol()
     if (_hasFailed)
         return;
 
-    _resourceLoader->didFail(ResourceError(error));
+    if (_resourceLoader)
+        _resourceLoader->didFail(ResourceError(error));
 }
 
 - (void)detachHandle
index 7ef3d1f..fc3b4d9 100644 (file)
@@ -1,3 +1,23 @@
+2016-10-04  Andy Estes  <aestes@apple.com>
+
+        [iOS] Crash in WebResourceLoaderQuickLookDelegate when the client cancels the navigation to a QuickLook resource
+        https://bugs.webkit.org/show_bug.cgi?id=162950
+        <rdar://problem/23759114>
+
+        Reviewed by Brady Eidson.
+
+        Added a new API test.
+
+        * TestWebKitAPI/Tests/WebKit2Cocoa/QuickLook.mm: Sorted imports and removed redundant
+        initialization of static bools.
+        (runTest): Factored out the common test logic between QuickLook.NavigationDelegate and
+        QuickLook.CancelNavigationAfterResponse.
+        (TEST): Added QuickLook.CancelNavigationAfterResponse.
+        (-[QuickLookDecidePolicyDelegate
+        webView:decidePolicyForNavigationResponse:decisionHandler:]): Canceled the navigation.
+        (-[QuickLookDecidePolicyDelegate webView:didFailProvisionalNavigation:withError:]): Set
+        isDone to true.
+
 2016-10-04  Ryosuke Niwa  <rniwa@webkit.org>
 
         Add the support for running ES6SampleBench to run-benchmark
index 95bfc05..15158c7 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 
 #if PLATFORM(IOS)
 
+#import "PlatformUtilities.h"
 #import <WebKit/WKNavigationDelegatePrivate.h>
 #import <WebKit/WKWebView.h>
 #import <wtf/RetainPtr.h>
-#import "PlatformUtilities.h"
 
-static bool isDone = false;
-static bool didStartQuickLookLoad = false;
-static bool didFinishQuickLookLoad = false;
+static bool isDone;
+static bool didStartQuickLookLoad;
+static bool didFinishQuickLookLoad;
 
 @interface QuickLookNavigationDelegate : NSObject <WKNavigationDelegatePrivate>
 @end
@@ -61,17 +61,44 @@ static bool didFinishQuickLookLoad = false;
 
 @end
 
-TEST(QuickLook, NavigationDelegate)
+static void runTest(Class navigationDelegateClass)
 {
     auto webView = adoptNS([[WKWebView alloc] init]);
-    auto navigationDelegate = adoptNS([[QuickLookNavigationDelegate alloc] init]);
+    auto navigationDelegate = adoptNS([[navigationDelegateClass alloc] init]);
     [webView setNavigationDelegate:navigationDelegate.get()];
     [webView loadRequest:[NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"pages" withExtension:@"pages" subdirectory:@"TestWebKitAPI.resources"]]];
 
+    isDone = false;
     TestWebKitAPI::Util::run(&isDone);
+}
 
+TEST(QuickLook, NavigationDelegate)
+{
+    runTest([QuickLookNavigationDelegate class]);
     EXPECT_TRUE(didStartQuickLookLoad);
     EXPECT_TRUE(didFinishQuickLookLoad);
 }
 
+@interface QuickLookDecidePolicyDelegate : NSObject <WKNavigationDelegate>
+@end
+
+@implementation QuickLookDecidePolicyDelegate
+
+- (void)webView:(WKWebView *)webView decidePolicyForNavigationResponse:(WKNavigationResponse *)navigationResponse decisionHandler:(void (^)(WKNavigationResponsePolicy))decisionHandler
+{
+    decisionHandler(WKNavigationResponsePolicyCancel);
+}
+
+- (void)webView:(WKWebView *)webView didFailProvisionalNavigation:(WKNavigation *)navigation withError:(NSError *)error
+{
+    isDone = true;
+}
+
+@end
+
+TEST(QuickLook, CancelNavigationAfterResponse)
+{
+    runTest([QuickLookDecidePolicyDelegate class]);
+}
+
 #endif // PLATFORM(IOS)