Bug 1600283 - Reduce uses of plain pointers. r=#dom-workers-and-storage draft
authorSimon Giesecke <sgiesecke@mozilla.com>
Wed, 04 Dec 2019 16:47:44 +0000
changeset 2515975 cdc40458cc3b26221aadd15714a9272a0055d2af
parent 2515974 4956813097ce3d3ad83b07d1c4bca314279e435a
child 2515976 2440f9a3dda4be26ed7a156e96cb8002bdfdf74c
push id460090
push userreviewbot
push dateWed, 04 Dec 2019 16:51:29 +0000
treeherdertry@35d310cdd9cf [default view] [failures only]
bugs1600283
milestone73.0a1
Bug 1600283 - Reduce uses of plain pointers. r=#dom-workers-and-storage Differential Revision: https://phabricator.services.mozilla.com/D55477 Differential Diff: PHID-DIFF-q3fplhjmyplxyoddzh2a
dom/indexedDB/ActorsChild.cpp
dom/indexedDB/ActorsParent.cpp
dom/indexedDB/IDBDatabase.cpp
--- a/dom/indexedDB/ActorsChild.cpp
+++ b/dom/indexedDB/ActorsChild.cpp
@@ -3423,20 +3423,19 @@ void BackgroundCursorChild::SendContinue
     mDelayedResponses.emplace_back(std::move(mCachedResponses.front()));
     mCachedResponses.pop_front();
 
     // We cannot send the response right away, as we must preserve the request
     // order. Dispatching a DelayedActionRunnable only partially addresses this.
     // This is accompanied by invalidating cached entries at proper locations to
     // make it correct. To avoid this, further changes are necessary, see Bug
     // 1580499.
-    nsCOMPtr<nsIRunnable> continueRunnable = new DelayedActionRunnable(
-        this, &BackgroundCursorChild::CompleteContinueRequestFromCache);
-    MOZ_ALWAYS_TRUE(
-        NS_SUCCEEDED(NS_DispatchToCurrentThread(continueRunnable.forget())));
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
+        NS_DispatchToCurrentThread(MakeAndAddRef<DelayedActionRunnable>(
+            this, &BackgroundCursorChild::CompleteContinueRequestFromCache))));
 
     // TODO: Could we preload further entries in the background when the size of
     // mCachedResponses falls under some threshold? Or does the response
     // handling model disallow this?
   } else {
     MOZ_ALWAYS_TRUE(PBackgroundIDBCursorChild::SendContinue(
         params, currentKey, currentObjectStoreKey));
   }
@@ -3573,20 +3572,20 @@ void BackgroundCursorChild::HandleRespon
   if (mCursor) {
     mCursor->Reset();
   }
 
   ResultHelper helper(mRequest, mTransaction, &JS::NullHandleValue);
   DispatchSuccessEvent(&helper);
 
   if (!mCursor) {
-    nsCOMPtr<nsIRunnable> deleteRunnable = new DelayedActionRunnable(
-        this, &BackgroundCursorChild::SendDeleteMeInternal);
     MOZ_ALWAYS_SUCCEEDS(this->GetActorEventTarget()->Dispatch(
-        deleteRunnable.forget(), NS_DISPATCH_NORMAL));
+        MakeAndAddRef<DelayedActionRunnable>(
+            this, &BackgroundCursorChild::SendDeleteMeInternal),
+        NS_DISPATCH_NORMAL));
   }
 }
 
 template <typename... Args>
 RefPtr<IDBCursor> BackgroundCursorChild::HandleIndividualCursorResponse(
     const bool aUseAsCurrentResult, Args&&... aArgs) {
   if (mCursor) {
     if (aUseAsCurrentResult) {
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@@ -1901,21 +1901,20 @@ nsresult UpgradeSchemaFrom8To9_0(mozISto
 
   // We no longer use the dataVersion column.
   nsresult rv = aConnection->ExecuteSimpleSQL(
       NS_LITERAL_CSTRING("UPDATE database SET dataVersion = 0;"));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
-  nsCOMPtr<mozIStorageFunction> compressor = new CompressDataBlobsFunction();
-
   NS_NAMED_LITERAL_CSTRING(compressorName, "compress");
 
-  rv = aConnection->CreateFunction(compressorName, 1, compressor);
+  rv = aConnection->CreateFunction(
+      compressorName, 1, MakeAndAddRef<CompressDataBlobsFunction>().take());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // Turn off foreign key constraints before we do anything here.
   rv = aConnection->ExecuteSimpleSQL(
       NS_LITERAL_CSTRING("UPDATE object_data SET data = compress(data);"));
   if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -2193,19 +2192,18 @@ class EncodeKeysFunction final : public 
 nsresult UpgradeSchemaFrom11_0To12_0(mozIStorageConnection* aConnection) {
   AssertIsOnIOThread();
   MOZ_ASSERT(aConnection);
 
   AUTO_PROFILER_LABEL("UpgradeSchemaFrom11_0To12_0", DOM);
 
   NS_NAMED_LITERAL_CSTRING(encoderName, "encode");
 
-  nsCOMPtr<mozIStorageFunction> encoder = new EncodeKeysFunction();
-
-  nsresult rv = aConnection->CreateFunction(encoderName, 1, encoder);
+  nsresult rv = aConnection->CreateFunction(
+      encoderName, 1, MakeAndAddRef<EncodeKeysFunction>().take());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   rv = aConnection->ExecuteSimpleSQL(
       NS_LITERAL_CSTRING("CREATE TEMPORARY TABLE temp_upgrade ("
                          "id INTEGER PRIMARY KEY, "
                          "object_store_id, "
@@ -10765,21 +10763,19 @@ void DatabaseConnection::UpdateRefcountF
   mJournalsToRemoveAfterCommit.Clear();
   mJournalsToRemoveAfterAbort.Clear();
 
   // FileInfo implementation automatically removes unreferenced files, but it's
   // done asynchronously and with a delay. We want to remove them (and decrease
   // quota usage) before we fire the commit event.
   for (const auto& entry : mFileInfoEntries) {
     FileInfoEntry* const value = entry.GetData();
-
     MOZ_ASSERT(value);
 
     FileInfo* const fileInfo = value->mFileInfo.forget().take();
-
     MOZ_ASSERT(fileInfo);
 
     CustomCleanupCallback customCleanupCallback;
     fileInfo->Release(&customCleanupCallback);
   }
 
   mFileInfoEntries.Clear();
 }
@@ -11381,19 +11377,18 @@ void ConnectionPool::WaitForDatabasesToC
     }
   }
 
   if (mayRunCallbackImmediately) {
     Unused << aCallback->Run();
     return;
   }
 
-  nsAutoPtr<DatabasesCompleteCallback> callback(
+  mCompleteCallbacks.EmplaceBack(
       new DatabasesCompleteCallback(std::move(aDatabaseIds), aCallback));
-  mCompleteCallbacks.AppendElement(callback.forget());
 }
 
 void ConnectionPool::Shutdown() {
   AssertIsOnOwningThread();
   MOZ_ASSERT(!mShutdownRequested);
   MOZ_ASSERT(!mShutdownComplete);
 
   AUTO_PROFILER_LABEL("ConnectionPool::Shutdown", DOM);
@@ -11428,20 +11423,23 @@ void ConnectionPool::Cleanup() {
   MOZ_ASSERT(!mDatabases.Count());
   MOZ_ASSERT(!mTransactions.Count());
   MOZ_ASSERT(mIdleThreads.IsEmpty());
 
   AUTO_PROFILER_LABEL("ConnectionPool::Cleanup", DOM);
 
   if (!mCompleteCallbacks.IsEmpty()) {
     // Run all callbacks manually now.
+    // TODO: Can a callback cause another entry to be added to
+    // mCompleteCallbacks? This would be skipped when iterating like this.
     for (uint32_t count = mCompleteCallbacks.Length(), index = 0; index < count;
          index++) {
-      nsAutoPtr<DatabasesCompleteCallback> completeCallback(
-          mCompleteCallbacks[index].forget());
+      // TODO: Why do we need to move the callback here to a local variable?
+      // mCompleteCallbacks is cleared afterwards anyway.
+      const auto completeCallback = std::move(mCompleteCallbacks[index]);
       MOZ_ASSERT(completeCallback);
       MOZ_ASSERT(completeCallback->mCallback);
 
       Unused << completeCallback->mCallback->Run();
     }
 
     mCompleteCallbacks.Clear();
 
@@ -11694,23 +11692,18 @@ bool ConnectionPool::ScheduleTransaction
   MOZ_ASSERT(!aTransactionInfo->mRunning);
   aTransactionInfo->mRunning = true;
 
   nsTArray<nsCOMPtr<nsIRunnable>>& queuedRunnables =
       aTransactionInfo->mQueuedRunnables;
 
   if (!queuedRunnables.IsEmpty()) {
     for (auto& queuedRunnable : queuedRunnables) {
-      // TODO: Why do we need this? queuedRunnables is cleared afterwards
-      // anyway.
-      nsCOMPtr<nsIRunnable> runnable;
-      queuedRunnable.swap(runnable);
-
       MOZ_ALWAYS_SUCCEEDS(dbInfo->mThreadInfo.mThread->Dispatch(
-          runnable.forget(), NS_DISPATCH_NORMAL));
+          queuedRunnable.forget(), NS_DISPATCH_NORMAL));
     }
 
     queuedRunnables.Clear();
   }
 
   return true;
 }
 
@@ -11978,44 +11971,43 @@ void ConnectionPool::PerformIdleDatabase
   MOZ_ASSERT(aDatabaseInfo->mThreadInfo.mThread);
   MOZ_ASSERT(aDatabaseInfo->mThreadInfo.mRunnable);
   MOZ_ASSERT(aDatabaseInfo->mIdle);
   MOZ_ASSERT(!aDatabaseInfo->mCloseOnIdle);
   MOZ_ASSERT(!aDatabaseInfo->mClosing);
   MOZ_ASSERT(mIdleDatabases.Contains(aDatabaseInfo));
   MOZ_ASSERT(!mDatabasesPerformingIdleMaintenance.Contains(aDatabaseInfo));
 
-  nsCOMPtr<nsIRunnable> runnable = new IdleConnectionRunnable(
-      aDatabaseInfo, aDatabaseInfo->mNeedsCheckpoint);
+  const bool neededCheckpoint = aDatabaseInfo->mNeedsCheckpoint;
 
   aDatabaseInfo->mNeedsCheckpoint = false;
   aDatabaseInfo->mIdle = false;
 
   mDatabasesPerformingIdleMaintenance.AppendElement(aDatabaseInfo);
 
   MOZ_ALWAYS_SUCCEEDS(aDatabaseInfo->mThreadInfo.mThread->Dispatch(
-      runnable.forget(), NS_DISPATCH_NORMAL));
+      MakeAndAddRef<IdleConnectionRunnable>(aDatabaseInfo, neededCheckpoint),
+      NS_DISPATCH_NORMAL));
 }
 
 void ConnectionPool::CloseDatabase(DatabaseInfo* aDatabaseInfo) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(aDatabaseInfo);
   MOZ_DIAGNOSTIC_ASSERT(!aDatabaseInfo->TotalTransactionCount());
   MOZ_ASSERT(aDatabaseInfo->mThreadInfo.mThread);
   MOZ_ASSERT(aDatabaseInfo->mThreadInfo.mRunnable);
   MOZ_ASSERT(!aDatabaseInfo->mClosing);
 
   aDatabaseInfo->mIdle = false;
   aDatabaseInfo->mNeedsCheckpoint = false;
   aDatabaseInfo->mClosing = true;
 
-  nsCOMPtr<nsIRunnable> runnable = new CloseConnectionRunnable(aDatabaseInfo);
-
   MOZ_ALWAYS_SUCCEEDS(aDatabaseInfo->mThreadInfo.mThread->Dispatch(
-      runnable.forget(), NS_DISPATCH_NORMAL));
+      MakeAndAddRef<CloseConnectionRunnable>(aDatabaseInfo),
+      NS_DISPATCH_NORMAL));
 }
 
 bool ConnectionPool::CloseDatabaseWhenIdleInternal(
     const nsACString& aDatabaseId) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(!aDatabaseId.IsEmpty());
 
   AUTO_PROFILER_LABEL("ConnectionPool::CloseDatabaseWhenIdleInternal", DOM);
@@ -12600,19 +12592,17 @@ already_AddRefed<Factory> Factory::Creat
         "NextRequestSerialNumber doesn't match!");
 #endif  // !DISABLE_ASSERTS_FOR_FUZZING
   } else {
     loggingInfo = new DatabaseLoggingInfo(aLoggingInfo);
     gLoggingInfoHashtable->Put(aLoggingInfo.backgroundChildLoggingId(),
                                loggingInfo);
   }
 
-  RefPtr<Factory> actor = new Factory(loggingInfo.forget());
-
-  return actor.forget();
+  return RefPtr<Factory>(new Factory(loggingInfo.forget())).forget();
 }
 
 void Factory::ActorDestroy(ActorDestroyReason aWhy) {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(!mActorDestroyed);
 
 #ifdef DEBUG
   mActorDestroyed = true;
@@ -12697,22 +12687,22 @@ Factory::AllocPBackgroundIDBFactoryReque
   if (NS_WARN_IF(!QuotaManager::IsPrincipalInfoValid(principalInfo))) {
     ASSERT_UNLESS_FUZZING();
     return nullptr;
   }
 
   RefPtr<ContentParent> contentParent =
       BackgroundParent::GetContentParent(Manager());
 
-  RefPtr<FactoryOp> actor;
-  if (aParams.type() == FactoryRequestParams::TOpenDatabaseRequestParams) {
-    actor = new OpenDatabaseOp(this, contentParent.forget(), *commonParams);
-  } else {
-    actor = new DeleteDatabaseOp(this, contentParent.forget(), *commonParams);
-  }
+  auto actor =
+      aParams.type() == FactoryRequestParams::TOpenDatabaseRequestParams
+          ? RefPtr<FactoryOp>(MakeRefPtr<OpenDatabaseOp>(
+                this, contentParent.forget(), *commonParams))
+          : MakeRefPtr<DeleteDatabaseOp>(this, contentParent.forget(),
+                                         *commonParams);
 
   gFactoryOps->AppendElement(actor);
 
   // Balanced in CleanupMetadata() which is/must always called by SendResults().
   IncreaseBusyCount();
 
   // Transfer ownership to IPDL.
   return actor.forget().take();
@@ -16663,19 +16653,17 @@ void QuotaClient::AbortOperationsForProc
 }
 
 void QuotaClient::StartIdleMaintenance() {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(!mShutdownRequested);
 
   mBackgroundThread = GetCurrentThreadEventTarget();
 
-  RefPtr<Maintenance> maintenance = new Maintenance(this);
-
-  mMaintenanceQueue.AppendElement(maintenance.forget());
+  mMaintenanceQueue.EmplaceBack(MakeRefPtr<Maintenance>(this));
   ProcessMaintenanceQueue();
 }
 
 void QuotaClient::StopIdleMaintenance() {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(!mShutdownRequested);
 
   if (mCurrentMaintenance) {
--- a/dom/indexedDB/IDBDatabase.cpp
+++ b/dom/indexedDB/IDBDatabase.cpp
@@ -294,19 +294,19 @@ void IDBDatabase::ExitSetVersionTransact
 
 void IDBDatabase::RevertToPreviousState() {
   AssertIsOnOwningThread();
   MOZ_ASSERT(RunningVersionChangeTransaction());
   MOZ_ASSERT(mPreviousSpec);
 
   // Hold the current spec alive until RefreshTransactionsSpecEnumerator has
   // finished!
-  nsAutoPtr<DatabaseSpec> currentSpec(mSpec.forget());
+  auto currentSpec = std::move(mSpec);
 
-  mSpec = mPreviousSpec.forget();
+  mSpec = std::move(mPreviousSpec);
 
   RefreshSpec(/* aMayDelete */ true);
 }
 
 void IDBDatabase::RefreshSpec(bool aMayDelete) {
   AssertIsOnOwningThread();
 
   for (auto iter = mTransactions.Iter(); !iter.Done(); iter.Next()) {