Attachment 347903 Details for Bug 188880 – Removes the assertion

[patch] Removes the assertion

bug-188880-20180822230709.patch (text/plain), 8.96 KB, created by Ryosuke Niwa on 2018-08-22 23:07:10 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-08-22 23:07:10 PDT

Size: 8.96 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 235213)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,20 @@
>+2018-08-22  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Assert in NetworkBlobRegistry::unregisterBlobURL after network process had terminated
>+        https://bugs.webkit.org/show_bug.cgi?id=188880
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Removed the debug assertion. WebContent process might be asking this network process
>+        to unregister a blob registered from another network processs which had since crashed.
>+
>+        We could keep track of which blob had been registered with which network process
>+        in WebContent process and avoid sending IPC to the network process but that's a lot of
>+        house-keeping for virtually no benefit other than not hitting this assertion.
>+
>+        * NetworkProcess/FileAPI/NetworkBlobRegistry.cpp:
>+        (WebKit::NetworkBlobRegistry::unregisterBlobURL):
>+
> 2018-08-22  John Wilander  <wilander@apple.com>
> 
>         The Storage Access API prompt should show the eTLD+1s, not the full host names
>Index: Source/WebKit/NetworkProcess/FileAPI/NetworkBlobRegistry.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/FileAPI/NetworkBlobRegistry.cpp	(revision 235213)
>+++ Source/WebKit/NetworkProcess/FileAPI/NetworkBlobRegistry.cpp	(working copy)
>@@ -122,7 +122,6 @@ void NetworkBlobRegistry::unregisterBlob
> 
>     blobRegistry().unregisterBlobURL(url);
> 
>-    ASSERT(mapIterator->value.contains(url));
>     mapIterator->value.remove(url);
> }
> 
>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 235216)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2018-08-22  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Assert in NetworkBlobRegistry::unregisterBlobURL after network process had terminated
>+        https://bugs.webkit.org/show_bug.cgi?id=188880
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fixed the bug that testRunner's terminateNetworkProcess, terminateServiceWorkerProcess, and terminateStorageProcess
>+        were asynchronously terminating respective processes. Do so synchronously so that we can deterministically
>+        test WebKit's behavior in layout tests.
>+
>+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
>+        (WTR::TestRunner::terminateNetworkProcess):
>+        (WTR::TestRunner::terminateServiceWorkerProcess):
>+        (WTR::TestRunner::terminateStorageProcess):
>+        * WebKitTestRunner/TestInvocation.cpp:
>+        (WTR::TestInvocation::didReceiveMessageFromInjectedBundle):
>+        (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
>+
> 2018-08-22  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [Attachment Support] Support dragging attachment elements out as files on macOS
>Index: Tools/WebKitTestRunner/TestInvocation.cpp
>===================================================================
>--- Tools/WebKitTestRunner/TestInvocation.cpp	(revision 235213)
>+++ Tools/WebKitTestRunner/TestInvocation.cpp	(working copy)
>@@ -735,24 +735,6 @@ void TestInvocation::didReceiveMessageFr
>         return;
>     }
> 
>-    if (WKStringIsEqualToUTF8CString(messageName, "TerminateStorageProcess")) {
>-        ASSERT(!messageBody);
>-        TestController::singleton().terminateStorageProcess();
>-        return;
>-    }
>-
>-    if (WKStringIsEqualToUTF8CString(messageName, "TerminateNetworkProcess")) {
>-        ASSERT(!messageBody);
>-        TestController::singleton().terminateNetworkProcess();
>-        return;
>-    }
>-
>-    if (WKStringIsEqualToUTF8CString(messageName, "TerminateServiceWorkerProcess")) {
>-        ASSERT(!messageBody);
>-        TestController::singleton().terminateServiceWorkerProcess();
>-        return;
>-    }
>-
>     if (WKStringIsEqualToUTF8CString(messageName, "RunUIProcessScript")) {
>         WKDictionaryRef messageBodyDictionary = static_cast<WKDictionaryRef>(messageBody);
>         WKRetainPtr<WKStringRef> scriptKey(AdoptWK, WKStringCreateWithUTF8CString("Script"));
>@@ -1435,6 +1417,24 @@ WKRetainPtr<WKTypeRef> TestInvocation::d
>         return nullptr;
>     }
> 
>+    if (WKStringIsEqualToUTF8CString(messageName, "TerminateStorageProcess")) {
>+        ASSERT(!messageBody);
>+        TestController::singleton().terminateStorageProcess();
>+        return nullptr;
>+    }
>+
>+    if (WKStringIsEqualToUTF8CString(messageName, "TerminateNetworkProcess")) {
>+        ASSERT(!messageBody);
>+        TestController::singleton().terminateNetworkProcess();
>+        return nullptr;
>+    }
>+
>+    if (WKStringIsEqualToUTF8CString(messageName, "TerminateServiceWorkerProcess")) {
>+        ASSERT(!messageBody);
>+        TestController::singleton().terminateServiceWorkerProcess();
>+        return nullptr;
>+    }
>+
>     ASSERT_NOT_REACHED();
>     return nullptr;
> }
>Index: Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
>===================================================================
>--- Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp	(revision 235213)
>+++ Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp	(working copy)
>@@ -1236,19 +1236,19 @@ void TestRunner::setShouldDownloadUndisp
> void TestRunner::terminateNetworkProcess()
> {
>     WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("TerminateNetworkProcess"));
>-    WKBundlePagePostMessage(InjectedBundle::singleton().page()->page(), messageName.get(), nullptr);
>+    WKBundlePagePostSynchronousMessageForTesting(InjectedBundle::singleton().page()->page(), messageName.get(), nullptr, nullptr);
> }
> 
> void TestRunner::terminateServiceWorkerProcess()
> {
>     WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("TerminateServiceWorkerProcess"));
>-    WKBundlePagePostMessage(InjectedBundle::singleton().page()->page(), messageName.get(), nullptr);
>+    WKBundlePagePostSynchronousMessageForTesting(InjectedBundle::singleton().page()->page(), messageName.get(), nullptr, nullptr);
> }
> 
> void TestRunner::terminateStorageProcess()
> {
>     WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("TerminateStorageProcess"));
>-    WKBundlePagePostMessage(InjectedBundle::singleton().page()->page(), messageName.get(), nullptr);
>+    WKBundlePagePostSynchronousMessageForTesting(InjectedBundle::singleton().page()->page(), messageName.get(), nullptr, nullptr);
> }
> 
> static unsigned nextUIScriptCallbackID()
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 235213)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2018-08-22  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Assert in NetworkBlobRegistry::unregisterBlobURL after network process had terminated
>+        https://bugs.webkit.org/show_bug.cgi?id=188880
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added a layout test which demonstrates this debug assertion.
>+
>+        * fast/files/blob-network-process-crash-expected.txt: Added.
>+        * fast/files/blob-network-process-crash.html: Added.
>+
> 2018-08-22  John Wilander  <wilander@apple.com>
> 
>         Further adjustments to http/tests/websocket/connection-refusal-in-frame-resource-load-statistics.html
>Index: LayoutTests/fast/files/blob-network-process-crash-expected.txt
>===================================================================
>--- LayoutTests/fast/files/blob-network-process-crash-expected.txt	(nonexistent)
>+++ LayoutTests/fast/files/blob-network-process-crash-expected.txt	(working copy)
>@@ -0,0 +1,2 @@
>+This tests unregistering a Blob object after the network process to which it was registered crashed.
>+WebKit should not hit a debug assertion in the network process.
>Index: LayoutTests/fast/files/blob-network-process-crash.html
>===================================================================
>--- LayoutTests/fast/files/blob-network-process-crash.html	(nonexistent)
>+++ LayoutTests/fast/files/blob-network-process-crash.html	(working copy)
>@@ -0,0 +1,39 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<p>This tests unregistering a Blob object after the network process to which it was registered crashed.<br>
>+WebKit should not hit a debug assertion in the network process.</p>
>+<script>
>+
>+if (!window.testRunner)
>+    document.write('This test requires testRunner and GCController');
>+else {
>+    testRunner.dumpAsText();
>+    testRunner.waitUntilDone();
>+
>+    let blobs = new Array(100);
>+    for (let i = 0; i < 100; i++) {
>+        blobs[i] = new Array(10);
>+        for (let j = 0; j < 10; j++)
>+            blobs[i][j] = new Blob(["some text"]);
>+    }
>+
>+    testRunner.terminateNetworkProcess();
>+
>+    setTimeout(() => {
>+        const newBlob = new Blob(["some text"]);
>+        for (let i = 0; i < 100; i++) {
>+            for (let j = 0; j < 10; j++)
>+                blobs[i][j] = { };
>+        }
>+        blobs = null;
>+        GCController.collect();
>+        fetch('blob-network-process-crash.html').then(() => {
>+            testRunner.notifyDone();
>+        });
>+    }, 0);
>+}
>+
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 188880: 347902 | 347903 | 347911 | 347913