Custom protocols should not continue loading after a network process crash
authoraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Feb 2017 01:57:21 +0000 (01:57 +0000)
committeraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Feb 2017 01:57:21 +0000 (01:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=168028
<rdar://problem/27607520>

Reviewed by Brady Eidson.

Source/WebKit2:

WKCustomProtocolLoaders are meant to be owned by CustomProtocolManagerProxy and have their
loads cancelled when CustomProtocolManagerProxy is destroyed. However, WKCustomProtocolLoader
creates a NSURLConnection for which it is the client, so NSURLConnection retains it for the
duration of the load.

These loaders should be explicitly cancelled when their CustomProtocolManagerProxy is destroyed.

New API test: WebKit2CustomProtocolsTest.CloseDuringCustomProtocolLoad

* UIProcess/Network/CustomProtocols/mac/CustomProtocolManagerProxyMac.mm:
(-[WKCustomProtocolLoader customProtocolManagerProxyDestroyed]): Added. Cancels the
_urlConnection and sets _customProtocolManagerProxy to nullptr.
(WebKit::CustomProtocolManagerProxy::~CustomProtocolManagerProxy):
Called -customProtocolManagerProxyDestroyed on every loader in m_loaderMap.

Tools:

* TestWebKitAPI/Tests/WebKit2ObjC/CustomProtocolsTest.mm:
(processGroup):
(-[CloseWhileStartingProtocol startLoading]):
(-[CloseWhileStartingProtocol stopLoading]):
(TestWebKitAPI::runTest):
(TestWebKitAPI::TEST):

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

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/Network/CustomProtocols/mac/CustomProtocolManagerProxyMac.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit2ObjC/CustomProtocolsTest.mm

index 95ed525..2850c26 100644 (file)
@@ -1,3 +1,26 @@
+2017-02-08  Andy Estes  <aestes@apple.com>
+
+        Custom protocols should not continue loading after a network process crash
+        https://bugs.webkit.org/show_bug.cgi?id=168028
+        <rdar://problem/27607520>
+
+        Reviewed by Brady Eidson.
+
+        WKCustomProtocolLoaders are meant to be owned by CustomProtocolManagerProxy and have their
+        loads cancelled when CustomProtocolManagerProxy is destroyed. However, WKCustomProtocolLoader
+        creates a NSURLConnection for which it is the client, so NSURLConnection retains it for the
+        duration of the load.
+
+        These loaders should be explicitly cancelled when their CustomProtocolManagerProxy is destroyed.
+
+        New API test: WebKit2CustomProtocolsTest.CloseDuringCustomProtocolLoad
+
+        * UIProcess/Network/CustomProtocols/mac/CustomProtocolManagerProxyMac.mm:
+        (-[WKCustomProtocolLoader customProtocolManagerProxyDestroyed]): Added. Cancels the
+        _urlConnection and sets _customProtocolManagerProxy to nullptr.
+        (WebKit::CustomProtocolManagerProxy::~CustomProtocolManagerProxy):
+        Called -customProtocolManagerProxyDestroyed on every loader in m_loaderMap.
+
 2017-02-08  Dan Bernstein  <mitz@apple.com>
 
         [Cocoa] WKRemoteObjectCoder doesn’t handle CGSize
index 979b0d2..24186d3 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -49,6 +49,7 @@ using namespace WebKit;
     NSURLConnection *_urlConnection;
 }
 - (id)initWithCustomProtocolManagerProxy:(CustomProtocolManagerProxy*)customProtocolManagerProxy customProtocolID:(uint64_t)customProtocolID request:(NSURLRequest *)request connection:(Connection *)connection;
+- (void)customProtocolManagerProxyDestroyed;
 @end
 
 @implementation WKCustomProtocolLoader
@@ -84,6 +85,13 @@ using namespace WebKit;
     [super dealloc];
 }
 
+- (void)customProtocolManagerProxyDestroyed
+{
+    ASSERT(_customProtocolManagerProxy);
+    _customProtocolManagerProxy = nullptr;
+    [_urlConnection cancel];
+}
+
 - (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error
 {
     ResourceError coreError(error);
@@ -139,6 +147,8 @@ CustomProtocolManagerProxy::CustomProtocolManagerProxy(ChildProcessProxy* childP
 
 CustomProtocolManagerProxy::~CustomProtocolManagerProxy()
 {
+    for (auto& loader : m_loaderMap)
+        [loader.value customProtocolManagerProxyDestroyed];
     m_childProcessProxy->removeMessageReceiver(Messages::CustomProtocolManagerProxy::messageReceiverName());
 }
 
index 951334b..9d6c91d 100644 (file)
@@ -1,3 +1,18 @@
+2017-02-08  Andy Estes  <aestes@apple.com>
+
+        Custom protocols should not continue loading after a network process crash
+        https://bugs.webkit.org/show_bug.cgi?id=168028
+        <rdar://problem/27607520>
+
+        Reviewed by Brady Eidson.
+
+        * TestWebKitAPI/Tests/WebKit2ObjC/CustomProtocolsTest.mm:
+        (processGroup):
+        (-[CloseWhileStartingProtocol startLoading]):
+        (-[CloseWhileStartingProtocol stopLoading]):
+        (TestWebKitAPI::runTest):
+        (TestWebKitAPI::TEST):
+
 2017-02-08  Dan Bernstein  <mitz@apple.com>
 
         [Cocoa] WKRemoteObjectCoder doesn’t handle CGSize
index 48b4cf5..5e58b2c 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 #import "PlatformUtilities.h"
 #import "TestBrowsingContextLoadDelegate.h"
 #import "TestProtocol.h"
+#import <WebKit/WKContextPrivate.h>
+#import <WebKit/WKProcessGroupPrivate.h>
 #import <wtf/RetainPtr.h>
 
 #if WK_API_ENABLED && PLATFORM(MAC)
 
-static bool testFinished = false;
+static bool testFinished;
 
 @interface CustomProtocolsLoadDelegate : NSObject <WKBrowsingContextLoadDelegate>
 @end
@@ -66,23 +68,70 @@ static bool testFinished = false;
 
 @end
 
-namespace TestWebKitAPI {
+static WKProcessGroup *processGroup()
+{
+    static WKProcessGroup *processGroup = [[WKProcessGroup alloc] init];
+    return processGroup;
+}
 
-TEST(WebKit2CustomProtocolsTest, MainResource)
+@interface CloseWhileStartingProtocol : TestProtocol
+@end
+
+@implementation CloseWhileStartingProtocol
+
+- (void)startLoading
 {
-    [TestProtocol registerWithScheme:@"http"];
+    dispatch_async(dispatch_get_main_queue(), ^ {
+        WKContextClientV0 client;
+        memset(&client, 0, sizeof(client));
+        client.base.clientInfo = self;
+        client.networkProcessDidCrash = [](WKContextRef context, const void* clientInfo) {
+            auto protocol = (CloseWhileStartingProtocol *)clientInfo;
+            [protocol.client URLProtocol:protocol didFailWithError:[NSError errorWithDomain:NSCocoaErrorDomain code:0 userInfo:nil]];
+        };
+        WKContextSetClient([processGroup() _contextRef], &client.base);
+
+        kill(WKContextGetNetworkProcessIdentifier([processGroup() _contextRef]), SIGKILL);
+        [self.client URLProtocol:self didFailWithError:[NSError errorWithDomain:NSCocoaErrorDomain code:0 userInfo:nil]];
+    });
+}
+
+- (void)stopLoading
+{
+    dispatch_async(dispatch_get_main_queue(), ^ {
+        testFinished = true;
+    });
+}
 
-    RetainPtr<WKProcessGroup> processGroup = adoptNS([[WKProcessGroup alloc] init]);
+@end
+
+namespace TestWebKitAPI {
+
+static void runTest()
+{
     RetainPtr<WKBrowsingContextGroup> browsingContextGroup = adoptNS([[WKBrowsingContextGroup alloc] initWithIdentifier:@"TestIdentifier"]);
-    RetainPtr<WKView> wkView = adoptNS([[WKView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) processGroup:processGroup.get() browsingContextGroup:browsingContextGroup.get()]);
+    RetainPtr<WKView> wkView = adoptNS([[WKView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) processGroup:processGroup() browsingContextGroup:browsingContextGroup.get()]);
     RetainPtr<CustomProtocolsLoadDelegate> loadDelegate = adoptNS([[CustomProtocolsLoadDelegate alloc] init]);
     [wkView browsingContextController].loadDelegate = loadDelegate.get();
     [[wkView browsingContextController] loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"%@://redirect?test", [TestProtocol scheme]]]]];
 
     Util::run(&testFinished);
+}
+
+TEST(WebKit2CustomProtocolsTest, MainResource)
+{
+    [TestProtocol registerWithScheme:@"http"];
+    runTest();
     [TestProtocol unregister];
 }
 
+TEST(WebKit2CustomProtocolsTest, CloseDuringCustomProtocolLoad)
+{
+    [CloseWhileStartingProtocol registerWithScheme:@"http"];
+    runTest();
+    [CloseWhileStartingProtocol unregister];
+}
+
 } // namespace TestWebKitAPI
 
 #endif // WK_API_ENABLED