Attachment 339063 Details for Bug 183348 – Updated patch using platform ifdefs

[patch] Updated patch using platform ifdefs

wk2-init-crash.diff (text/plain), 6.07 KB, created by Carlos Garcia Campos on 2018-04-28 03:07:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Carlos Garcia Campos

Created: 2018-04-28 03:07:02 PDT

Size: 6.07 KB

patch

obsolete

>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 542de02e0c4..170b855b233 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-04-28  Carlos Garcia Campos  <cgarcia@igalia.com>
>+
>+        REGRESSION(r222772): [GTK][WPE] WebProcess from WebKitGtk+ 2.19.9x SIGSEVs in WebKit::WebProcess::ensureNetworkProcessConnection() at Source/WebKit/WebProcess/WebProcess.cpp:1127
>+        https://bugs.webkit.org/show_bug.cgi?id=183348
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When connection doesn't exit in case of sync message failure, always exit in case of failing to send
>+        GetNetworkProcessConnection or GetStorageProcessConnection messages. This can happen when the WebView is created
>+        and destroyed quickly.
>+
>+        * WebProcess/WebProcess.cpp:
>+        (WebKit::WebProcess::ensureNetworkProcessConnection):
>+        (WebKit::WebProcess::ensureWebToStorageProcessConnection):
>+
> 2018-04-28  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
>         [GTK] WebProcess from WebKitGtk+ 2.19.92 SIGSEVs in WebCore::TextureMapperGL::~TextureMapperGL
>diff --git a/Source/WebKit/WebProcess/WebProcess.cpp b/Source/WebKit/WebProcess/WebProcess.cpp
>index 239705db6d9..b8e62a8d187 100644
>--- a/Source/WebKit/WebProcess/WebProcess.cpp
>+++ b/Source/WebKit/WebProcess/WebProcess.cpp
>@@ -1148,8 +1148,18 @@ NetworkProcessConnection& WebProcess::ensureNetworkProcessConnection()
>     if (!m_networkProcessConnection) {
>         IPC::Attachment encodedConnectionIdentifier;
> 
>-        if (!parentProcessConnection()->sendSync(Messages::WebProcessProxy::GetNetworkProcessConnection(), Messages::WebProcessProxy::GetNetworkProcessConnection::Reply(encodedConnectionIdentifier), 0, Seconds::infinity(), IPC::SendSyncOption::DoNotProcessIncomingMessagesWhenWaitingForSyncReply))
>+        if (!parentProcessConnection()->sendSync(Messages::WebProcessProxy::GetNetworkProcessConnection(), Messages::WebProcessProxy::GetNetworkProcessConnection::Reply(encodedConnectionIdentifier), 0, Seconds::infinity(), IPC::SendSyncOption::DoNotProcessIncomingMessagesWhenWaitingForSyncReply)) {
>+#if PLATFORM(GTK) || PLATFORM(WPE)
>+            // GTK+ and WPE ports don't exit on send sync message failure.
>+            // In this particular case, the network process can be terminated by the UI process while the
>+            // Web process is still initializing, so we always want to exit instead of crashing. This can
>+            // happen when the WebView is created and then destroyed quickly.
>+            // See https://bugs.webkit.org/show_bug.cgi?id=183348.
>+            exit(0);
>+#else
>             CRASH();
>+#endif
>+        }
> 
> #if USE(UNIX_DOMAIN_SOCKETS)
>         IPC::Connection::Identifier connectionIdentifier = encodedConnectionIdentifier.releaseFileDescriptor();
>@@ -1222,8 +1232,16 @@ WebToStorageProcessConnection& WebProcess::ensureWebToStorageProcessConnection(P
>     if (!m_webToStorageProcessConnection) {
>         IPC::Attachment encodedConnectionIdentifier;
> 
>-        if (!parentProcessConnection()->sendSync(Messages::WebProcessProxy::GetStorageProcessConnection(initialSessionID), Messages::WebProcessProxy::GetStorageProcessConnection::Reply(encodedConnectionIdentifier), 0))
>+        if (!parentProcessConnection()->sendSync(Messages::WebProcessProxy::GetStorageProcessConnection(initialSessionID), Messages::WebProcessProxy::GetStorageProcessConnection::Reply(encodedConnectionIdentifier), 0)) {
>+#if PLATFORM(GTK) || PLATFORM(WPE)
>+            // GTK+ and WPE ports don't exit on send sync message failure.
>+            // In this particular case, the storage process can be terminated by the UI process while the
>+            // connection is being done, so we always want to exit instead of crashing.
>+            // See https://bugs.webkit.org/show_bug.cgi?id=183348.
>+#else
>             CRASH();
>+#endif
>+        }
> 
> #if USE(UNIX_DOMAIN_SOCKETS)
>         IPC::Connection::Identifier connectionIdentifier = encodedConnectionIdentifier.releaseFileDescriptor();
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 7746fa037a6..b3fde790294 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-04-28  Carlos Garcia Campos  <cgarcia@igalia.com>
>+
>+        REGRESSION(r222772): [GTK][WPE] WebProcess from WebKitGtk+ 2.19.9x SIGSEVs in WebKit::WebProcess::ensureNetworkProcessConnection() at Source/WebKit/WebProcess/WebProcess.cpp:1127
>+        https://bugs.webkit.org/show_bug.cgi?id=183348
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a test case to reproduce the crash.
>+
>+        * TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebView.cpp:
>+        (testWebViewCloseQuickly):
>+        (beforeAll):
>+
> 2018-04-27  Stephan Szabo  <stephan.szabo@sony.com>
> 
>         [WinCairo] Support --no-copy for jsc tests in wincairo tests, add copying of dlls for copy case
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebView.cpp b/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebView.cpp
>index 823c0adf4a5..414a81c607b 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebView.cpp
>+++ b/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebView.cpp
>@@ -105,6 +105,18 @@ static void testWebViewWebContextLifetime(WebViewTest* test, gconstpointer)
>     g_object_unref(webContext2);
> }
> 
>+static void testWebViewCloseQuickly(WebViewTest* test, gconstpointer)
>+{
>+    auto webView = Test::adoptView(Test::createWebView());
>+    test->assertObjectIsDeletedWhenTestFinishes(G_OBJECT(webView.get()));
>+    g_idle_add([](gpointer userData) -> gboolean {
>+        static_cast<WebViewTest*>(userData)->quitMainLoop();
>+        return G_SOURCE_REMOVE;
>+    }, test);
>+    g_main_loop_run(test->m_mainLoop);
>+    webView = nullptr;
>+}
>+
> #if PLATFORM(WPE)
> static void testWebViewWebBackend(Test* test, gconstpointer)
> {
>@@ -1203,6 +1215,7 @@ void beforeAll()
> 
>     WebViewTest::add("WebKitWebView", "web-context", testWebViewWebContext);
>     WebViewTest::add("WebKitWebView", "web-context-lifetime", testWebViewWebContextLifetime);
>+    WebViewTest::add("WebKitWebView", "close-quickly", testWebViewCloseQuickly);
> #if PLATFORM(WPE)
>     Test::add("WebKitWebView", "backend", testWebViewWebBackend);
> #endif

Flags:

mcatanzaro: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 183348: 335030 | 337480 | 337489 | 339063