Changeset 258272 in webkit

Timestamp:

Mar 11, 2020 12:34:01 PM (4 years ago)

Author:

Chris Dumez

Message:

Assertion hit under WebPageProxy::suspendCurrentPageIfPossible() when bypassing certificate warning page
​https://bugs.webkit.org/show_bug.cgi?id=208927
<rdar://problem/55448331>

Reviewed by Alex Christensen.

Source/WebKit:

When suspending a previous page after a process-swap, we refuse to do the suspension if the currently
committed URL does not match the url of the current back/forward list item. This makes sense since we
will store the suspended page on the back/forward list item and restore that back when navigating back
to that item. I had a debug assertion in there to help me figure out how this could happen. It turns
out that calling loadAlternateHTML on the view will replace the content of the view with the provided
HTML string, update the committed URL but not create a new back/forward list item. This is so that
if you reload or go/back to this back/forward list item, we will attempt to load the original URL
again. This would however trip my assertion. I am therefore removing my debug assertion and adding API
test coverage.

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::suspendCurrentPageIfPossible):

Tools:

Add API test coverage.

• TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-03-11  Chris Dumez  <cdumez@apple.com>
+
+        Assertion hit under WebPageProxy::suspendCurrentPageIfPossible() when bypassing certificate warning page
+        https://bugs.webkit.org/show_bug.cgi?id=208927
+        <rdar://problem/55448331>
+
+        Reviewed by Alex Christensen.
+
+        When suspending a previous page after a process-swap, we refuse to do the suspension if the currently
+        committed URL does not match the url of the current back/forward list item. This makes sense since we
+        will store the suspended page on the back/forward list item and restore that back when navigating back
+        to that item. I had a debug assertion in there to help me figure out how this could happen. It turns
+        out that calling loadAlternateHTML on the view will replace the content of the view with the provided
+        HTML string, update the committed URL but not create a new back/forward list item. This is so that
+        if you reload or go/back to this back/forward list item, we will attempt to load the original URL
+        again. This would however trip my assertion. I am therefore removing my debug assertion and adding API
+        test coverage.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::suspendCurrentPageIfPossible):
+
 2020-03-11  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -833 +833 @@
     if (fromItem && fromItem->url() != pageLoadState().url()) {
         RELEASE_LOG_ERROR_IF_ALLOWED(ProcessSwapping, "suspendCurrentPageIfPossible: Not suspending current page for process pid %i because fromItem's URL does not match the page URL.", m_process->processIdentifier());
-        ASSERT_NOT_REACHED();
         return false;
     }

trunk/Tools/ChangeLog

@@ +1 @@
+2020-03-11  Chris Dumez  <cdumez@apple.com>
+
+        Assertion hit under WebPageProxy::suspendCurrentPageIfPossible() when bypassing certificate warning page
+        https://bugs.webkit.org/show_bug.cgi?id=208927
+        <rdar://problem/55448331>
+
+        Reviewed by Alex Christensen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2020-03-11  Diego Pino Garcia  <dpino@igalia.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

@@ -6238 +6238 @@
 }
 
+TEST(ProcessSwap, LoadAlternativeHTML)
+{
+    [[WKContentRuleListStore defaultStore] removeContentRuleListForIdentifier:@"ContentBlockingAfterProcessSwapExtension" completionHandler:^(NSError *error) {
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto processPoolConfiguration = psonProcessPoolConfiguration();
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    webViewConfiguration.get()._allowTopNavigationToDataURLs = YES;
+
+    RetainPtr<PSONMessageHandler> messageHandler = adoptNS([[PSONMessageHandler alloc] init]);
+    [[webViewConfiguration userContentController] addScriptMessageHandler:messageHandler.get() name:@"pson"];
+
+    __block bool doneCompiling = false;
+    [[WKContentRuleListStore defaultStore] compileContentRuleListForIdentifier:@"ContentBlockingAfterProcessSwapExtension" encodedContentRuleList:blockmeFilter completionHandler:^(WKContentRuleList *ruleList, NSError *error) {
+
+        EXPECT_NOT_NULL(ruleList);
+        EXPECT_NULL(error);
+
+        [webViewConfiguration.get().userContentController addContentRuleList:ruleList];
+
+        doneCompiling = true;
+    }];
+    TestWebKitAPI::Util::run(&doneCompiling);
+
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+    [handler addMappingFromURLString:@"pson://www.apple.com/blockme.html" toData:notifyLoadedBytes];
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+    [webView loadRequest:request];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    receivedMessage = false;
+    failed = false;
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/blockme.html"]];
+    [webView loadRequest:request];
+    TestWebKitAPI::Util::run(&failed);
+    failed = false;
+    EXPECT_FALSE(receivedMessage);
+
+    [webView _loadAlternateHTMLString:@"Blocked" baseURL:[NSURL URLWithString:@"foo:blockedWarning.html"] forUnreachableURL:[NSURL URLWithString:@"pson://www.apple.com/blockme.html"]];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+    [webView loadRequest:request];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+}
+
 static bool isCapturing = false;
 @interface GetUserMediaUIDelegate : NSObject<WKUIDelegate>