Attachment 358930 Details for Bug 193097 – Patch for landing

[patch] Patch for landing

bug-193097-20190111123835.patch (text/plain), 15.91 KB, created by Sihui Liu on 2019-01-11 12:38:36 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-01-11 12:38:36 PST

Size: 15.91 KB

patch

obsolete

>Subversion Revision: 239864
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 8e2c89eef40eda86ba584289ec31574a441f2664..576bd18e32dfc1db514a52433766ad489ae0fd44 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,42 @@
>+2019-01-11  Sihui Liu  <sihui_liu@apple.com>
>+
>+        IndexedDB: leak WebIDBConnectionToClient for retain cycle
>+        https://bugs.webkit.org/show_bug.cgi?id=193097
>+        <rdar://problem/46899601>
>+
>+        Reviewed by Brady Eidson.
>+
>+        Let IDBConnectionToClient hold a WeakPtr of IDBConnectionToClientDelegate.
>+
>+        * Modules/indexeddb/server/IDBConnectionToClient.cpp:
>+        (WebCore::IDBServer::IDBConnectionToClient::IDBConnectionToClient):
>+        (WebCore::IDBServer::IDBConnectionToClient::identifier const):
>+        (WebCore::IDBServer::IDBConnectionToClient::didDeleteDatabase):
>+        (WebCore::IDBServer::IDBConnectionToClient::didOpenDatabase):
>+        (WebCore::IDBServer::IDBConnectionToClient::didAbortTransaction):
>+        (WebCore::IDBServer::IDBConnectionToClient::didCreateObjectStore):
>+        (WebCore::IDBServer::IDBConnectionToClient::didDeleteObjectStore):
>+        (WebCore::IDBServer::IDBConnectionToClient::didRenameObjectStore):
>+        (WebCore::IDBServer::IDBConnectionToClient::didClearObjectStore):
>+        (WebCore::IDBServer::IDBConnectionToClient::didCreateIndex):
>+        (WebCore::IDBServer::IDBConnectionToClient::didDeleteIndex):
>+        (WebCore::IDBServer::IDBConnectionToClient::didRenameIndex):
>+        (WebCore::IDBServer::IDBConnectionToClient::didPutOrAdd):
>+        (WebCore::IDBServer::IDBConnectionToClient::didGetRecord):
>+        (WebCore::IDBServer::IDBConnectionToClient::didGetAllRecords):
>+        (WebCore::IDBServer::IDBConnectionToClient::didGetCount):
>+        (WebCore::IDBServer::IDBConnectionToClient::didDeleteRecord):
>+        (WebCore::IDBServer::IDBConnectionToClient::didOpenCursor):
>+        (WebCore::IDBServer::IDBConnectionToClient::didIterateCursor):
>+        (WebCore::IDBServer::IDBConnectionToClient::didCommitTransaction):
>+        (WebCore::IDBServer::IDBConnectionToClient::fireVersionChangeEvent):
>+        (WebCore::IDBServer::IDBConnectionToClient::didStartTransaction):
>+        (WebCore::IDBServer::IDBConnectionToClient::didCloseFromServer):
>+        (WebCore::IDBServer::IDBConnectionToClient::notifyOpenDBRequestBlocked):
>+        (WebCore::IDBServer::IDBConnectionToClient::didGetAllDatabaseNames):
>+        * Modules/indexeddb/server/IDBConnectionToClient.h:
>+        * Modules/indexeddb/server/IDBConnectionToClientDelegate.h:
>+
> 2019-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Introduce IDL files for runtime-enabled UndoManager and UndoItem JavaScript API
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index d53cc33c6927e61aa4e69dae90da8051257de925..7bc7cabde54b51e5317a6a58c87f58576aa8e5af 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-01-11  Sihui Liu  <sihui_liu@apple.com>
>+
>+        IndexedDB: leak WebIDBConnectionToClient for retain cycle
>+        https://bugs.webkit.org/show_bug.cgi?id=193097
>+        <rdar://problem/46899601>
>+
>+        Reviewed by Brady Eidson.
>+
>+        Let WebIDBConnectionToClient hold reference to IPC::Connection instead of NetworkConnectionToWebProcess to break
>+        the cycle.
>+
>+        * NetworkProcess/IndexedDB/WebIDBConnectionToClient.cpp:
>+        (WebKit::WebIDBConnectionToClient::create):
>+        (WebKit::WebIDBConnectionToClient::WebIDBConnectionToClient):
>+        (WebKit::WebIDBConnectionToClient::messageSenderConnection):
>+        * NetworkProcess/IndexedDB/WebIDBConnectionToClient.h:
>+        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
>+        (WebKit::NetworkConnectionToWebProcess::establishIDBConnectionToServer):
>+
> 2019-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Introduce IDL files for runtime-enabled UndoManager and UndoItem JavaScript API
>diff --git a/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.cpp b/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.cpp
>index 55bea542432a89f74ab2abd97890ca878cb29deb..8c8b72052243769cc9e83e34908ad63c4631a3bb 100644
>--- a/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.cpp
>+++ b/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.cpp
>@@ -39,128 +39,152 @@ Ref<IDBConnectionToClient> IDBConnectionToClient::create(IDBConnectionToClientDe
> }
> 
> IDBConnectionToClient::IDBConnectionToClient(IDBConnectionToClientDelegate& delegate)
>-    : m_delegate(delegate)
>+    : m_delegate(makeWeakPtr(delegate))
> {
> }
> 
> uint64_t IDBConnectionToClient::identifier() const
> {
>+    ASSERT(m_delegate);
>     return m_delegate->identifier();
> }
> 
> void IDBConnectionToClient::didDeleteDatabase(const IDBResultData& result)
> {
>-    m_delegate->didDeleteDatabase(result);
>+    if (m_delegate)
>+        m_delegate->didDeleteDatabase(result);
> }
> 
> void IDBConnectionToClient::didOpenDatabase(const IDBResultData& result)
> {
>-    m_delegate->didOpenDatabase(result);
>+    if (m_delegate)
>+        m_delegate->didOpenDatabase(result);
> }
> 
> void IDBConnectionToClient::didAbortTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError& error)
> {
>-    m_delegate->didAbortTransaction(transactionIdentifier, error);
>+    if (m_delegate)
>+        m_delegate->didAbortTransaction(transactionIdentifier, error);
> }
> 
> void IDBConnectionToClient::didCreateObjectStore(const IDBResultData& result)
> {
>-    m_delegate->didCreateObjectStore(result);
>+    if (m_delegate)
>+        m_delegate->didCreateObjectStore(result);
> }
> 
> void IDBConnectionToClient::didDeleteObjectStore(const IDBResultData& result)
> {
>-    m_delegate->didDeleteObjectStore(result);
>+    if (m_delegate)
>+        m_delegate->didDeleteObjectStore(result);
> }
> 
> void IDBConnectionToClient::didRenameObjectStore(const IDBResultData& result)
> {
>-    m_delegate->didRenameObjectStore(result);
>+    if (m_delegate)
>+        m_delegate->didRenameObjectStore(result);
> }
> 
> void IDBConnectionToClient::didClearObjectStore(const IDBResultData& result)
> {
>-    m_delegate->didClearObjectStore(result);
>+    if (m_delegate)
>+        m_delegate->didClearObjectStore(result);
> }
> 
> void IDBConnectionToClient::didCreateIndex(const IDBResultData& result)
> {
>-    m_delegate->didCreateIndex(result);
>+    if (m_delegate)
>+        m_delegate->didCreateIndex(result);
> }
> 
> void IDBConnectionToClient::didDeleteIndex(const IDBResultData& result)
> {
>-    m_delegate->didDeleteIndex(result);
>+    if (m_delegate)
>+        m_delegate->didDeleteIndex(result);
> }
> 
> void IDBConnectionToClient::didRenameIndex(const IDBResultData& result)
> {
>-    m_delegate->didRenameIndex(result);
>+    if (m_delegate)
>+        m_delegate->didRenameIndex(result);
> }
> 
> void IDBConnectionToClient::didPutOrAdd(const IDBResultData& result)
> {
>-    m_delegate->didPutOrAdd(result);
>+    if (m_delegate)
>+        m_delegate->didPutOrAdd(result);
> }
> 
> void IDBConnectionToClient::didGetRecord(const IDBResultData& result)
> {
>-    m_delegate->didGetRecord(result);
>+    if (m_delegate)
>+        m_delegate->didGetRecord(result);
> }
> 
> void IDBConnectionToClient::didGetAllRecords(const IDBResultData& result)
> {
>-    m_delegate->didGetAllRecords(result);
>+    if (m_delegate)
>+        m_delegate->didGetAllRecords(result);
> }
> 
> void IDBConnectionToClient::didGetCount(const IDBResultData& result)
> {
>-    m_delegate->didGetCount(result);
>+    if (m_delegate)
>+        m_delegate->didGetCount(result);
> }
> 
> void IDBConnectionToClient::didDeleteRecord(const IDBResultData& result)
> {
>-    m_delegate->didDeleteRecord(result);
>+    if (m_delegate)
>+        m_delegate->didDeleteRecord(result);
> }
> 
> void IDBConnectionToClient::didOpenCursor(const IDBResultData& result)
> {
>-    m_delegate->didOpenCursor(result);
>+    if (m_delegate)
>+        m_delegate->didOpenCursor(result);
> }
> 
> void IDBConnectionToClient::didIterateCursor(const IDBResultData& result)
> {
>-    m_delegate->didIterateCursor(result);
>+    if (m_delegate)
>+        m_delegate->didIterateCursor(result);
> }
> 
> void IDBConnectionToClient::didCommitTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError& error)
> {
>-    m_delegate->didCommitTransaction(transactionIdentifier, error);
>+    if (m_delegate)
>+        m_delegate->didCommitTransaction(transactionIdentifier, error);
> }
> 
> void IDBConnectionToClient::fireVersionChangeEvent(UniqueIDBDatabaseConnection& connection, const IDBResourceIdentifier& requestIdentifier, uint64_t requestedVersion)
> {
>-    m_delegate->fireVersionChangeEvent(connection, requestIdentifier, requestedVersion);
>+    if (m_delegate)
>+        m_delegate->fireVersionChangeEvent(connection, requestIdentifier, requestedVersion);
> }
> 
> void IDBConnectionToClient::didStartTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError& error)
> {
>-    m_delegate->didStartTransaction(transactionIdentifier, error);
>+    if (m_delegate)
>+        m_delegate->didStartTransaction(transactionIdentifier, error);
> }
> 
> void IDBConnectionToClient::didCloseFromServer(UniqueIDBDatabaseConnection& connection, const IDBError& error)
> {
>-    m_delegate->didCloseFromServer(connection, error);
>+    if (m_delegate)
>+        m_delegate->didCloseFromServer(connection, error);
> }
> 
> void IDBConnectionToClient::notifyOpenDBRequestBlocked(const IDBResourceIdentifier& requestIdentifier, uint64_t oldVersion, uint64_t newVersion)
> {
>-    m_delegate->notifyOpenDBRequestBlocked(requestIdentifier, oldVersion, newVersion);
>+    if (m_delegate)
>+        m_delegate->notifyOpenDBRequestBlocked(requestIdentifier, oldVersion, newVersion);
> }
> 
> void IDBConnectionToClient::didGetAllDatabaseNames(uint64_t callbackID, const Vector<String>& databaseNames)
> {
>-    m_delegate->didGetAllDatabaseNames(callbackID, databaseNames);
>+    if (m_delegate)
>+        m_delegate->didGetAllDatabaseNames(callbackID, databaseNames);
> }
> 
> void IDBConnectionToClient::registerDatabaseConnection(UniqueIDBDatabaseConnection& connection)
>diff --git a/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.h b/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.h
>index 642813dd0f961fda2b701b493c5fce47b73e9926..160abfeec96003cbb1df335297f7b099d0f6800f 100644
>--- a/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.h
>+++ b/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.h
>@@ -31,6 +31,7 @@
> #include <wtf/HashSet.h>
> #include <wtf/Ref.h>
> #include <wtf/RefCounted.h>
>+#include <wtf/WeakPtr.h>
> 
> namespace WebCore {
> 
>@@ -82,7 +83,7 @@ public:
> private:
>     IDBConnectionToClient(IDBConnectionToClientDelegate&);
>     
>-    Ref<IDBConnectionToClientDelegate> m_delegate;
>+    WeakPtr<IDBConnectionToClientDelegate> m_delegate;
>     HashSet<UniqueIDBDatabaseConnection*> m_databaseConnections;
> };
> 
>diff --git a/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClientDelegate.h b/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClientDelegate.h
>index 479742a9da94e8548d72a2d64eb48594fab5fb47..fdf66407fe358ed937bbedcb69b2528063f5af41 100644
>--- a/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClientDelegate.h
>+++ b/Source/WebCore/Modules/indexeddb/server/IDBConnectionToClientDelegate.h
>@@ -28,6 +28,7 @@
> #if ENABLE(INDEXED_DATABASE)
> 
> #include <wtf/Forward.h>
>+#include <wtf/WeakPtr.h>
> #include <wtf/text/WTFString.h>
> 
> namespace WebCore {
>@@ -40,7 +41,7 @@ namespace IDBServer {
> 
> class UniqueIDBDatabaseConnection;
> 
>-class IDBConnectionToClientDelegate {
>+class IDBConnectionToClientDelegate : public CanMakeWeakPtr<IDBConnectionToClientDelegate> {
> public:
>     virtual ~IDBConnectionToClientDelegate() = default;
>     
>diff --git a/Source/WebKit/NetworkProcess/IndexedDB/WebIDBConnectionToClient.cpp b/Source/WebKit/NetworkProcess/IndexedDB/WebIDBConnectionToClient.cpp
>index 7fb2b97792b3018d5e1e0c378723a136c88d3d41..a1fecd2e03605dc8607169b373d295ed265d0b05 100644
>--- a/Source/WebKit/NetworkProcess/IndexedDB/WebIDBConnectionToClient.cpp
>+++ b/Source/WebKit/NetworkProcess/IndexedDB/WebIDBConnectionToClient.cpp
>@@ -44,12 +44,12 @@
> namespace WebKit {
> using namespace WebCore;
> 
>-Ref<WebIDBConnectionToClient> WebIDBConnectionToClient::create(NetworkProcess& networkProcess, NetworkConnectionToWebProcess& connection, uint64_t serverConnectionIdentifier, PAL::SessionID sessionID)
>+Ref<WebIDBConnectionToClient> WebIDBConnectionToClient::create(NetworkProcess& networkProcess, IPC::Connection& connection, uint64_t serverConnectionIdentifier, PAL::SessionID sessionID)
> {
>     return adoptRef(*new WebIDBConnectionToClient(networkProcess, connection, serverConnectionIdentifier, sessionID));
> }
> 
>-WebIDBConnectionToClient::WebIDBConnectionToClient(NetworkProcess& networkProcess, NetworkConnectionToWebProcess& connection, uint64_t serverConnectionIdentifier, PAL::SessionID sessionID)
>+WebIDBConnectionToClient::WebIDBConnectionToClient(NetworkProcess& networkProcess, IPC::Connection& connection, uint64_t serverConnectionIdentifier, PAL::SessionID sessionID)
>     : m_connection(connection)
>     , m_networkProcess(networkProcess)
>     , m_identifier(serverConnectionIdentifier)
>@@ -71,7 +71,7 @@ void WebIDBConnectionToClient::disconnectedFromWebProcess()
> 
> IPC::Connection* WebIDBConnectionToClient::messageSenderConnection()
> {
>-    return &m_connection->connection();
>+    return m_connection.ptr();
> }
> 
> WebCore::IDBServer::IDBConnectionToClient& WebIDBConnectionToClient::connectionToClient()
>diff --git a/Source/WebKit/NetworkProcess/IndexedDB/WebIDBConnectionToClient.h b/Source/WebKit/NetworkProcess/IndexedDB/WebIDBConnectionToClient.h
>index 9c74a58ee18dd695312760593a7356145591849e..54b1359f29cd965e6199efd4a4707c53a67ea1a0 100644
>--- a/Source/WebKit/NetworkProcess/IndexedDB/WebIDBConnectionToClient.h
>+++ b/Source/WebKit/NetworkProcess/IndexedDB/WebIDBConnectionToClient.h
>@@ -54,7 +54,7 @@ class NetworkProcess;
> 
> class WebIDBConnectionToClient final : public WebCore::IDBServer::IDBConnectionToClientDelegate, public IPC::MessageSender, public RefCounted<WebIDBConnectionToClient> {
> public:
>-    static Ref<WebIDBConnectionToClient> create(NetworkProcess&, NetworkConnectionToWebProcess&, uint64_t serverConnectionIdentifier, PAL::SessionID);
>+    static Ref<WebIDBConnectionToClient> create(NetworkProcess&, IPC::Connection&, uint64_t serverConnectionIdentifier, PAL::SessionID);
> 
>     virtual ~WebIDBConnectionToClient();
> 
>@@ -128,13 +128,13 @@ public:
>     void didReceiveMessage(IPC::Connection&, IPC::Decoder&);
> 
> private:
>-    WebIDBConnectionToClient(NetworkProcess&, NetworkConnectionToWebProcess&, uint64_t serverConnectionIdentifier, PAL::SessionID);
>+    WebIDBConnectionToClient(NetworkProcess&, IPC::Connection&, uint64_t serverConnectionIdentifier, PAL::SessionID);
> 
>     IPC::Connection* messageSenderConnection() final;
> 
>     template<class MessageType> void handleGetResult(const WebCore::IDBResultData&);
> 
>-    Ref<NetworkConnectionToWebProcess> m_connection;
>+    Ref<IPC::Connection> m_connection;
>     Ref<NetworkProcess> m_networkProcess;
> 
>     uint64_t m_identifier;
>diff --git a/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp b/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp
>index bc1969554582b756c54d8bee4b425509e1d547a1..73cf708a18fa9c9211db371ca0d03616fa3bf772 100644
>--- a/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp
>@@ -701,7 +701,7 @@ void NetworkConnectionToWebProcess::establishIDBConnectionToServer(PAL::SessionI
>     LOG(IndexedDB, "NetworkConnectionToWebProcess::establishIDBConnectionToServer - %" PRIu64, serverConnectionIdentifier);
>     ASSERT(!m_webIDBConnections.contains(serverConnectionIdentifier));
>     
>-    m_webIDBConnections.set(serverConnectionIdentifier, WebIDBConnectionToClient::create(m_networkProcess, *this, serverConnectionIdentifier, sessionID));
>+    m_webIDBConnections.set(serverConnectionIdentifier, WebIDBConnectionToClient::create(m_networkProcess, m_connection.get(), serverConnectionIdentifier, sessionID));
> }
> 
> void NetworkConnectionToWebProcess::removeIDBConnectionToServer(uint64_t serverConnectionIdentifier)

Actions: View | Formatted Diff | Diff

Attachments on bug 193097: 358271 | 358930 | 359249