Skip to site navigation (Press enter)

[webkit-changes] [234664] trunk/Source/WebKit

achristensen Tue, 07 Aug 2018 12:09:29 -0700

Title: [234664] trunk/Source/WebKit

Revision: 234664
Author: achristen...@apple.com
Date: 2018-08-07 12:09:16 -0700 (Tue, 07 Aug 2018)

Log Message

StorageManager should stop ref'ing IPC::Connections as this is leak-prone
https://bugs.webkit.org/show_bug.cgi?id=188380


Patch by Chris Dumez <cdu...@apple.com> on 2018-08-07
Reviewed by Alex Christensen.

StorageManager should stop ref'ing IPC::Connections as this is leak-prone. Instead, assign a unique identifier
to each IPC::Connection and store this identifier intead of a RefPtr<IPC::Connection>. When the StorageManager
needs an actual IPC::Connection, it can look it up from the identifier.

* Platform/IPC/Connection.cpp:
(IPC::Connection::Connection):
(IPC::Connection::~Connection):
(IPC::Connection::connection):
* Platform/IPC/Connection.h:
(IPC::Connection::uniqueID const):
* UIProcess/WebStorage/StorageManager.cpp:
(WebKit::StorageManager::StorageArea::addListener):
(WebKit::StorageManager::StorageArea::removeListener):
(WebKit::StorageManager::StorageArea::hasListener const):
(WebKit::StorageManager::StorageArea::setItem):
(WebKit::StorageManager::StorageArea::removeItem):
(WebKit::StorageManager::StorageArea::clear):
(WebKit::StorageManager::StorageArea::dispatchEvents const):
(WebKit::StorageManager::SessionStorageNamespace::allowedConnection const):
(WebKit::StorageManager::SessionStorageNamespace::setAllowedConnection):
(WebKit::StorageManager::setAllowedSessionStorageNamespaceConnection):
(WebKit::StorageManager::processDidCloseConnection):
(WebKit::StorageManager::createLocalStorageMap):
(WebKit::StorageManager::createTransientLocalStorageMap):
(WebKit::StorageManager::createSessionStorageMap):
(WebKit::StorageManager::destroyStorageMap):
(WebKit::StorageManager::setItem):
(WebKit::StorageManager::removeItem):
(WebKit::StorageManager::clear):
(WebKit::StorageManager::applicationWillTerminate):
(WebKit::StorageManager::findStorageArea const):
* UIProcess/WebStorage/StorageManager.h:

Modified Paths

trunk/Source/WebKit/ChangeLog
trunk/Source/WebKit/Platform/IPC/Connection.cpp
trunk/Source/WebKit/Platform/IPC/Connection.h
trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.cpp
trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.h

Diff

Modified: trunk/Source/WebKit/ChangeLog (234663 => 234664)


--- trunk/Source/WebKit/ChangeLog	2018-08-07 18:46:03 UTC (rev 234663)
+++ trunk/Source/WebKit/ChangeLog	2018-08-07 19:09:16 UTC (rev 234664)
@@ -1,3 +1,43 @@
+2018-08-07  Chris Dumez  <cdu...@apple.com>
+
+        StorageManager should stop ref'ing IPC::Connections as this is leak-prone
+        https://bugs.webkit.org/show_bug.cgi?id=188380
+
+        Reviewed by Alex Christensen.
+
+        StorageManager should stop ref'ing IPC::Connections as this is leak-prone. Instead, assign a unique identifier
+        to each IPC::Connection and store this identifier intead of a RefPtr<IPC::Connection>. When the StorageManager
+        needs an actual IPC::Connection, it can look it up from the identifier.
+
+        * Platform/IPC/Connection.cpp:
+        (IPC::Connection::Connection):
+        (IPC::Connection::~Connection):
+        (IPC::Connection::connection):
+        * Platform/IPC/Connection.h:
+        (IPC::Connection::uniqueID const):
+        * UIProcess/WebStorage/StorageManager.cpp:
+        (WebKit::StorageManager::StorageArea::addListener):
+        (WebKit::StorageManager::StorageArea::removeListener):
+        (WebKit::StorageManager::StorageArea::hasListener const):
+        (WebKit::StorageManager::StorageArea::setItem):
+        (WebKit::StorageManager::StorageArea::removeItem):
+        (WebKit::StorageManager::StorageArea::clear):
+        (WebKit::StorageManager::StorageArea::dispatchEvents const):
+        (WebKit::StorageManager::SessionStorageNamespace::allowedConnection const):
+        (WebKit::StorageManager::SessionStorageNamespace::setAllowedConnection):
+        (WebKit::StorageManager::setAllowedSessionStorageNamespaceConnection):
+        (WebKit::StorageManager::processDidCloseConnection):
+        (WebKit::StorageManager::createLocalStorageMap):
+        (WebKit::StorageManager::createTransientLocalStorageMap):
+        (WebKit::StorageManager::createSessionStorageMap):
+        (WebKit::StorageManager::destroyStorageMap):
+        (WebKit::StorageManager::setItem):
+        (WebKit::StorageManager::removeItem):
+        (WebKit::StorageManager::clear):
+        (WebKit::StorageManager::applicationWillTerminate):
+        (WebKit::StorageManager::findStorageArea const):
+        * UIProcess/WebStorage/StorageManager.h:
+
 2018-08-07  Eric Carlson  <eric.carl...@apple.com>
 
         NotReadableError when calling getUserMedia

Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (234663 => 234664)


--- trunk/Source/WebKit/Platform/IPC/Connection.cpp	2018-08-07 18:46:03 UTC (rev 234663)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp	2018-08-07 19:09:16 UTC (rev 234664)
@@ -231,8 +231,15 @@
     return adoptRef(*new Connection(identifier, false, client));
 }
 
+static HashMap<IPC::Connection::UniqueID, Connection*>& allConnections()
+{
+    static NeverDestroyed<HashMap<IPC::Connection::UniqueID, Connection*>> map;
+    return map;
+}
+
 Connection::Connection(Identifier identifier, bool isServer, Client& client)
     : m_client(client)
+    , m_uniqueID(generateObjectIdentifier<UniqueIDType>())
     , m_isServer(isServer)
     , m_syncRequestID(0)
     , m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(false)
@@ -248,6 +255,7 @@
     , m_shouldWaitForSyncReplies(true)
 {
     ASSERT(RunLoop::isMain());
+    allConnections().add(m_uniqueID, this);
 
     platformInitialize(identifier);
 
@@ -259,9 +267,18 @@
 
 Connection::~Connection()
 {
+    ASSERT(RunLoop::isMain());
     ASSERT(!isValid());
+
+    allConnections().remove(m_uniqueID);
 }
 
+Connection* Connection::connection(UniqueID uniqueID)
+{
+    ASSERT(RunLoop::isMain());
+    return allConnections().get(uniqueID);
+}
+
 void Connection::setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(bool flag)
 {
     ASSERT(!m_isConnected);

Modified: trunk/Source/WebKit/Platform/IPC/Connection.h (234663 => 234664)


--- trunk/Source/WebKit/Platform/IPC/Connection.h	2018-08-07 18:46:03 UTC (rev 234663)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h	2018-08-07 19:09:16 UTC (rev 234664)
@@ -39,6 +39,7 @@
 #include <wtf/Forward.h>
 #include <wtf/HashMap.h>
 #include <wtf/Lock.h>
+#include <wtf/ObjectIdentifier.h>
 #include <wtf/OptionSet.h>
 #include <wtf/RunLoop.h>
 #include <wtf/WorkQueue.h>
@@ -86,7 +87,7 @@
 class MachMessage;
 class UnixMessage;
 
-class Connection : public ThreadSafeRefCounted<Connection> {
+class Connection : public ThreadSafeRefCounted<Connection, WTF::DestructionThread::Main> {
 public:
     class Client : public MessageReceiver {
     public:
@@ -151,6 +152,12 @@
 
     Client& client() const { return m_client; }
 
+    enum UniqueIDType { };
+    using UniqueID = ObjectIdentifier<UniqueIDType>;
+
+    static Connection* connection(UniqueID);
+    UniqueID uniqueID() const { return m_uniqueID; }
+
     void setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(bool);
     void setShouldExitOnSyncMessageSendFailure(bool);
 
@@ -271,6 +278,7 @@
     };
 
     Client& m_client;
+    UniqueID m_uniqueID;
     bool m_isServer;
     std::atomic<bool> m_isValid { true };
     std::atomic<uint64_t> m_syncRequestID;

Modified: trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.cpp (234663 => 234664)


--- trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.cpp	2018-08-07 18:46:03 UTC (rev 234663)
+++ trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.cpp	2018-08-07 19:09:16 UTC (rev 234664)
@@ -49,15 +49,15 @@
 
     const WebCore::SecurityOriginData& securityOrigin() const { return m_securityOrigin; }
 
-    void addListener(IPC::Connection&, uint64_t storageMapID);
-    void removeListener(IPC::Connection&, uint64_t storageMapID);
-    bool hasListener(IPC::Connection&, uint64_t storageMapID) const;
+    void addListener(IPC::Connection::UniqueID, uint64_t storageMapID);
+    void removeListener(IPC::Connection::UniqueID, uint64_t storageMapID);
+    bool hasListener(IPC::Connection::UniqueID, uint64_t storageMapID) const;
 
     Ref<StorageArea> clone() const;
 
-    void setItem(IPC::Connection* sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& value, const String& urlString, bool& quotaException);
-    void removeItem(IPC::Connection* sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& urlString);
-    void clear(IPC::Connection* sourceConnection, uint64_t sourceStorageAreaID, const String& urlString);
+    void setItem(IPC::Connection::UniqueID sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& value, const String& urlString, bool& quotaException);
+    void removeItem(IPC::Connection::UniqueID sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& urlString);
+    void clear(IPC::Connection::UniqueID sourceConnection, uint64_t sourceStorageAreaID, const String& urlString);
 
     const HashMap<String, String>& items() const;
     void clear();
@@ -69,7 +69,7 @@
 
     void openDatabaseAndImportItemsIfNeeded() const;
 
-    void dispatchEvents(IPC::Connection* sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& oldValue, const String& newValue, const String& urlString) const;
+    void dispatchEvents(IPC::Connection::UniqueID sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& oldValue, const String& newValue, const String& urlString) const;
 
     // Will be null if the storage area belongs to a session storage namespace.
     LocalStorageNamespace* m_localStorageNamespace;
@@ -80,7 +80,7 @@
     unsigned m_quotaInBytes;
 
     RefPtr<StorageMap> m_storageMap;
-    HashSet<std::pair<RefPtr<IPC::Connection>, uint64_t>> m_eventListeners;
+    HashSet<std::pair<IPC::Connection::UniqueID, uint64_t>> m_eventListeners;
 };
 
 class StorageManager::LocalStorageNamespace : public ThreadSafeRefCounted<LocalStorageNamespace> {
@@ -185,21 +185,21 @@
         m_localStorageNamespace->didDestroyStorageArea(this);
 }
 
-void StorageManager::StorageArea::addListener(IPC::Connection& connection, uint64_t storageMapID)
+void StorageManager::StorageArea::addListener(IPC::Connection::UniqueID connectionID, uint64_t storageMapID)
 {
-    ASSERT(!m_eventListeners.contains(std::make_pair(&connection, storageMapID)));
-    m_eventListeners.add(std::make_pair(&connection, storageMapID));
+    ASSERT(!m_eventListeners.contains(std::make_pair(connectionID, storageMapID)));
+    m_eventListeners.add(std::make_pair(connectionID, storageMapID));
 }
 
-void StorageManager::StorageArea::removeListener(IPC::Connection& connection, uint64_t storageMapID)
+void StorageManager::StorageArea::removeListener(IPC::Connection::UniqueID connectionID, uint64_t storageMapID)
 {
-    ASSERT(isSessionStorage() || m_eventListeners.contains(std::make_pair(&connection, storageMapID)));
-    m_eventListeners.remove(std::make_pair(&connection, storageMapID));
+    ASSERT(isSessionStorage() || m_eventListeners.contains(std::make_pair(connectionID, storageMapID)));
+    m_eventListeners.remove(std::make_pair(connectionID, storageMapID));
 }
 
-bool StorageManager::StorageArea::hasListener(IPC::Connection& connection, uint64_t storageMapID) const
+bool StorageManager::StorageArea::hasListener(IPC::Connection::UniqueID connectionID, uint64_t storageMapID) const
 {
-    return m_eventListeners.contains(std::make_pair(&connection, storageMapID));
+    return m_eventListeners.contains(std::make_pair(connectionID, storageMapID));
 }
 
 Ref<StorageManager::StorageArea> StorageManager::StorageArea::clone() const
@@ -212,7 +212,7 @@
     return storageArea;
 }
 
-void StorageManager::StorageArea::setItem(IPC::Connection* sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& value, const String& urlString, bool& quotaException)
+void StorageManager::StorageArea::setItem(IPC::Connection::UniqueID sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& value, const String& urlString, bool& quotaException)
 {
     openDatabaseAndImportItemsIfNeeded();
 
@@ -231,7 +231,7 @@
     dispatchEvents(sourceConnection, sourceStorageAreaID, key, oldValue, value, urlString);
 }
 
-void StorageManager::StorageArea::removeItem(IPC::Connection* sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& urlString)
+void StorageManager::StorageArea::removeItem(IPC::Connection::UniqueID sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& urlString)
 {
     openDatabaseAndImportItemsIfNeeded();
 
@@ -249,7 +249,7 @@
     dispatchEvents(sourceConnection, sourceStorageAreaID, key, oldValue, String(), urlString);
 }
 
-void StorageManager::StorageArea::clear(IPC::Connection* sourceConnection, uint64_t sourceStorageAreaID, const String& urlString)
+void StorageManager::StorageArea::clear(IPC::Connection::UniqueID sourceConnection, uint64_t sourceStorageAreaID, const String& urlString)
 {
     openDatabaseAndImportItemsIfNeeded();
 
@@ -280,8 +280,12 @@
         m_localStorageDatabase = nullptr;
     }
 
-    for (auto it = m_eventListeners.begin(), end = m_eventListeners.end(); it != end; ++it)
-        it->first->send(Messages::StorageAreaMap::ClearCache(), it->second);
+    for (auto it = m_eventListeners.begin(), end = m_eventListeners.end(); it != end; ++it) {
+        RunLoop::main().dispatch([connectionID = it->first, destinationStorageAreaID = it->second] {
+            if (auto* connection = IPC::Connection::connection(connectionID))
+                connection->send(Messages::StorageAreaMap::ClearCache(), destinationStorageAreaID);
+        });
+    }
 }
 
 void StorageManager::StorageArea::openDatabaseAndImportItemsIfNeeded() const
@@ -300,12 +304,15 @@
     m_didImportItemsFromDatabase = true;
 }
 
-void StorageManager::StorageArea::dispatchEvents(IPC::Connection* sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& oldValue, const String& newValue, const String& urlString) const
+void StorageManager::StorageArea::dispatchEvents(IPC::Connection::UniqueID sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& oldValue, const String& newValue, const String& urlString) const
 {
-    for (HashSet<std::pair<RefPtr<IPC::Connection>, uint64_t>>::const_iterator it = m_eventListeners.begin(), end = m_eventListeners.end(); it != end; ++it) {
-        uint64_t storageAreaID = it->first == sourceConnection ? sourceStorageAreaID : 0;
+    for (auto it = m_eventListeners.begin(), end = m_eventListeners.end(); it != end; ++it) {
+        sourceStorageAreaID = it->first == sourceConnection ? sourceStorageAreaID : 0;
 
-        it->first->send(Messages::StorageAreaMap::DispatchStorageEvent(storageAreaID, key, oldValue, newValue, urlString), it->second);
+        RunLoop::main().dispatch([connectionID = it->first, sourceStorageAreaID, destinationStorageAreaID = it->second, key = key.isolatedCopy(), oldValue = oldValue.isolatedCopy(), newValue = newValue.isolatedCopy(), urlString = urlString.isolatedCopy()] {
+            if (auto* connection = IPC::Connection::connection(connectionID))
+                connection->send(Messages::StorageAreaMap::DispatchStorageEvent(sourceStorageAreaID, key, oldValue, newValue, urlString), destinationStorageAreaID);
+        });
     }
 }
 
@@ -373,8 +380,8 @@
 
     bool isEmpty() const { return m_storageAreaMap.isEmpty(); }
 
-    IPC::Connection* allowedConnection() const { return m_allowedConnection.get(); }
-    void setAllowedConnection(IPC::Connection*);
+    IPC::Connection::UniqueID allowedConnection() const { return m_allowedConnection; }
+    void setAllowedConnection(IPC::Connection::UniqueID);
 
     Ref<StorageArea> getOrCreateStorageArea(SecurityOriginData&&);
 
@@ -409,7 +416,7 @@
 private:
     explicit SessionStorageNamespace(unsigned quotaInBytes);
 
-    RefPtr<IPC::Connection> m_allowedConnection;
+    IPC::Connection::UniqueID m_allowedConnection;
     unsigned m_quotaInBytes;
 
     HashMap<SecurityOriginData, RefPtr<StorageArea>> m_storageAreaMap;
@@ -429,7 +436,7 @@
 {
 }
 
-void StorageManager::SessionStorageNamespace::setAllowedConnection(IPC::Connection* allowedConnection)
+void StorageManager::SessionStorageNamespace::setAllowedConnection(IPC::Connection::UniqueID allowedConnection)
 {
     m_allowedConnection = allowedConnection;
 }
@@ -485,10 +492,11 @@
 
 void StorageManager::setAllowedSessionStorageNamespaceConnection(uint64_t storageNamespaceID, IPC::Connection* allowedConnection)
 {
-    m_queue->dispatch([this, protectedThis = makeRef(*this), connection = RefPtr<IPC::Connection>(allowedConnection), storageNamespaceID]() mutable {
+    auto allowedConnectionID = allowedConnection ? allowedConnection->uniqueID() : IPC::Connection::UniqueID { };
+    m_queue->dispatch([this, protectedThis = makeRef(*this), allowedConnectionID, storageNamespaceID]() mutable {
         ASSERT(m_sessionStorageNamespaces.contains(storageNamespaceID));
 
-        m_sessionStorageNamespaces.get(storageNamespaceID)->setAllowedConnection(connection.get());
+        m_sessionStorageNamespaces.get(storageNamespaceID)->setAllowedConnection(allowedConnectionID);
     });
 }
 
@@ -519,13 +527,13 @@
 {
     connection.removeWorkQueueMessageReceiver(Messages::StorageManager::messageReceiverName());
 
-    m_queue->dispatch([this, protectedThis = makeRef(*this), connection = Ref<IPC::Connection>(connection)]() mutable {
-        Vector<std::pair<RefPtr<IPC::Connection>, uint64_t>> connectionAndStorageMapIDPairsToRemove;
+    m_queue->dispatch([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID()]() mutable {
+        Vector<std::pair<IPC::Connection::UniqueID, uint64_t>> connectionAndStorageMapIDPairsToRemove;
         for (auto& storageArea : m_storageAreasByConnection) {
-            if (storageArea.key.first != connection.ptr())
+            if (storageArea.key.first != connectionID)
                 continue;
 
-            storageArea.value->removeListener(*storageArea.key.first, storageArea.key.second);
+            storageArea.value->removeListener(storageArea.key.first, storageArea.key.second);
             connectionAndStorageMapIDPairsToRemove.append(storageArea.key);
         }
 
@@ -665,12 +673,12 @@
 
 void StorageManager::createLocalStorageMap(IPC::Connection& connection, uint64_t storageMapID, uint64_t storageNamespaceID, SecurityOriginData&& securityOriginData)
 {
-    std::pair<RefPtr<IPC::Connection>, uint64_t> connectionAndStorageMapIDPair(&connection, storageMapID);
+    std::pair<IPC::Connection::UniqueID, uint64_t> connectionAndStorageMapIDPair(connection.uniqueID(), storageMapID);
 
     // FIXME: This should be a message check.
-    ASSERT((HashMap<std::pair<RefPtr<IPC::Connection>, uint64_t>, RefPtr<StorageArea>>::isValidKey(connectionAndStorageMapIDPair)));
+    ASSERT((HashMap<std::pair<IPC::Connection::UniqueID, uint64_t>, RefPtr<StorageArea>>::isValidKey(connectionAndStorageMapIDPair)));
 
-    HashMap<std::pair<RefPtr<IPC::Connection>, uint64_t>, RefPtr<StorageArea>>::AddResult result = m_storageAreasByConnection.add(connectionAndStorageMapIDPair, nullptr);
+    auto result = m_storageAreasByConnection.add(connectionAndStorageMapIDPair, nullptr);
 
     // FIXME: These should be a message checks.
     ASSERT(result.isNewEntry);
@@ -682,7 +690,7 @@
     ASSERT(localStorageNamespace);
 
     auto storageArea = localStorageNamespace->getOrCreateStorageArea(WTFMove(securityOriginData));
-    storageArea->addListener(connection, storageMapID);
+    storageArea->addListener(connection.uniqueID(), storageMapID);
 
     result.iterator->value = WTFMove(storageArea);
 }
@@ -690,12 +698,12 @@
 void StorageManager::createTransientLocalStorageMap(IPC::Connection& connection, uint64_t storageMapID, uint64_t storageNamespaceID, SecurityOriginData&& topLevelOriginData, SecurityOriginData&& origin)
 {
     // FIXME: This should be a message check.
-    ASSERT(m_storageAreasByConnection.isValidKey({ &connection, storageMapID }));
+    ASSERT(m_storageAreasByConnection.isValidKey({ connection.uniqueID(), storageMapID }));
 
     // See if we already have session storage for this connection/origin combo.
     // If so, update the map with the new ID, otherwise keep on trucking.
     for (auto it = m_storageAreasByConnection.begin(), end = m_storageAreasByConnection.end(); it != end; ++it) {
-        if (it->key.first != &connection)
+        if (it->key.first != connection.uniqueID())
             continue;
         Ref<StorageArea> area = *it->value;
         if (!area->isSessionStorage())
@@ -702,18 +710,18 @@
             continue;
         if (!origin.securityOrigin()->isSameSchemeHostPort(area->securityOrigin().securityOrigin().get()))
             continue;
-        area->addListener(connection, storageMapID);
+        area->addListener(connection.uniqueID(), storageMapID);
         // If the storageMapID used as key in m_storageAreasByConnection is no longer one of the StorageArea's listeners, then this means
         // that destroyStorageMap() was already called for that storageMapID but it decided not to remove it from m_storageAreasByConnection
         // so that we could reuse it later on for the same connection/origin combo. In this case, it is safe to remove the previous
         // storageMapID from m_storageAreasByConnection.
-        if (!area->hasListener(connection, it->key.second))
+        if (!area->hasListener(connection.uniqueID(), it->key.second))
             m_storageAreasByConnection.remove(it);
-        m_storageAreasByConnection.add({ &connection, storageMapID }, WTFMove(area));
+        m_storageAreasByConnection.add({ connection.uniqueID(), storageMapID }, WTFMove(area));
         return;
     }
 
-    auto& slot = m_storageAreasByConnection.add({ &connection, storageMapID }, nullptr).iterator->value;
+    auto& slot = m_storageAreasByConnection.add({ connection.uniqueID(), storageMapID }, nullptr).iterator->value;
 
     // FIXME: This should be a message check.
     ASSERT(!slot);
@@ -721,7 +729,7 @@
     TransientLocalStorageNamespace* transientLocalStorageNamespace = getOrCreateTransientLocalStorageNamespace(storageNamespaceID, WTFMove(topLevelOriginData));
 
     auto storageArea = transientLocalStorageNamespace->getOrCreateStorageArea(WTFMove(origin));
-    storageArea->addListener(connection, storageMapID);
+    storageArea->addListener(connection.uniqueID(), storageMapID);
 
     slot = WTFMove(storageArea);
 }
@@ -739,18 +747,18 @@
     }
 
     // FIXME: This should be a message check.
-    ASSERT(m_storageAreasByConnection.isValidKey({ &connection, storageMapID }));
+    ASSERT(m_storageAreasByConnection.isValidKey({ connection.uniqueID(), storageMapID }));
 
-    auto& slot = m_storageAreasByConnection.add({ &connection, storageMapID }, nullptr).iterator->value;
+    auto& slot = m_storageAreasByConnection.add({ connection.uniqueID(), storageMapID }, nullptr).iterator->value;
 
     // FIXME: This should be a message check.
     ASSERT(!slot);
 
     // FIXME: This should be a message check.
-    ASSERT(&connection == sessionStorageNamespace->allowedConnection());
+    ASSERT(connection.uniqueID() == sessionStorageNamespace->allowedConnection());
 
     auto storageArea = sessionStorageNamespace->getOrCreateStorageArea(WTFMove(securityOriginData));
-    storageArea->addListener(connection, storageMapID);
+    storageArea->addListener(connection.uniqueID(), storageMapID);
 
     slot = WTFMove(storageArea);
 }
@@ -757,7 +765,7 @@
 
 void StorageManager::destroyStorageMap(IPC::Connection& connection, uint64_t storageMapID)
 {
-    std::pair<RefPtr<IPC::Connection>, uint64_t> connectionAndStorageMapIDPair(&connection, storageMapID);
+    std::pair<IPC::Connection::UniqueID, uint64_t> connectionAndStorageMapIDPair(connection.uniqueID(), storageMapID);
 
     // FIXME: This should be a message check.
     ASSERT(m_storageAreasByConnection.isValidKey(connectionAndStorageMapIDPair));
@@ -768,7 +776,7 @@
         return;
     }
 
-    it->value->removeListener(connection, storageMapID);
+    it->value->removeListener(connection.uniqueID(), storageMapID);
 
     // Don't remove session storage maps. The web process may reconnect and expect the data to still be around.
     if (it->value->isSessionStorage())
@@ -798,7 +806,7 @@
     }
 
     bool quotaError;
-    storageArea->setItem(&connection, sourceStorageAreaID, key, value, urlString, quotaError);
+    storageArea->setItem(connection.uniqueID(), sourceStorageAreaID, key, value, urlString, quotaError);
     connection.send(Messages::StorageAreaMap::DidSetItem(storageMapSeed, key, quotaError), storageMapID);
 }
 
@@ -810,7 +818,7 @@
         return;
     }
 
-    storageArea->removeItem(&connection, sourceStorageAreaID, key, urlString);
+    storageArea->removeItem(connection.uniqueID(), sourceStorageAreaID, key, urlString);
     connection.send(Messages::StorageAreaMap::DidRemoveItem(storageMapSeed, key), storageMapID);
 }
 
@@ -822,7 +830,7 @@
         return;
     }
 
-    storageArea->clear(&connection, sourceStorageAreaID, urlString);
+    storageArea->clear(connection.uniqueID(), sourceStorageAreaID, urlString);
     connection.send(Messages::StorageAreaMap::DidClear(storageMapSeed), storageMapID);
 }
 
@@ -830,9 +838,9 @@
 {
     BinarySemaphore semaphore;
     m_queue->dispatch([this, &semaphore] {
-        Vector<std::pair<RefPtr<IPC::Connection>, uint64_t>> connectionAndStorageMapIDPairsToRemove;
+        Vector<std::pair<IPC::Connection::UniqueID, uint64_t>> connectionAndStorageMapIDPairsToRemove;
         for (auto& connectionStorageAreaPair : m_storageAreasByConnection) {
-            connectionStorageAreaPair.value->removeListener(*connectionStorageAreaPair.key.first, connectionStorageAreaPair.key.second);
+            connectionStorageAreaPair.value->removeListener(connectionStorageAreaPair.key.first, connectionStorageAreaPair.key.second);
             connectionAndStorageMapIDPairsToRemove.append(connectionStorageAreaPair.key);
         }
 
@@ -846,7 +854,7 @@
 
 StorageManager::StorageArea* StorageManager::findStorageArea(IPC::Connection& connection, uint64_t storageMapID) const
 {
-    std::pair<IPC::Connection*, uint64_t> connectionAndStorageMapIDPair(&connection, storageMapID);
+    std::pair<IPC::Connection::UniqueID, uint64_t> connectionAndStorageMapIDPair(connection.uniqueID(), storageMapID);
 
     if (!m_storageAreasByConnection.isValidKey(connectionAndStorageMapIDPair))
         return nullptr;

Modified: trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.h (234663 => 234664)


--- trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.h	2018-08-07 18:46:03 UTC (rev 234663)
+++ trunk/Source/WebKit/UIProcess/WebStorage/StorageManager.h	2018-08-07 19:09:16 UTC (rev 234664)
@@ -105,7 +105,7 @@
     class SessionStorageNamespace;
     HashMap<uint64_t, RefPtr<SessionStorageNamespace>> m_sessionStorageNamespaces;
 
-    HashMap<std::pair<RefPtr<IPC::Connection>, uint64_t>, RefPtr<StorageArea>> m_storageAreasByConnection;
+    HashMap<std::pair<IPC::Connection::UniqueID, uint64_t>, RefPtr<StorageArea>> m_storageAreasByConnection;
 };
 
 } // namespace WebKit

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes