Title: [248899] trunk/Source/WebCore
Revision
248899
Author
you...@apple.com
Date
2019-08-20 06:26:06 -0700 (Tue, 20 Aug 2019)

Log Message

Make IDB quota check lambdas take a weak of UniqueIDBDatabaseTransaction instead of a ref
https://bugs.webkit.org/show_bug.cgi?id=196696

Reviewed by Alex Christensen.

Refing the transaction in the lambdas extend their lifetime.
Taking a weak pointer of them is better as this will not extend their lifetime.
In particular, if the database is deleted, the corresponding transactions might be deleted.
This makes quota checks less intrusive in IDB lifetime management.
Covered by existing tests.

* Modules/indexeddb/server/UniqueIDBDatabase.cpp:
(WebCore::IDBServer::UniqueIDBDatabase::createObjectStore):
(WebCore::IDBServer::UniqueIDBDatabase::deleteObjectStore):
(WebCore::IDBServer::UniqueIDBDatabase::renameObjectStore):
(WebCore::IDBServer::UniqueIDBDatabase::clearObjectStore):
(WebCore::IDBServer::UniqueIDBDatabase::createIndex):
(WebCore::IDBServer::UniqueIDBDatabase::deleteIndex):
(WebCore::IDBServer::UniqueIDBDatabase::renameIndex):
(WebCore::IDBServer::UniqueIDBDatabase::commitTransaction):
(WebCore::IDBServer::UniqueIDBDatabase::abortTransaction):
* Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (248898 => 248899)


--- trunk/Source/WebCore/ChangeLog	2019-08-20 10:41:33 UTC (rev 248898)
+++ trunk/Source/WebCore/ChangeLog	2019-08-20 13:26:06 UTC (rev 248899)
@@ -1,5 +1,30 @@
 2019-08-20  Youenn Fablet  <you...@apple.com>
 
+        Make IDB quota check lambdas take a weak of UniqueIDBDatabaseTransaction instead of a ref
+        https://bugs.webkit.org/show_bug.cgi?id=196696
+
+        Reviewed by Alex Christensen.
+
+        Refing the transaction in the lambdas extend their lifetime.
+        Taking a weak pointer of them is better as this will not extend their lifetime.
+        In particular, if the database is deleted, the corresponding transactions might be deleted.
+        This makes quota checks less intrusive in IDB lifetime management.
+        Covered by existing tests.
+
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::createObjectStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::deleteObjectStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::renameObjectStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::clearObjectStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::createIndex):
+        (WebCore::IDBServer::UniqueIDBDatabase::deleteIndex):
+        (WebCore::IDBServer::UniqueIDBDatabase::renameIndex):
+        (WebCore::IDBServer::UniqueIDBDatabase::commitTransaction):
+        (WebCore::IDBServer::UniqueIDBDatabase::abortTransaction):
+        * Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h:
+
+2019-08-20  Youenn Fablet  <you...@apple.com>
+
         Remove DeferredPromise::sessionID()
         https://bugs.webkit.org/show_bug.cgi?id=200616
 

Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp (248898 => 248899)


--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp	2019-08-20 10:41:33 UTC (rev 248898)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp	2019-08-20 13:26:06 UTC (rev 248899)
@@ -802,12 +802,16 @@
     LOG(IndexedDB, "(main) UniqueIDBDatabase::createObjectStore");
 
     auto taskSize = defaultWriteOperationCost + estimateSize(info);
-    requestSpace(taskSize, "createObjectStore", [this, taskSize, transaction = makeRef(transaction), info, callback = WTFMove(callback)](auto error) mutable {
+    requestSpace(taskSize, "createObjectStore", [this, taskSize, transaction = makeWeakPtr(transaction), info, callback = WTFMove(callback)](auto error) mutable {
         if (error) {
             callback(WTFMove(*error));
             return;
         }
-        this->createObjectStoreAfterQuotaCheck(taskSize, transaction.get(), info, WTFMove(callback));
+        if (!transaction) {
+            callback(IDBError { UnknownError });
+            return;
+        }
+        this->createObjectStoreAfterQuotaCheck(taskSize, *transaction, info, WTFMove(callback));
     });
 }
 
@@ -848,12 +852,16 @@
     ASSERT(isMainThread());
     LOG(IndexedDB, "(main) UniqueIDBDatabase::deleteObjectStore");
 
-    waitForRequestSpaceCompletion([this, transaction = makeRef(transaction), objectStoreName, callback = WTFMove(callback)](auto error) mutable {
+    waitForRequestSpaceCompletion([this, transaction = makeWeakPtr(transaction), objectStoreName, callback = WTFMove(callback)](auto error) mutable {
         if (error) {
             callback(WTFMove(*error));
             return;
         }
-        this->deleteObjectStoreAfterQuotaCheck(transaction, objectStoreName, WTFMove(callback));
+        if (!transaction) {
+            callback(IDBError { UnknownError });
+            return;
+        }
+        this->deleteObjectStoreAfterQuotaCheck(*transaction, objectStoreName, WTFMove(callback));
     });
 }
 
@@ -901,12 +909,16 @@
     LOG(IndexedDB, "(main) UniqueIDBDatabase::renameObjectStore");
 
     auto taskSize = defaultWriteOperationCost + newName.sizeInBytes();
-    requestSpace(taskSize, "renameObjectStore", [this, taskSize, transaction = makeRef(transaction), objectStoreIdentifier, newName, callback = WTFMove(callback)](auto error) mutable {
+    requestSpace(taskSize, "renameObjectStore", [this, taskSize, transaction = makeWeakPtr(transaction), objectStoreIdentifier, newName, callback = WTFMove(callback)](auto error) mutable {
         if (error) {
             callback(WTFMove(*error));
             return;
         }
-        this->renameObjectStoreAfterQuotaCheck(taskSize, transaction.get(), objectStoreIdentifier, newName, WTFMove(callback));
+        if (!transaction) {
+            callback(IDBError { UnknownError });
+            return;
+        }
+        this->renameObjectStoreAfterQuotaCheck(taskSize, *transaction, objectStoreIdentifier, newName, WTFMove(callback));
     });
 }
 
@@ -953,12 +965,16 @@
     ASSERT(isMainThread());
     LOG(IndexedDB, "(main) UniqueIDBDatabase::clearObjectStore");
 
-    waitForRequestSpaceCompletion([this, transaction = makeRef(transaction), objectStoreIdentifier, callback = WTFMove(callback)](auto error) mutable {
+    waitForRequestSpaceCompletion([this, transaction = makeWeakPtr(transaction), objectStoreIdentifier, callback = WTFMove(callback)](auto error) mutable {
         if (error) {
             callback(WTFMove(*error));
             return;
         }
-        this->clearObjectStoreAfetQuotaCheck(transaction, objectStoreIdentifier, WTFMove(callback));
+        if (!transaction) {
+            callback(IDBError { UnknownError });
+            return;
+        }
+        this->clearObjectStoreAfetQuotaCheck(*transaction, objectStoreIdentifier, WTFMove(callback));
     });
 }
 
@@ -996,12 +1012,16 @@
     LOG(IndexedDB, "(main) UniqueIDBDatabase::createIndex");
 
     auto taskSize = defaultWriteOperationCost + estimateSize(info);
-    requestSpace(taskSize, "createIndex", [this, taskSize, transaction = makeRef(transaction), info, callback = WTFMove(callback)](auto error) mutable {
+    requestSpace(taskSize, "createIndex", [this, taskSize, transaction = makeWeakPtr(transaction), info, callback = WTFMove(callback)](auto error) mutable {
         if (error) {
             callback(WTFMove(*error));
             return;
         }
-        this->createIndexAfterQuotaCheck(taskSize, transaction.get(), info, WTFMove(callback));
+        if (!transaction) {
+            callback(IDBError { UnknownError });
+            return;
+        }
+        this->createIndexAfterQuotaCheck(taskSize, *transaction, info, WTFMove(callback));
     });
 }
 
@@ -1051,12 +1071,16 @@
     ASSERT(isMainThread());
     LOG(IndexedDB, "(main) UniqueIDBDatabase::deleteIndex");
 
-    waitForRequestSpaceCompletion([this, transaction = makeRef(transaction), objectStoreIdentifier, indexName, callback = WTFMove(callback)](auto error) mutable {
+    waitForRequestSpaceCompletion([this, transaction = makeWeakPtr(transaction), objectStoreIdentifier, indexName, callback = WTFMove(callback)](auto error) mutable {
         if (error) {
             callback(WTFMove(*error));
             return;
         }
-        this->deleteIndexAfterQuotaCheck(transaction, objectStoreIdentifier, indexName, WTFMove(callback));
+        if (!transaction) {
+            callback(IDBError { UnknownError });
+            return;
+        }
+        this->deleteIndexAfterQuotaCheck(*transaction, objectStoreIdentifier, indexName, WTFMove(callback));
     });
 }
 
@@ -1113,12 +1137,16 @@
     LOG(IndexedDB, "(main) UniqueIDBDatabase::renameIndex");
 
     auto taskSize = defaultWriteOperationCost + newName.sizeInBytes();
-    requestSpace(taskSize, "renameIndex", [this, taskSize, transaction = makeRef(transaction), objectStoreIdentifier, indexIdentifier, newName, callback = WTFMove(callback)](auto error) mutable {
+    requestSpace(taskSize, "renameIndex", [this, taskSize, transaction = makeWeakPtr(transaction), objectStoreIdentifier, indexIdentifier, newName, callback = WTFMove(callback)](auto error) mutable {
         if (error) {
             callback(WTFMove(*error));
             return;
         }
-        this->renameIndexAfterQuotaCheck(taskSize, transaction.get(), objectStoreIdentifier, indexIdentifier, newName, WTFMove(callback));
+        if (!transaction) {
+            callback(IDBError { UnknownError });
+            return;
+        }
+        this->renameIndexAfterQuotaCheck(taskSize, *transaction, objectStoreIdentifier, indexIdentifier, newName, WTFMove(callback));
     });
 }
 
@@ -1591,12 +1619,16 @@
 
     ASSERT(transaction.databaseConnection().database() == this);
 
-    waitForRequestSpaceCompletion([this, transaction = makeRef(transaction), callback = WTFMove(callback)](auto error) mutable {
+    waitForRequestSpaceCompletion([this, transaction = makeWeakPtr(transaction), callback = WTFMove(callback)](auto error) mutable {
         if (error) {
             callback(WTFMove(*error));
             return;
         }
-        this->commitTransactionAfterQuotaCheck(transaction, WTFMove(callback));
+        if (!transaction) {
+            callback(IDBError { UnknownError });
+            return;
+        }
+        this->commitTransactionAfterQuotaCheck(*transaction, WTFMove(callback));
     });
 }
 
@@ -1663,12 +1695,16 @@
     ASSERT(transaction.databaseConnection().database() == this);
 
     if (waitForPendingTasks == WaitForPendingTasks::Yes) {
-        waitForRequestSpaceCompletion([this, transaction = makeRef(transaction), callback = WTFMove(callback)](auto&& error) mutable {
+        waitForRequestSpaceCompletion([this, transaction = makeWeakPtr(transaction), callback = WTFMove(callback)](auto&& error) mutable {
             if (error) {
                 callback(WTFMove(*error));
                 return;
             }
-            this->abortTransaction(transaction, WaitForPendingTasks::No, WTFMove(callback));
+            if (!transaction) {
+                callback(IDBError { UnknownError });
+                return;
+            }
+            this->abortTransaction(*transaction, WaitForPendingTasks::No, WTFMove(callback));
         });
         return;
     }

Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h (248898 => 248899)


--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h	2019-08-20 10:41:33 UTC (rev 248898)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h	2019-08-20 13:26:06 UTC (rev 248899)
@@ -53,7 +53,7 @@
 class IDBServer;
 class UniqueIDBDatabaseConnection;
 
-class UniqueIDBDatabaseTransaction : public RefCounted<UniqueIDBDatabaseTransaction> {
+class UniqueIDBDatabaseTransaction : public RefCounted<UniqueIDBDatabaseTransaction>, public CanMakeWeakPtr<UniqueIDBDatabaseTransaction> {
 public:
     enum class State { Running, Aborting, Committing, Aborted, Committed };
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to