Changeset 206801 in webkit

Timestamp:

Oct 4, 2016, 6:02:27 PM (9 years ago)

Author:

aestes@apple.com

Message:

[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.

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.

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/network/ios/QuickLook.mm (modified) (4 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit2Cocoa/QuickLook.mm (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/platform/network/ios/QuickLook.mm

@@ -1 +1 @@
 /*
- * 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
@@ -310 +310 @@
         return;
 
-    _resourceLoader->didReceiveDataArray(reinterpret_cast<CFArrayRef>(dataArray));
+    if (_resourceLoader)
+        _resourceLoader->didReceiveDataArray(reinterpret_cast<CFArrayRef>(dataArray));
 }
 #endif
@@ -328 +329 @@
     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);
 }
 
@@ -349 +352 @@
         return;
 
-    _resourceLoader->didFail(ResourceError(error));
+    if (_resourceLoader)
+        _resourceLoader->didFail(ResourceError(error));
 }
 

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/QuickLook.mm

@@ -1 +1 @@
 /*
- * 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
@@ -28 +28 @@
 #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>
@@ -62 +62 @@
 @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)