- Revision
- 235243
- Author
- [email protected]
- Date
- 2018-08-23 13:51:19 -0700 (Thu, 23 Aug 2018)
Log Message
Assert in NetworkBlobRegistry::unregisterBlobURL after network process had terminated
https://bugs.webkit.org/show_bug.cgi?id=188880
Reviewed by Saam Barati.
Source/WebKit:
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):
Tools:
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):
LayoutTests:
Added a layout test which demonstrates this debug assertion.
* TestExpectations:
* fast/files/blob-network-process-crash-expected.txt: Added.
* fast/files/blob-network-process-crash.html: Added.
* platform/wk2/TestExpectations:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (235242 => 235243)
--- trunk/LayoutTests/ChangeLog 2018-08-23 20:20:15 UTC (rev 235242)
+++ trunk/LayoutTests/ChangeLog 2018-08-23 20:51:19 UTC (rev 235243)
@@ -1,3 +1,17 @@
+2018-08-22 Ryosuke Niwa <[email protected]>
+
+ Assert in NetworkBlobRegistry::unregisterBlobURL after network process had terminated
+ https://bugs.webkit.org/show_bug.cgi?id=188880
+
+ Reviewed by Saam Barati.
+
+ Added a layout test which demonstrates this debug assertion.
+
+ * TestExpectations:
+ * fast/files/blob-network-process-crash-expected.txt: Added.
+ * fast/files/blob-network-process-crash.html: Added.
+ * platform/wk2/TestExpectations:
+
2018-08-23 Youenn Fablet <[email protected]>
Update libwebrtc up to 984f1a80c0
Modified: trunk/LayoutTests/TestExpectations (235242 => 235243)
--- trunk/LayoutTests/TestExpectations 2018-08-23 20:20:15 UTC (rev 235242)
+++ trunk/LayoutTests/TestExpectations 2018-08-23 20:51:19 UTC (rev 235243)
@@ -389,6 +389,9 @@
# Only supported in WebKit2.
http/wpt/cross-origin-resource-policy/ [ Skip ]
+# testRunner.terminateNetworkProcess() only works in WebKit2
+fast/files/blob-network-process-crash.html [ Skip ]
+
#//////////////////////////////////////////////////////////////////////////////////////////
# End platform-specific tests.
#//////////////////////////////////////////////////////////////////////////////////////////
Added: trunk/LayoutTests/fast/files/blob-network-process-crash-expected.txt (0 => 235243)
--- trunk/LayoutTests/fast/files/blob-network-process-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/files/blob-network-process-crash-expected.txt 2018-08-23 20:51:19 UTC (rev 235243)
@@ -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.
Added: trunk/LayoutTests/fast/files/blob-network-process-crash.html (0 => 235243)
--- trunk/LayoutTests/fast/files/blob-network-process-crash.html (rev 0)
+++ trunk/LayoutTests/fast/files/blob-network-process-crash.html 2018-08-23 20:51:19 UTC (rev 235243)
@@ -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>
Modified: trunk/LayoutTests/platform/wk2/TestExpectations (235242 => 235243)
--- trunk/LayoutTests/platform/wk2/TestExpectations 2018-08-23 20:20:15 UTC (rev 235242)
+++ trunk/LayoutTests/platform/wk2/TestExpectations 2018-08-23 20:51:19 UTC (rev 235243)
@@ -740,6 +740,8 @@
http/tests/navigation/useragent-reload.php [ Pass ]
webkit.org/b/187931 storage/indexeddb/modern/opendatabase-after-storage-crash.html [ Skip ]
+# testRunner.terminateNetworkProcess() only works in WebKit2
+fast/files/blob-network-process-crash.html [ Pass ]
### END OF (5) Progressions, expected successes that are expected failures in WebKit1.
########################################
Modified: trunk/Source/WebKit/ChangeLog (235242 => 235243)
--- trunk/Source/WebKit/ChangeLog 2018-08-23 20:20:15 UTC (rev 235242)
+++ trunk/Source/WebKit/ChangeLog 2018-08-23 20:51:19 UTC (rev 235243)
@@ -1,3 +1,20 @@
+2018-08-22 Ryosuke Niwa <[email protected]>
+
+ Assert in NetworkBlobRegistry::unregisterBlobURL after network process had terminated
+ https://bugs.webkit.org/show_bug.cgi?id=188880
+
+ Reviewed by Saam Barati.
+
+ 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-23 Sihui Liu <[email protected]>
Remove keys of defaults that are no longer used in webProcessPool
Modified: trunk/Source/WebKit/NetworkProcess/FileAPI/NetworkBlobRegistry.cpp (235242 => 235243)
--- trunk/Source/WebKit/NetworkProcess/FileAPI/NetworkBlobRegistry.cpp 2018-08-23 20:20:15 UTC (rev 235242)
+++ trunk/Source/WebKit/NetworkProcess/FileAPI/NetworkBlobRegistry.cpp 2018-08-23 20:51:19 UTC (rev 235243)
@@ -122,7 +122,6 @@
blobRegistry().unregisterBlobURL(url);
- ASSERT(mapIterator->value.contains(url));
mapIterator->value.remove(url);
}
Modified: trunk/Tools/ChangeLog (235242 => 235243)
--- trunk/Tools/ChangeLog 2018-08-23 20:20:15 UTC (rev 235242)
+++ trunk/Tools/ChangeLog 2018-08-23 20:51:19 UTC (rev 235243)
@@ -1,3 +1,22 @@
+2018-08-22 Ryosuke Niwa <[email protected]>
+
+ Assert in NetworkBlobRegistry::unregisterBlobURL after network process had terminated
+ https://bugs.webkit.org/show_bug.cgi?id=188880
+
+ Reviewed by Saam Barati.
+
+ 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-23 Myles C. Maxfield <[email protected]>
[WHLSL] Allow native types to have type arguments (like "vector<float, 4>")
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp (235242 => 235243)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp 2018-08-23 20:20:15 UTC (rev 235242)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp 2018-08-23 20:51:19 UTC (rev 235243)
@@ -1236,19 +1236,19 @@
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()
Modified: trunk/Tools/WebKitTestRunner/TestInvocation.cpp (235242 => 235243)
--- trunk/Tools/WebKitTestRunner/TestInvocation.cpp 2018-08-23 20:20:15 UTC (rev 235242)
+++ trunk/Tools/WebKitTestRunner/TestInvocation.cpp 2018-08-23 20:51:19 UTC (rev 235243)
@@ -735,24 +735,6 @@
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 @@
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;
}
