Title: [236420] trunk
- Revision
- 236420
- Author
- cdu...@apple.com
- Date
- 2018-09-24 12:44:08 -0700 (Mon, 24 Sep 2018)
Log Message
Do not do early processing of incoming sync IPC unless we're waiting for a sync IPC reply
https://bugs.webkit.org/show_bug.cgi?id=186941
Reviewed by Alex Christensen.
Source/WebKit:
The comment was claiming we were processing incoming sync messages while waiting for a
sync IPC reply to prevent deadlocks. However, the code was failing to check if we were
waiting for a sync IPC reply. As a result, incoming sync IPC messages would get processed
early no matter what, jumping the line. This was the source of the flakiness in the blob
tests since the IPC to register the blob in the network process was async and the follow-up
IPC to ask the network process for the blob size was sync. The sync message to get the blob
size would jump the line and get processed before the async message to register the blob.
As a result, the network process would not know about the blob yet and return size 0. Of
course, this could happen if the network process was sending sync IPC at the time. However,
the network process never sends any sync IPC and therefore, should never process incoming
IPC messages out of order.
* Platform/IPC/Connection.cpp:
(IPC::Connection::processIncomingMessage):
LayoutTests:
Add layout test coverage.
* http/tests/misc/blob-size-expected.txt: Added.
* http/tests/misc/blob-size.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (236419 => 236420)
--- trunk/LayoutTests/ChangeLog 2018-09-24 19:42:31 UTC (rev 236419)
+++ trunk/LayoutTests/ChangeLog 2018-09-24 19:44:08 UTC (rev 236420)
@@ -1,5 +1,17 @@
2018-09-24 Chris Dumez <cdu...@apple.com>
+ Do not do early processing of incoming sync IPC unless we're waiting for a sync IPC reply
+ https://bugs.webkit.org/show_bug.cgi?id=186941
+
+ Reviewed by Alex Christensen.
+
+ Add layout test coverage.
+
+ * http/tests/misc/blob-size-expected.txt: Added.
+ * http/tests/misc/blob-size.html: Added.
+
+2018-09-24 Chris Dumez <cdu...@apple.com>
+
Unreviewed, skip imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/document-write/contentType.window.html
This newly imported test flakily times out.
Added: trunk/LayoutTests/http/tests/misc/blob-size-expected.txt (0 => 236420)
--- trunk/LayoutTests/http/tests/misc/blob-size-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/misc/blob-size-expected.txt 2018-09-24 19:44:08 UTC (rev 236420)
@@ -0,0 +1,10 @@
+Tests that constructing a Blob and querying its size right away is not flaky
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Getting the Blob size is not flaky
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/misc/blob-size.html (0 => 236420)
--- trunk/LayoutTests/http/tests/misc/blob-size.html (rev 0)
+++ trunk/LayoutTests/http/tests/misc/blob-size.html 2018-09-24 19:44:08 UTC (rev 236420)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that constructing a Blob and querying its size right away is not flaky");
+
+function runTest() {
+ for (let i = 0; i < 50; i++) {
+ for (let j = 0; j < 200; j++) {
+ fetch('/resources/square100.png', { cache: 'no-store' }).then(function(res) { });
+ }
+ for (let k = 0; k < 5; k++) {
+ if ((new Blob(["aaaa"])).size != 4) {
+ testFailed("Getting the Blob size is flaky");
+ return;
+ }
+ }
+ }
+ testPassed("Getting the Blob size is not flaky");
+}
+
+runTest();
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/win/TestExpectations (236419 => 236420)
--- trunk/LayoutTests/platform/win/TestExpectations 2018-09-24 19:42:31 UTC (rev 236419)
+++ trunk/LayoutTests/platform/win/TestExpectations 2018-09-24 19:44:08 UTC (rev 236420)
@@ -4173,6 +4173,8 @@
webkit.org/b/188948 fast/events/dblclick-event-getModifierState.html [ Failure ]
+webkit.org/b/186941 http/tests/misc/blob-size.html [ Crash ]
+
# Tests failing on EWS after updating to iTunes 12.8
webkit.org/b/189194 http/tests/IndexedDB/collect-IDB-objects.https.html [ Failure ]
webkit.org/b/189194 http/tests/referrer-policy/origin-when-cross-origin/cross-origin-http.https.html [ Failure ]
Modified: trunk/Source/WebKit/ChangeLog (236419 => 236420)
--- trunk/Source/WebKit/ChangeLog 2018-09-24 19:42:31 UTC (rev 236419)
+++ trunk/Source/WebKit/ChangeLog 2018-09-24 19:44:08 UTC (rev 236420)
@@ -1,3 +1,25 @@
+2018-09-24 Chris Dumez <cdu...@apple.com>
+
+ Do not do early processing of incoming sync IPC unless we're waiting for a sync IPC reply
+ https://bugs.webkit.org/show_bug.cgi?id=186941
+
+ Reviewed by Alex Christensen.
+
+ The comment was claiming we were processing incoming sync messages while waiting for a
+ sync IPC reply to prevent deadlocks. However, the code was failing to check if we were
+ waiting for a sync IPC reply. As a result, incoming sync IPC messages would get processed
+ early no matter what, jumping the line. This was the source of the flakiness in the blob
+ tests since the IPC to register the blob in the network process was async and the follow-up
+ IPC to ask the network process for the blob size was sync. The sync message to get the blob
+ size would jump the line and get processed before the async message to register the blob.
+ As a result, the network process would not know about the blob yet and return size 0. Of
+ course, this could happen if the network process was sending sync IPC at the time. However,
+ the network process never sends any sync IPC and therefore, should never process incoming
+ IPC messages out of order.
+
+ * Platform/IPC/Connection.cpp:
+ (IPC::Connection::processIncomingMessage):
+
2018-09-24 Daniel Bates <daba...@apple.com>
[iOS] Key code is 0 for many hardware keyboard keys
Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (236419 => 236420)
--- trunk/Source/WebKit/Platform/IPC/Connection.cpp 2018-09-24 19:42:31 UTC (rev 236419)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp 2018-09-24 19:44:08 UTC (rev 236420)
@@ -719,8 +719,13 @@
// Check if this is a sync message or if it's a message that should be dispatched even when waiting for
// a sync reply. If it is, and we're waiting for a sync reply this message needs to be dispatched.
// If we don't we'll end up with a deadlock where both sync message senders are stuck waiting for a reply.
- if (SyncMessageState::singleton().processIncomingMessage(*this, message))
- return;
+ {
+ LockHolder locker(m_syncReplyStateMutex);
+ // If the m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage flag is set, then our sync messages will not be processed by the destination process while it is waiting for a sync IPC reply.
+ // As a result, we need to process incoming sync messages early here, even if we're not waiting for a sync IPC reply in order to prevent deadlocks.
+ if ((m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage || !m_pendingSyncReplies.isEmpty()) && SyncMessageState::singleton().processIncomingMessage(*this, message))
+ return;
+ }
// Check if we're waiting for this message.
{
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
