Attachment 368515 Details for Bug 197393 – Patch

[patch] Patch

bug-197393-20190429165002.patch (text/plain), 6.40 KB, created by Chris Dumez on 2019-04-29 16:50:03 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-04-29 16:50:03 PDT

Size: 6.40 KB

patch

obsolete

>Subversion Revision: 244755
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 9cfc789a2682e59163ea898ba7e4cafc337821ab..43911e639181dc01e9d16f9165cca4587b454778 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-04-29  Chris Dumez  <cdumez@apple.com>
>+
>+        Only use a related page's process if that page has not been closed yet
>+        https://bugs.webkit.org/show_bug.cgi?id=197393
>+        <rdar://problem/50302423>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We should not attempt to use a related page's process if that related page has already been closed.
>+        Once closed, a page's process is invalid and trying to launch a new process for the closed page
>+        leads to crashes such as the one in the radar.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::launchProcess):
>+        * UIProcess/WebProcessPool.cpp:
>+        (WebKit::WebProcessPool::createWebPage):
>+
> 2019-04-29  Zalan Bujtas  <zalan@apple.com>
> 
>         [iOS] Double-tapping a post to like doesn't work on Instagram.com (needs 'dblclick' event)
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 3d2c9e1707824f058f873d7a91a43529dade7846..323db28d6e9ba134444e8a02c7046bf05ae66438 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -727,7 +727,8 @@ void WebPageProxy::launchProcess(const RegistrableDomain& registrableDomain)
> 
>     auto& processPool = m_process->processPool();
> 
>-    if (auto* relatedPage = m_configuration->relatedPage())
>+    auto* relatedPage = m_configuration->relatedPage();
>+    if (relatedPage && !relatedPage->isClosed())
>         m_process = relatedPage->ensureRunningProcess();
>     else
>         m_process = processPool.processForRegistrableDomain(m_websiteDataStore.get(), this, registrableDomain);
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
>index 28f8c34590fbb04e69f9657a2731f9986a48ff8d..3a6dacc145447d253a55df8348a377cb33be0ec3 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -1210,7 +1210,8 @@ Ref<WebPageProxy> WebProcessPool::createWebPage(PageClient& pageClient, Ref<API:
>     }
> 
>     RefPtr<WebProcessProxy> process;
>-    if (pageConfiguration->relatedPage()) {
>+    auto* relatedPage = pageConfiguration->relatedPage();
>+    if (relatedPage && !relatedPage->isClosed()) {
>         // Sharing processes, e.g. when creating the page via window.open().
>         process = &pageConfiguration->relatedPage()->ensureRunningProcess();
>         // We do not support several WebsiteDataStores sharing a single process.
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 163cfb6536d02b19981573e369ec594711cd6bac..b28976676c09792d46fd0daa9830271064e720ce 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-04-29  Chris Dumez  <cdumez@apple.com>
>+
>+        Only use a related page's process if that page has not been closed yet
>+        https://bugs.webkit.org/show_bug.cgi?id=197393
>+        <rdar://problem/50302423>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+
> 2019-04-29  Youenn Fablet  <youenn@apple.com>
> 
>         Remove spurious GVA printf logging
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index a476fdb0fe7490ecafc3dbaa9b166e0f5683b465..7f5ddea5abca477763d277948fd1e2c7776d06a6 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -4709,6 +4709,66 @@ TEST(ProcessSwap, RelatedWebViewBeforeWebProcessLaunch)
>     EXPECT_EQ(pid1, pid2); // WebViews are related so they should share the same process.
> }
> 
>+TEST(ProcessSwap, ReloadRelatedWebViewAfterCrash)
>+{
>+    auto processPoolConfiguration = psonProcessPoolConfiguration();
>+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
>+
>+    auto webView1Configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    [webView1Configuration setProcessPool:processPool.get()];
>+    auto handler = adoptNS([[PSONScheme alloc] init]);
>+    [webView1Configuration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
>+
>+    auto webView1 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webView1Configuration.get()]);
>+    auto delegate = adoptNS([[TestNavigationDelegate alloc] init]);
>+    __block bool didCrash = false;
>+    [delegate setWebContentProcessDidTerminate:^(WKWebView *view) {
>+        [view reload];
>+        didCrash = true;
>+    }];
>+    [delegate setDidFinishNavigation:^(WKWebView *, WKNavigation *) {
>+        done = true;
>+    }];
>+
>+    [webView1 setNavigationDelegate:delegate.get()];
>+
>+    auto webView2Configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    [webView2Configuration setProcessPool:processPool.get()];
>+    [webView2Configuration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
>+    webView2Configuration.get()._relatedWebView = webView1.get(); // webView2 will be related to webView1 and webView1's URL will be used for process swap decision.
>+    auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webView2Configuration.get()]);
>+    [webView2 setNavigationDelegate:delegate.get()];
>+
>+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]];
>+    [webView1 loadRequest:request];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto pid1 = [webView1 _webProcessIdentifier];
>+
>+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]];
>+    [webView2 loadRequest:request];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto pid2 = [webView2 _webProcessIdentifier];
>+
>+    EXPECT_EQ(pid1, pid2); // WebViews are related so they should share the same process.
>+
>+    [webView1 _close];
>+    webView1 = nullptr;
>+
>+    kill(pid1, 9);
>+
>+    TestWebKitAPI::Util::run(&didCrash);
>+    didCrash = false;
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+}
>+
> TEST(ProcessSwap, TerminatedSuspendedPageProcess)
> {
>     auto processPoolConfiguration = psonProcessPoolConfiguration();

Actions: View | Formatted Diff | Diff

Attachments on bug 197393: 368515