Title: [286609] trunk
Revision
286609
Author
[email protected]
Date
2021-12-07 12:42:25 -0800 (Tue, 07 Dec 2021)

Log Message

ASSERTION FAILED: m_messagesBeingDispatched.isEmpty() on http/tests/resourceLoadStatistics/website-data-removal-for-site-with-user-interaction.html
https://bugs.webkit.org/show_bug.cgi?id=228164
<rdar://problem/80914914>

Reviewed by Darin Adler.

Source/WebKit:

dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection() has an assertion to make sure that m_messagesBeingDispatched
was empty when getting called. However, in the case where there is a nested run loop, the call to
`m_messagesBeingDispatched.takeFirst().dispatch()` at the end of the function may cause the function to re-enter. When entering,
m_messagesBeingDispatched may not be empty so the assertion is wrong. Looking at the code, I think dropping the assertion is the
right thing to do as the implementation seems to be doing something sane upon re-entering. It will just append new messages to
m_messagesBeingDispatched and then try and dispatch them. Based on how it is implemented, message ordering would be preserved.

No new tests, unskipped existing test.

* Platform/IPC/Connection.cpp:
(IPC::Connection::SyncMessageState::dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection):

LayoutTests:

Unskip test that should no longer be crashing.

* platform/ios-wk2/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (286608 => 286609)


--- trunk/LayoutTests/ChangeLog	2021-12-07 20:40:06 UTC (rev 286608)
+++ trunk/LayoutTests/ChangeLog	2021-12-07 20:42:25 UTC (rev 286609)
@@ -1,3 +1,15 @@
+2021-12-07  Chris Dumez  <[email protected]>
+
+        ASSERTION FAILED: m_messagesBeingDispatched.isEmpty() on http/tests/resourceLoadStatistics/website-data-removal-for-site-with-user-interaction.html
+        https://bugs.webkit.org/show_bug.cgi?id=228164
+        <rdar://problem/80914914>
+
+        Reviewed by Darin Adler.
+
+        Unskip test that should no longer be crashing.
+
+        * platform/ios-wk2/TestExpectations:
+
 2021-12-07  Kyle Piddington  <[email protected]>
 
         Roll ANGLE to include upstreamed Metal backend

Modified: trunk/LayoutTests/platform/ios-wk2/TestExpectations (286608 => 286609)


--- trunk/LayoutTests/platform/ios-wk2/TestExpectations	2021-12-07 20:40:06 UTC (rev 286608)
+++ trunk/LayoutTests/platform/ios-wk2/TestExpectations	2021-12-07 20:42:25 UTC (rev 286609)
@@ -2022,8 +2022,6 @@
 
 http/tests/resourceLoadStatistics/exemptDomains/ [ Pass ]
 
-webkit.org/b/228164 [ Debug ] http/tests/resourceLoadStatistics/website-data-removal-for-site-with-user-interaction.html [ Pass Crash ]
-
 webkit.org/b/216492 [ Debug ] imported/w3c/web-platform-tests/selection/extend-20.html [ Slow ]
 
 webkit.org/b/231299 [ Debug ] imported/w3c/web-platform-tests/selection/extend-00.html [ Slow ]

Modified: trunk/Source/WebKit/ChangeLog (286608 => 286609)


--- trunk/Source/WebKit/ChangeLog	2021-12-07 20:40:06 UTC (rev 286608)
+++ trunk/Source/WebKit/ChangeLog	2021-12-07 20:42:25 UTC (rev 286609)
@@ -1,5 +1,25 @@
 2021-12-07  Chris Dumez  <[email protected]>
 
+        ASSERTION FAILED: m_messagesBeingDispatched.isEmpty() on http/tests/resourceLoadStatistics/website-data-removal-for-site-with-user-interaction.html
+        https://bugs.webkit.org/show_bug.cgi?id=228164
+        <rdar://problem/80914914>
+
+        Reviewed by Darin Adler.
+
+        dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection() has an assertion to make sure that m_messagesBeingDispatched
+        was empty when getting called. However, in the case where there is a nested run loop, the call to
+        `m_messagesBeingDispatched.takeFirst().dispatch()` at the end of the function may cause the function to re-enter. When entering,
+        m_messagesBeingDispatched may not be empty so the assertion is wrong. Looking at the code, I think dropping the assertion is the
+        right thing to do as the implementation seems to be doing something sane upon re-entering. It will just append new messages to
+        m_messagesBeingDispatched and then try and dispatch them. Based on how it is implemented, message ordering would be preserved.
+
+        No new tests, unskipped existing test.
+
+        * Platform/IPC/Connection.cpp:
+        (IPC::Connection::SyncMessageState::dispatchMessagesAndResetDidScheduleDispatchMessagesForConnection):
+
+2021-12-07  Chris Dumez  <[email protected]>
+
         Make WebLockRegistryProxy::processDidExit() return early if process never registered any locks
         https://bugs.webkit.org/show_bug.cgi?id=233832
 

Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (286608 => 286609)


--- trunk/Source/WebKit/Platform/IPC/Connection.cpp	2021-12-07 20:40:06 UTC (rev 286608)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp	2021-12-07 20:42:25 UTC (rev 286609)
@@ -226,7 +226,6 @@
         Locker locker { m_lock };
         ASSERT(m_didScheduleDispatchMessagesWorkSet.contains(&connection));
         m_didScheduleDispatchMessagesWorkSet.remove(&connection);
-        ASSERT(m_messagesBeingDispatched.isEmpty());
         Deque<ConnectionAndIncomingMessage> messagesToPutBack;
         for (auto& connectionAndIncomingMessage : m_messagesToDispatchWhileWaitingForSyncReply) {
             if (&connection == connectionAndIncomingMessage.connection.ptr())
@@ -238,7 +237,7 @@
     }
 
     while (!m_messagesBeingDispatched.isEmpty())
-        m_messagesBeingDispatched.takeFirst().dispatch();
+        m_messagesBeingDispatched.takeFirst().dispatch(); // This may cause the function to re-enter when there is a nested run loop.
 }
 
 // Represents a sync request for which we're waiting on a reply.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to