Title: [241501] branches/safari-607-branch
Revision
241501
Author
[email protected]
Date
2019-02-14 00:34:17 -0800 (Thu, 14 Feb 2019)

Log Message

Cherry-pick r241336. rdar://problem/48065621

    Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
    https://bugs.webkit.org/show_bug.cgi?id=194568
    <rdar://problem/47944490>

    Reviewed by Ryosuke Niwa.

    Source/WebKit:

    When the provisional process crashes, it is unsafe to call ProvisionalPageProxy::cancel() because
    the WebProcessProxy clears its frame map as soon as the process crashes. Calling cancel() after
    that would call WebPageProxy::didFailProvisionalLoadForFrameShared(), which would try to look up
    the frame by ID and MESSAGE_CHECK() that the frame is not null. We would fail this check since
    the frame has been removed from the WebProcessProxy at this point.

    * UIProcess/API/Cocoa/WKWebView.mm:
    (-[WKWebView _provisionalWebProcessIdentifier]):
    * UIProcess/API/Cocoa/WKWebViewPrivate.h:
    * UIProcess/WebPageProxy.cpp:
    (WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
    (WebKit::WebPageProxy::provisionalProcessDidTerminate):

    Tools:

    Add API test coverage.

    * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
    (-[PSONNavigationDelegate webView:didStartProvisionalNavigation:]):

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

Modified Paths

Diff

Modified: branches/safari-607-branch/Source/WebKit/ChangeLog (241500 => 241501)


--- branches/safari-607-branch/Source/WebKit/ChangeLog	2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Source/WebKit/ChangeLog	2019-02-14 08:34:17 UTC (rev 241501)
@@ -1,3 +1,59 @@
+2019-02-13  Babak Shafiei  <[email protected]>
+
+        Cherry-pick r241336. rdar://problem/48065621
+
+    Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
+    https://bugs.webkit.org/show_bug.cgi?id=194568
+    <rdar://problem/47944490>
+    
+    Reviewed by Ryosuke Niwa.
+    
+    Source/WebKit:
+    
+    When the provisional process crashes, it is unsafe to call ProvisionalPageProxy::cancel() because
+    the WebProcessProxy clears its frame map as soon as the process crashes. Calling cancel() after
+    that would call WebPageProxy::didFailProvisionalLoadForFrameShared(), which would try to look up
+    the frame by ID and MESSAGE_CHECK() that the frame is not null. We would fail this check since
+    the frame has been removed from the WebProcessProxy at this point.
+    
+    * UIProcess/API/Cocoa/WKWebView.mm:
+    (-[WKWebView _provisionalWebProcessIdentifier]):
+    * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+    * UIProcess/WebPageProxy.cpp:
+    (WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
+    (WebKit::WebPageProxy::provisionalProcessDidTerminate):
+    
+    Tools:
+    
+    Add API test coverage.
+    
+    * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+    (-[PSONNavigationDelegate webView:didStartProvisionalNavigation:]):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241336 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-02-12  Chris Dumez  <[email protected]>
+
+            Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
+            https://bugs.webkit.org/show_bug.cgi?id=194568
+            <rdar://problem/47944490>
+
+            Reviewed by Ryosuke Niwa.
+
+            When the provisional process crashes, it is unsafe to call ProvisionalPageProxy::cancel() because
+            the WebProcessProxy clears its frame map as soon as the process crashes. Calling cancel() after
+            that would call WebPageProxy::didFailProvisionalLoadForFrameShared(), which would try to look up
+            the frame by ID and MESSAGE_CHECK() that the frame is not null. We would fail this check since
+            the frame has been removed from the WebProcessProxy at this point.
+
+            * UIProcess/API/Cocoa/WKWebView.mm:
+            (-[WKWebView _provisionalWebProcessIdentifier]):
+            * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+            * UIProcess/WebPageProxy.cpp:
+            (WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
+            (WebKit::WebPageProxy::provisionalProcessDidTerminate):
+
 2019-02-13  Alan Coon  <[email protected]>
 
         Apply patch. rdar://problem/40966400

Modified: branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (241500 => 241501)


--- branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2019-02-14 08:34:17 UTC (rev 241501)
@@ -4654,6 +4654,18 @@
     return _page->processIdentifier();
 }
 
+- (pid_t)_provisionalWebProcessIdentifier
+{
+    if (![self _isValid])
+        return 0;
+
+    auto* provisionalPage = _page->provisionalPageProxy();
+    if (!provisionalPage)
+        return 0;
+
+    return provisionalPage->process().processIdentifier();
+}
+
 - (void)_killWebContentProcess
 {
     if (![self _isValid])

Modified: branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h (241500 => 241501)


--- branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h	2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h	2019-02-14 08:34:17 UTC (rev 241501)
@@ -147,6 +147,7 @@
 @property (nonatomic, setter=_setUserContentExtensionsEnabled:) BOOL _userContentExtensionsEnabled WK_API_AVAILABLE(macosx(10.11), ios(9.0));
 
 @property (nonatomic, readonly) pid_t _webProcessIdentifier;
+@property (nonatomic, readonly) pid_t _provisionalWebProcessIdentifier WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 @property (nonatomic, getter=_isEditable, setter=_setEditable:) BOOL _editable WK_API_AVAILABLE(macosx(10.11), ios(9.0));
 

Modified: branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp (241500 => 241501)


--- branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp	2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp	2019-02-14 08:34:17 UTC (rev 241501)
@@ -3938,7 +3938,7 @@
     PageClientProtector protector(pageClient());
 
     WebFrameProxy* frame = process->webFrame(frameID);
-    MESSAGE_CHECK(m_process, frame);
+    MESSAGE_CHECK(process, frame);
 
     if (m_controlledByAutomation) {
         if (auto* automationSession = process->processPool().automationSession())
@@ -6452,7 +6452,6 @@
 void WebPageProxy::provisionalProcessDidTerminate()
 {
     ASSERT(m_provisionalPage);
-    m_provisionalPage->cancel();
     m_provisionalPage = nullptr;
 }
 

Modified: branches/safari-607-branch/Tools/ChangeLog (241500 => 241501)


--- branches/safari-607-branch/Tools/ChangeLog	2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Tools/ChangeLog	2019-02-14 08:34:17 UTC (rev 241501)
@@ -1,3 +1,51 @@
+2019-02-13  Babak Shafiei  <[email protected]>
+
+        Cherry-pick r241336. rdar://problem/48065621
+
+    Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
+    https://bugs.webkit.org/show_bug.cgi?id=194568
+    <rdar://problem/47944490>
+    
+    Reviewed by Ryosuke Niwa.
+    
+    Source/WebKit:
+    
+    When the provisional process crashes, it is unsafe to call ProvisionalPageProxy::cancel() because
+    the WebProcessProxy clears its frame map as soon as the process crashes. Calling cancel() after
+    that would call WebPageProxy::didFailProvisionalLoadForFrameShared(), which would try to look up
+    the frame by ID and MESSAGE_CHECK() that the frame is not null. We would fail this check since
+    the frame has been removed from the WebProcessProxy at this point.
+    
+    * UIProcess/API/Cocoa/WKWebView.mm:
+    (-[WKWebView _provisionalWebProcessIdentifier]):
+    * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+    * UIProcess/WebPageProxy.cpp:
+    (WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
+    (WebKit::WebPageProxy::provisionalProcessDidTerminate):
+    
+    Tools:
+    
+    Add API test coverage.
+    
+    * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+    (-[PSONNavigationDelegate webView:didStartProvisionalNavigation:]):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241336 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-02-12  Chris Dumez  <[email protected]>
+
+            Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
+            https://bugs.webkit.org/show_bug.cgi?id=194568
+            <rdar://problem/47944490>
+
+            Reviewed by Ryosuke Niwa.
+
+            Add API test coverage.
+
+            * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+            (-[PSONNavigationDelegate webView:didStartProvisionalNavigation:]):
+
 2019-02-13  Alan Coon  <[email protected]>
 
         Cherry-pick r241288. rdar://problem/47992210

Modified: branches/safari-607-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (241500 => 241501)


--- branches/safari-607-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2019-02-14 06:47:32 UTC (rev 241500)
+++ branches/safari-607-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2019-02-14 08:34:17 UTC (rev 241501)
@@ -98,6 +98,7 @@
 
 @interface PSONNavigationDelegate : NSObject <WKNavigationDelegate> {
     @public void (^decidePolicyForNavigationAction)(WKNavigationAction *, void (^)(WKNavigationActionPolicy));
+    @public void (^didStartProvisionalNavigationHandler)();
     @public void (^didCommitNavigationHandler)();
 }
 @end
@@ -125,6 +126,8 @@
 - (void)webView:(WKWebView *)webView didStartProvisionalNavigation:(null_unspecified WKNavigation *)navigation
 {
     didStartProvisionalLoad = true;
+    if (didStartProvisionalNavigationHandler)
+        didStartProvisionalNavigationHandler();
 }
 
 - (void)webView:(WKWebView *)webView didCommitNavigation:(null_unspecified WKNavigation *)navigation
@@ -1488,6 +1491,40 @@
     EXPECT_WK_STREQ(@"pson://www.webkit.org/main1.html", [[webView URL] absoluteString]);
 }
 
+TEST(ProcessSwap, TerminateProcessRightAfterSwap)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+    [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()];
+
+    [webView configuration].preferences.safeBrowsingEnabled = NO;
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    delegate->didStartProvisionalNavigationHandler = ^{
+        EXPECT_NE(0, [webView _provisionalWebProcessIdentifier]);
+        kill([webView _provisionalWebProcessIdentifier], 9);
+    };
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::sleep(0.5);
+}
+
 static const char* linkToWebKitBytes = R"PSONRESOURCE(
 <body>
   <a id="testLink" href=""
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to