Attachment 361871 Details for Bug 194568 – Patch

[patch] Patch

bug-194568-20190212170625.patch (text/plain), 7.66 KB, created by Chris Dumez on 2019-02-12 17:06:26 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-02-12 17:06:26 PST

Size: 7.66 KB

patch

obsolete

>Subversion Revision: 241276
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 2a4168d1ff2b64eddeaa71838be97866be3cdc62..423efb2698177e2a8a1c057f8b5bec0dc4bb1b52 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-02-12  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
>+        https://bugs.webkit.org/show_bug.cgi?id=194568
>+        <rdar://problem/47944490>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-11  Alex Christensen  <achristensen@webkit.org>
> 
>         Fix internal iOS build after r241272
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index 26d8ca051bc4822e3cde36e35c9fa6b559287231..c8dc20d4816767fcfdc80fba759c386d974930de 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -4768,6 +4768,18 @@ - (pid_t)_webProcessIdentifier
>     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])
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
>index 12b01e85d193601df1a2ba3e8411e4815cf7e477..185109962b92664c762241bb482f493ed61faf8b 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
>@@ -154,6 +154,7 @@ typedef NS_OPTIONS(NSUInteger, _WKRectEdge) {
> @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));
> 
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 6a177453fdcd25a99eeca01e799c5c5e24834b3c..c0b3ce7650ce18c70c6ef6e8544588ce1d3b24de 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -3936,7 +3936,7 @@ void WebPageProxy::didFailProvisionalLoadForFrameShared(Ref<WebProcessProxy>&& p
>     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())
>@@ -6503,7 +6503,6 @@ void WebPageProxy::processDidTerminate(ProcessTerminationReason reason)
> void WebPageProxy::provisionalProcessDidTerminate()
> {
>     ASSERT(m_provisionalPage);
>-    m_provisionalPage->cancel();
>     m_provisionalPage = nullptr;
> }
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index a16f2c2e929b6d009f942f93719e66f92e3fed5a..0640dfa305cbb5d573c52e2177f6021e587c0bde 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-02-12  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(PSON) MESSAGE_CHECK() hit under WebPageProxy::didFailProvisionalLoadForFrameShared()
>+        https://bugs.webkit.org/show_bug.cgi?id=194568
>+        <rdar://problem/47944490>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+        (-[PSONNavigationDelegate webView:didStartProvisionalNavigation:]):
>+
> 2019-02-11  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         fast/forms/ios/force-gregorian-calendar-for-credit-card-expiry.html does not work on iPad
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index 71cff6064d8c5b697a4b17f3ad8be0284b4b2028..67f88a2043241cd65aa6a18a214a817dde67ee65 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -98,6 +98,7 @@ @end
> 
> @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 didFinishNavigation:(WKNavigation *)navigat
> - (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 @@ TEST(ProcessSwap, ServerRedirect2)
>     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="pson://www.webkit.org/main.html">Link</a>

Actions: View | Formatted Diff | Diff

Attachments on bug 194568: 361871