Changeset 247565 in webkit

Timestamp:

Jul 18, 2019 10:44:54 AM (5 years ago)

Author:

Chris Dumez

Message:

Regression(r247486) Multiple Layout Tests in http/tests/cache/* are crashing on iOS Debug WK2
​https://bugs.webkit.org/show_bug.cgi?id=199892
<rdar://problem/53230217>

Reviewed by Geoffrey Garen.

The StorageManager was unregistering itself as a WorkQueueMessageReceiver on a given IPC
connection too early. As a result, we would sometimes receive a 'StorageManager:DestroyStorageMap'
IPC in NetworkConnectionToWebProcess::didReceiveMessage() with nobody to process it, which would
trigger an assertion. To address the issue, we stop unregistering the StorageManager as a
WorkQueueMessageReceiver inside removeAllowedSessionStorageNamespaceConnection(). Instead, we
let the logic inside processDidCloseConnection() take care of it once the connection closes.

• NetworkProcess/WebStorage/StorageManager.cpp:

(WebKit::StorageManager::removeAllowedSessionStorageNamespaceConnection):
(WebKit::StorageManager::processDidCloseConnection):

• NetworkProcess/WebStorage/StorageManager.h:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/WebStorage/StorageManager.cpp (modified) (2 diffs)
• NetworkProcess/WebStorage/StorageManager.h (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-07-18  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r247486) Multiple Layout Tests in http/tests/cache/* are crashing on iOS Debug WK2
+        https://bugs.webkit.org/show_bug.cgi?id=199892
+        <rdar://problem/53230217>
+
+        Reviewed by Geoffrey Garen.
+
+        The StorageManager was unregistering itself as a WorkQueueMessageReceiver on a given IPC
+        connection too early. As a result, we would sometimes receive a 'StorageManager:DestroyStorageMap'
+        IPC in NetworkConnectionToWebProcess::didReceiveMessage() with nobody to process it, which would
+        trigger an assertion. To address the issue, we stop unregistering the StorageManager as a
+        WorkQueueMessageReceiver inside removeAllowedSessionStorageNamespaceConnection(). Instead, we
+        let the logic inside processDidCloseConnection() take care of it once the connection closes.
+
+        * NetworkProcess/WebStorage/StorageManager.cpp:
+        (WebKit::StorageManager::removeAllowedSessionStorageNamespaceConnection):
+        (WebKit::StorageManager::processDidCloseConnection):
+        * NetworkProcess/WebStorage/StorageManager.h:
+
 2019-07-18  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp

@@ -538 +538 @@
 {
     auto allowedConnectionID = allowedConnection.uniqueID();
-    if (m_connections.remove(allowedConnectionID))
-        allowedConnection.removeWorkQueueMessageReceiver(Messages::StorageManager::messageReceiverName());
-
     m_queue->dispatch([this, protectedThis = makeRef(*this), allowedConnectionID, storageNamespaceID]() mutable {
         ASSERT(m_sessionStorageNamespaces.contains(storageNamespaceID));
@@ -575 +572 @@
 void StorageManager::processDidCloseConnection(IPC::Connection& connection)
 {
-    if (m_connections.removeAll(connection.uniqueID()))
+    if (m_connections.remove(connection.uniqueID()))
         connection.removeWorkQueueMessageReceiver(Messages::StorageManager::messageReceiverName());
 

trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h

@@ -32 +32 @@
 #include <wtf/Forward.h>
 #include <wtf/Function.h>
-#include <wtf/HashCountedSet.h>
 #include <wtf/HashSet.h>
 #include <wtf/text/StringHash.h>
@@ -114 +113 @@
 
     HashMap<std::pair<IPC::Connection::UniqueID, uint64_t>, RefPtr<StorageArea>> m_storageAreasByConnection;
-    HashCountedSet<IPC::Connection::UniqueID> m_connections;
+    HashSet<IPC::Connection::UniqueID> m_connections;
 
     enum class State {