Title: [227590] trunk
Revision
227590
Author
achristen...@apple.com
Date
2018-01-24 18:35:59 -0800 (Wed, 24 Jan 2018)

Log Message

Gracefully recover from NetworkProcess crashes in private browsing
https://bugs.webkit.org/show_bug.cgi?id=182073
<rdar://problem/36572023>

Reviewed by Geoff Garen.

Source/WebKit:

If we're using a non-persistent WKWebsiteDataStore and the NetworkProcess crashes and we try to do a load,
then the WebProcess restarts the NetworkProcess but doesn't recreate the ephemeral session in the NetworkProcess.
When this happens, we've already lost the browsing state in memory in the NetworkProcess, but we don't want to hang.
If this is the problem, then just recreate the ephemeral session and continue loading.

* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::removeStorageAccessForFrame):
(WebKit::NetworkConnectionToWebProcess::removeStorageAccessForAllFramesOnPage):
* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::startNetworkLoad):
* Shared/WebsiteDataStoreParameters.cpp:
(WebKit::WebsiteDataStoreParameters::privateSessionParameters):
(WebKit::WebsiteDataStoreParameters::legacyPrivateSessionParameters): Deleted.
* Shared/WebsiteDataStoreParameters.h:
(WebKit::WebsiteDataStoreParameters::legacyPrivateSessionParameters):

Tools:

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKitCocoa/NetworkProcessCrashNonPersistentDataStore.mm: Added.
(-[CrashDelegate webView:didFinishNavigation:]):
(-[CrashDelegate webView:didFailProvisionalNavigation:withError:]):
(-[CrashDelegate webView:didFailNavigation:withError:]):
(TEST):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (227589 => 227590)


--- trunk/Source/WebKit/ChangeLog	2018-01-25 01:28:47 UTC (rev 227589)
+++ trunk/Source/WebKit/ChangeLog	2018-01-25 02:35:59 UTC (rev 227590)
@@ -1,3 +1,27 @@
+2018-01-24  Alex Christensen  <achristen...@webkit.org>
+
+        Gracefully recover from NetworkProcess crashes in private browsing
+        https://bugs.webkit.org/show_bug.cgi?id=182073
+        <rdar://problem/36572023>
+
+        Reviewed by Geoff Garen.
+
+        If we're using a non-persistent WKWebsiteDataStore and the NetworkProcess crashes and we try to do a load,
+        then the WebProcess restarts the NetworkProcess but doesn't recreate the ephemeral session in the NetworkProcess.
+        When this happens, we've already lost the browsing state in memory in the NetworkProcess, but we don't want to hang.
+        If this is the problem, then just recreate the ephemeral session and continue loading.
+
+        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+        (WebKit::NetworkConnectionToWebProcess::removeStorageAccessForFrame):
+        (WebKit::NetworkConnectionToWebProcess::removeStorageAccessForAllFramesOnPage):
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::startNetworkLoad):
+        * Shared/WebsiteDataStoreParameters.cpp:
+        (WebKit::WebsiteDataStoreParameters::privateSessionParameters):
+        (WebKit::WebsiteDataStoreParameters::legacyPrivateSessionParameters): Deleted.
+        * Shared/WebsiteDataStoreParameters.h:
+        (WebKit::WebsiteDataStoreParameters::legacyPrivateSessionParameters):
+
 2018-01-24  Dan Bernstein  <m...@apple.com>
 
         Enable library validation on the Web Content service

Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (227589 => 227590)


--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp	2018-01-25 01:28:47 UTC (rev 227589)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp	2018-01-25 02:35:59 UTC (rev 227590)
@@ -463,7 +463,8 @@
 void NetworkConnectionToWebProcess::removeStorageAccessForFrame(PAL::SessionID sessionID, uint64_t frameID, uint64_t pageID)
 {
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
-    storageSession(sessionID).removeStorageAccessForFrame(frameID, pageID);
+    if (auto* storageSession = NetworkStorageSession::storageSession(sessionID))
+        storageSession->removeStorageAccessForFrame(frameID, pageID);
 #else
     UNUSED_PARAM(sessionID);
     UNUSED_PARAM(frameID);
@@ -474,7 +475,8 @@
 void NetworkConnectionToWebProcess::removeStorageAccessForAllFramesOnPage(PAL::SessionID sessionID, uint64_t pageID)
 {
 #if HAVE(CFNETWORK_STORAGE_PARTITIONING)
-    storageSession(sessionID).removeStorageAccessForAllFramesOnPage(pageID);
+    if (auto* storageSession = NetworkStorageSession::storageSession(sessionID))
+        storageSession->removeStorageAccessForAllFramesOnPage(pageID);
 #else
     UNUSED_PARAM(sessionID);
     UNUSED_PARAM(pageID);

Modified: trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp (227589 => 227590)


--- trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	2018-01-25 01:28:47 UTC (rev 227589)
+++ trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	2018-01-25 02:35:59 UTC (rev 227590)
@@ -38,6 +38,7 @@
 #include "WebCoreArgumentCoders.h"
 #include "WebErrors.h"
 #include "WebResourceLoaderMessages.h"
+#include "WebsiteDataStoreParameters.h"
 #include <WebCore/BlobDataFileReference.h>
 #include <WebCore/CertificateInfo.h>
 #include <WebCore/DiagnosticLoggingKeys.h>
@@ -225,6 +226,10 @@
         parameters.blobFileReferences = NetworkBlobRegistry::singleton().filesInBlob(m_connection, originalRequest().url());
 
     auto* networkSession = SessionTracker::networkSession(parameters.sessionID);
+    if (!networkSession && parameters.sessionID.isEphemeral()) {
+        NetworkProcess::singleton().addWebsiteDataStore(WebsiteDataStoreParameters::privateSessionParameters(parameters.sessionID));
+        networkSession = SessionTracker::networkSession(parameters.sessionID);
+    }
     if (!networkSession) {
         WTFLogAlways("Attempted to create a NetworkLoad with a session (id=%" PRIu64 ") that does not exist.", parameters.sessionID.sessionID());
         RELEASE_LOG_ERROR_IF_ALLOWED("startNetworkLoad: Attempted to create a NetworkLoad with a session that does not exist (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", sessionID=%" PRIu64 ")", m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier, parameters.sessionID.sessionID());

Modified: trunk/Source/WebKit/Shared/WebsiteDataStoreParameters.cpp (227589 => 227590)


--- trunk/Source/WebKit/Shared/WebsiteDataStoreParameters.cpp	2018-01-25 01:28:47 UTC (rev 227589)
+++ trunk/Source/WebKit/Shared/WebsiteDataStoreParameters.cpp	2018-01-25 02:35:59 UTC (rev 227590)
@@ -86,9 +86,10 @@
     return {{ WTFMove(*uiProcessCookieStorageIdentifier), WTFMove(*cookieStoragePathExtensionHandle), WTFMove(*pendingCookies), WTFMove(*cacheStorageDirectory), WTFMove(*cacheStoragePerOriginQuota), WTFMove(*cacheStorageDirectoryExtensionHandle), WTFMove(*networkSessionParameters)}};
 }
 
-WebsiteDataStoreParameters WebsiteDataStoreParameters::legacyPrivateSessionParameters()
+WebsiteDataStoreParameters WebsiteDataStoreParameters::privateSessionParameters(PAL::SessionID sessionID)
 {
-    return { { }, { }, { }, { }, WebsiteDataStore::defaultCacheStoragePerOriginQuota, { }, { PAL::SessionID::legacyPrivateSessionID(), { }, { }, { }}};
+    ASSERT(sessionID.isEphemeral());
+    return { { }, { }, { }, { }, WebsiteDataStore::defaultCacheStoragePerOriginQuota, { }, { sessionID, { }, { }, { }}};
 }
 
 } // namespace WebKit

Modified: trunk/Source/WebKit/Shared/WebsiteDataStoreParameters.h (227589 => 227590)


--- trunk/Source/WebKit/Shared/WebsiteDataStoreParameters.h	2018-01-25 01:28:47 UTC (rev 227589)
+++ trunk/Source/WebKit/Shared/WebsiteDataStoreParameters.h	2018-01-25 02:35:59 UTC (rev 227590)
@@ -44,7 +44,9 @@
     WebsiteDataStoreParameters(WebsiteDataStoreParameters&&) = default;
     WebsiteDataStoreParameters& operator=(WebsiteDataStoreParameters&&) = default;
     ~WebsiteDataStoreParameters();
-    static WebsiteDataStoreParameters legacyPrivateSessionParameters();
+
+    static WebsiteDataStoreParameters legacyPrivateSessionParameters() { return privateSessionParameters(PAL::SessionID::legacyPrivateSessionID()); }
+    static WebsiteDataStoreParameters privateSessionParameters(PAL::SessionID);
     
     void encode(IPC::Encoder&) const;
     static std::optional<WebsiteDataStoreParameters> decode(IPC::Decoder&);

Modified: trunk/Tools/ChangeLog (227589 => 227590)


--- trunk/Tools/ChangeLog	2018-01-25 01:28:47 UTC (rev 227589)
+++ trunk/Tools/ChangeLog	2018-01-25 02:35:59 UTC (rev 227590)
@@ -1,3 +1,18 @@
+2018-01-24  Alex Christensen  <achristen...@webkit.org>
+
+        Gracefully recover from NetworkProcess crashes in private browsing
+        https://bugs.webkit.org/show_bug.cgi?id=182073
+        <rdar://problem/36572023>
+
+        Reviewed by Geoff Garen.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKitCocoa/NetworkProcessCrashNonPersistentDataStore.mm: Added.
+        (-[CrashDelegate webView:didFinishNavigation:]):
+        (-[CrashDelegate webView:didFailProvisionalNavigation:withError:]):
+        (-[CrashDelegate webView:didFailNavigation:withError:]):
+        (TEST):
+
 2018-01-24  Ali Juma  <aj...@chromium.org>
 
         REGRESSION (r227430): ASSERTION FAILED: !self.zoomToScaleCompletionHandler in TestRunnerWKWebView::zoomToScale

Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (227589 => 227590)


--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2018-01-25 01:28:47 UTC (rev 227589)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2018-01-25 02:35:59 UTC (rev 227590)
@@ -259,6 +259,7 @@
 		5C9E59431D3EB5AC00E3C62E /* ApplicationCache.db-wal in Copy Resources */ = {isa = PBXBuildFile; fileRef = 5C9E59401D3EB1DE00E3C62E /* ApplicationCache.db-wal */; };
 		5CA1DEC81F71F70100E71BD3 /* HTTPHeaderField.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5CA1DEC71F71F40700E71BD3 /* HTTPHeaderField.cpp */; };
 		5CA1DED91F74A91A00E71BD3 /* ContentRuleListNotification.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CA1DED81F74A87100E71BD3 /* ContentRuleListNotification.mm */; };
+		5CAE463820193B6A0051610F /* NetworkProcessCrashNonPersistentDataStore.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CAE4637201937CD0051610F /* NetworkProcessCrashNonPersistentDataStore.mm */; };
 		5CB18BA81F5645E300EE23C4 /* ClickAutoFillButton.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CB18BA71F5645B200EE23C4 /* ClickAutoFillButton.mm */; };
 		5CB3CE391FA1697F00C3A2D6 /* WKWebViewConfiguration.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CB3CE381FA1691700C3A2D6 /* WKWebViewConfiguration.mm */; };
 		5CB40B4E1F4B98D3007DC7B9 /* UIDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5CB40B4D1F4B98BE007DC7B9 /* UIDelegate.mm */; };
@@ -1428,6 +1429,7 @@
 		5C9E59401D3EB1DE00E3C62E /* ApplicationCache.db-wal */ = {isa = PBXFileReference; lastKnownFileType = file; path = "ApplicationCache.db-wal"; sourceTree = "<group>"; };
 		5CA1DEC71F71F40700E71BD3 /* HTTPHeaderField.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HTTPHeaderField.cpp; sourceTree = "<group>"; };
 		5CA1DED81F74A87100E71BD3 /* ContentRuleListNotification.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ContentRuleListNotification.mm; sourceTree = "<group>"; };
+		5CAE4637201937CD0051610F /* NetworkProcessCrashNonPersistentDataStore.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = NetworkProcessCrashNonPersistentDataStore.mm; sourceTree = "<group>"; };
 		5CB18BA71F5645B200EE23C4 /* ClickAutoFillButton.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ClickAutoFillButton.mm; sourceTree = "<group>"; };
 		5CB3CE381FA1691700C3A2D6 /* WKWebViewConfiguration.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebViewConfiguration.mm; sourceTree = "<group>"; };
 		5CB40B4D1F4B98BE007DC7B9 /* UIDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = UIDelegate.mm; sourceTree = "<group>"; };
@@ -2110,14 +2112,15 @@
 				7A6A2C6F1DCCF87B00C0D085 /* LocalStorageQuirkTest.mm */,
 				51CD1C6A1B38CE3600142CA5 /* ModalAlerts.mm */,
 				1ABC3DED1899BE6D004F0626 /* Navigation.mm */,
+				5CAE4637201937CD0051610F /* NetworkProcessCrashNonPersistentDataStore.mm */,
 				2ECFF5541D9B12F800B55394 /* NowPlayingControlsTests.mm */,
 				A10F047C1E3AD29C00C95E19 /* NSFileManagerExtras.mm */,
 				37A22AA51DCAA27200AFBFC4 /* ObservedRenderingProgressEventsAfterCrash.mm */,
 				CEA6CF2219CCF5BD0064F5A7 /* OpenAndCloseWindow.mm */,
 				CEBCA12E1E3A660100C73293 /* OverrideContentSecurityPolicy.mm */,
+				9BCB7C2620130600003E7C0C /* PasteHTML.mm */,
 				9BDCCD851F7D0B0700009A18 /* PasteImage.mm */,
 				9BDD95561F83683600D20C60 /* PasteRTFD.mm */,
-				9BCB7C2620130600003E7C0C /* PasteHTML.mm */,
 				9B2346411F943A2400DB1D23 /* PasteWebArchive.mm */,
 				3FCC4FE41EC4E8520076E37C /* PictureInPictureDelegate.mm */,
 				83BAEE8C1EF4625500DDE894 /* PluginLoadClientPolicies.mm */,
@@ -3399,7 +3402,6 @@
 				7CCE7EBB1A411A7E00447C4C /* DOMHTMLTableCellCellAbove.mm in Sources */,
 				2D51A0C71C8BF00C00765C45 /* DOMHTMLVideoElementWrapper.mm in Sources */,
 				46397B951DC2C850009A78AE /* DOMNode.mm in Sources */,
-				9BCB7C2820130600003E7C0C /* PasteHTML.mm in Sources */,
 				7CCE7EBC1A411A7E00447C4C /* DOMNodeFromJSObject.mm in Sources */,
 				7CCE7EBD1A411A7E00447C4C /* DOMRangeOfString.mm in Sources */,
 				7CCE7EEC1A411AE600447C4C /* DOMWindowExtensionBasic.cpp in Sources */,
@@ -3517,6 +3519,7 @@
 				7CCE7F011A411AE600447C4C /* MouseMoveAfterCrash.cpp in Sources */,
 				7CCE7F241A411AF600447C4C /* Navigation.mm in Sources */,
 				5C0BF8951DD599CD00B00328 /* NavigatorLanguage.mm in Sources */,
+				5CAE463820193B6A0051610F /* NetworkProcessCrashNonPersistentDataStore.mm in Sources */,
 				9B19CDA01F06DFE3000548DD /* NetworkProcessCrashWithPendingConnection.mm in Sources */,
 				7CCE7F021A411AE600447C4C /* NewFirstVisuallyNonEmptyLayout.cpp in Sources */,
 				7CCE7F031A411AE600447C4C /* NewFirstVisuallyNonEmptyLayoutFails.cpp in Sources */,
@@ -3535,6 +3538,7 @@
 				7CCE7F091A411AE600447C4C /* ParentFrame.cpp in Sources */,
 				7C83E0511D0A641800FEBCF3 /* ParsedContentRange.cpp in Sources */,
 				7CCE7F0A1A411AE600447C4C /* PasteboardNotifications.mm in Sources */,
+				9BCB7C2820130600003E7C0C /* PasteHTML.mm in Sources */,
 				9BDCCD871F7D0B0700009A18 /* PasteImage.mm in Sources */,
 				9BDD95581F83683600D20C60 /* PasteRTFD.mm in Sources */,
 				9B2346421F943E2700DB1D23 /* PasteWebArchive.mm in Sources */,

Added: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcessCrashNonPersistentDataStore.mm (0 => 227590)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcessCrashNonPersistentDataStore.mm	                        (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcessCrashNonPersistentDataStore.mm	2018-01-25 02:35:59 UTC (rev 227590)
@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2018 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.
+ */
+
+#include "config.h"
+
+#import "PlatformUtilities.h"
+#import "Test.h"
+#import "TestWKWebView.h"
+#import <WebKit/WKProcessPoolPrivate.h>
+#import <wtf/RetainPtr.h>
+
+#if WK_API_ENABLED
+
+static bool done;
+
+@interface CrashDelegate : NSObject <WKNavigationDelegate>
+@end
+
+@implementation CrashDelegate
+
+- (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation
+{
+    done = true;
+}
+
+- (void)webView:(WKWebView *)webView didFailProvisionalNavigation:(WKNavigation *)navigation withError:(NSError *)error
+{
+    EXPECT_TRUE(false);
+}
+
+- (void)webView:(WKWebView *)webView didFailNavigation:(WKNavigation *)navigation withError:(NSError *)error
+{
+    EXPECT_TRUE(false);
+}
+
+@end
+
+TEST(WebKit, NetworkProcessCrashNonPersistentDataStore)
+{
+    NSURL *simple = [[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+    NSURL *simple2 = [[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+    
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [configuration setWebsiteDataStore:[WKWebsiteDataStore nonPersistentDataStore]];
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+    auto delegate = adoptNS([[CrashDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+
+    [webView loadRequest:[NSURLRequest requestWithURL:simple]];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    [[webView configuration].processPool _terminateNetworkProcess];
+    [webView loadRequest:[NSURLRequest requestWithURL:simple2]];
+    TestWebKitAPI::Util::run(&done);
+}
+
+#endif // WK_API_ENABLED
+
+
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to