Attachment 358421 Details for Bug 193167 – Patch

[patch] Patch

bug-193167-20190104191330.patch (text/plain), 7.27 KB, created by Sihui Liu on 2019-01-04 19:13:31 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-01-04 19:13:31 PST

Size: 7.27 KB

patch

obsolete

>Subversion Revision: 239647
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ffb5d36c8651e012ba5ec42fb56169b40e3d9147..bc63e1b90548fdd0e73f76aec9fe50f241a32dee 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-01-04  Sihui Liu  <sihui_liu@apple.com>
>+
>+        IndexedDB: leak IDBTransaction and TransactionOperationImpl due to retain cycle
>+        https://bugs.webkit.org/show_bug.cgi?id=193167
>+        <rdar://problem/46891688>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Break some retain cycles by using WeakPtr.
>+
>+        * Modules/indexeddb/IDBTransaction.cpp:
>+        (WebCore::IDBTransaction::IDBTransaction):
>+        (WebCore::IDBTransaction::~IDBTransaction):
>+        * Modules/indexeddb/IDBTransaction.h:
>+        * Modules/indexeddb/client/TransactionOperation.h:
>+        (WebCore::IDBClient::TransactionOperation::TransactionOperation):
>+        (WebCore::IDBClient::TransactionOperation::transaction):
>+
> 2019-01-04  Zalan Bujtas  <zalan@apple.com>
> 
>         [iOS] ERROR: post-layout: dirty renderer(s) in WebCore::RenderTreeNeedsLayoutChecker::~RenderTreeNeedsLayoutChecker()
>diff --git a/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp b/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp
>index b82b66a1d23f7af54383b2933c253ec433613626..990820d8b5242c048d57ffce936d8e6fc6162d8d 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp
>+++ b/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp
>@@ -75,7 +75,7 @@ IDBTransaction::IDBTransaction(IDBDatabase& database, const IDBTransactionInfo&
>     , m_info(info)
>     , m_pendingOperationTimer(*this, &IDBTransaction::pendingOperationTimerFired)
>     , m_completedOperationTimer(*this, &IDBTransaction::completedOperationTimerFired)
>-    , m_openDBRequest(request)
>+    , m_openDBRequest(makeWeakPtr(request))
>     , m_currentlyCompletingRequest(request)
> 
> {
>@@ -106,6 +106,8 @@ IDBTransaction::IDBTransaction(IDBDatabase& database, const IDBTransactionInfo&
> IDBTransaction::~IDBTransaction()
> {
>     ASSERT(&m_database->originThread() == &Thread::current());
>+    m_pendingOperationTimer.stop();
>+    m_completedOperationTimer.stop();
> }
> 
> IDBClient::IDBConnectionProxy& IDBTransaction::connectionProxy()
>diff --git a/Source/WebCore/Modules/indexeddb/IDBTransaction.h b/Source/WebCore/Modules/indexeddb/IDBTransaction.h
>index 1f76753bed5b55a8712d7a2df52bdd8cf80f8c46..14e606f9c48642bc67fdfcc5a975dd4d3076258d 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBTransaction.h
>+++ b/Source/WebCore/Modules/indexeddb/IDBTransaction.h
>@@ -65,7 +65,7 @@ class IDBConnectionProxy;
> class TransactionOperation;
> }
> 
>-class IDBTransaction : public ThreadSafeRefCounted<IDBTransaction>, public EventTargetWithInlineData, public IDBActiveDOMObject {
>+class IDBTransaction : public ThreadSafeRefCounted<IDBTransaction>, public EventTargetWithInlineData, public IDBActiveDOMObject, public CanMakeWeakPtr<IDBTransaction> {
> public:
>     static Ref<IDBTransaction> create(IDBDatabase&, const IDBTransactionInfo&);
>     static Ref<IDBTransaction> create(IDBDatabase&, const IDBTransactionInfo&, IDBOpenDBRequest&);
>@@ -241,7 +241,7 @@ private:
>     Timer m_completedOperationTimer;
>     std::unique_ptr<Timer> m_activationTimer;
> 
>-    RefPtr<IDBOpenDBRequest> m_openDBRequest;
>+    WeakPtr<IDBOpenDBRequest> m_openDBRequest;
> 
>     Deque<RefPtr<IDBClient::TransactionOperation>> m_pendingTransactionOperationQueue;
>     Deque<IDBClient::TransactionOperation*> m_transactionOperationsInProgressQueue;
>diff --git a/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h b/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h
>index 8ddb56e178b72ef21a666eb169dfc16ee9892de3..edf26a16b50d0e2518ff186889a619f95334ef0d 100644
>--- a/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h
>+++ b/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h
>@@ -111,14 +111,14 @@ public:
> 
> protected:
>     TransactionOperation(IDBTransaction& transaction)
>-        : m_transaction(transaction)
>+        : m_transaction(makeWeakPtr(transaction))
>         , m_identifier(transaction.connectionProxy())
>     {
>     }
> 
>     TransactionOperation(IDBTransaction&, IDBRequest&);
> 
>-    Ref<IDBTransaction> m_transaction;
>+    WeakPtr<IDBTransaction> m_transaction;
>     IDBResourceIdentifier m_identifier;
>     uint64_t m_objectStoreIdentifier { 0 };
>     uint64_t m_indexIdentifier { 0 };
>@@ -132,7 +132,7 @@ private:
>     uint64_t objectStoreIdentifier() const { return m_objectStoreIdentifier; }
>     uint64_t indexIdentifier() const { return m_indexIdentifier; }
>     IDBResourceIdentifier* cursorIdentifier() const { return m_cursorIdentifier.get(); }
>-    IDBTransaction& transaction() { return m_transaction.get(); }
>+    IDBTransaction& transaction() { return *m_transaction; }
>     IndexedDB::IndexRecordType indexRecordType() const { return m_indexRecordType; }
> 
>     Ref<Thread> m_originThread { Thread::current() };
>@@ -146,17 +146,15 @@ public:
>     TransactionOperationImpl(IDBTransaction& transaction, void (IDBTransaction::*completeMethod)(const IDBResultData&), void (IDBTransaction::*performMethod)(TransactionOperation&, Arguments...), Arguments&&... arguments)
>         : TransactionOperation(transaction)
>     {
>-        RefPtr<TransactionOperation> protectedThis(this);
>-
>         ASSERT(performMethod);
>-        m_performFunction = [protectedThis, this, performMethod, arguments...] {
>-            (&m_transaction.get()->*performMethod)(*this, arguments...);
>+        m_performFunction = [this, performMethod, arguments...] {
>+            (m_transaction.get()->*performMethod)(*this, arguments...);
>         };
> 
>         if (completeMethod) {
>-            m_completeFunction = [protectedThis, this, completeMethod](const IDBResultData& resultData) {
>+            m_completeFunction = [this, completeMethod](const IDBResultData& resultData) {
>                 if (completeMethod)
>-                    (&m_transaction.get()->*completeMethod)(resultData);
>+                    (m_transaction.get()->*completeMethod)(resultData);
>             };
>         }
>     }
>@@ -164,18 +162,16 @@ public:
>     TransactionOperationImpl(IDBTransaction& transaction, IDBRequest& request, void (IDBTransaction::*completeMethod)(IDBRequest&, const IDBResultData&), void (IDBTransaction::*performMethod)(TransactionOperation&, Arguments...), Arguments&&... arguments)
>         : TransactionOperation(transaction, request)
>     {
>-        RefPtr<TransactionOperation> protectedThis(this);
>-
>         ASSERT(performMethod);
>-        m_performFunction = [protectedThis, this, performMethod, arguments...] {
>-            (&m_transaction.get()->*performMethod)(*this, arguments...);
>+        m_performFunction = [this, performMethod, arguments...] {
>+            (m_transaction.get()->*performMethod)(*this, arguments...);
>         };
> 
>         if (completeMethod) {
>             RefPtr<IDBRequest> refRequest(&request);
>-            m_completeFunction = [protectedThis, this, refRequest, completeMethod](const IDBResultData& resultData) {
>+            m_completeFunction = [this, refRequest, completeMethod](const IDBResultData& resultData) {
>                 if (completeMethod)
>-                    (&m_transaction.get()->*completeMethod)(*refRequest, resultData);
>+                    (m_transaction.get()->*completeMethod)(*refRequest, resultData);
>             };
>         }
>     }

Actions: View | Formatted Diff | Diff

Attachments on bug 193167: 358421 | 358803 | 358815