Bug 1598164 - Remove uses of already_AddRefed. r=dom-workers-and-storage-reviewers,janv draft
authorSimon Giesecke <sgiesecke@mozilla.com>
Wed, 04 Dec 2019 12:51:17 +0000
changeset 2514408 710674fe9e6b8a520f8a6ec3b0b3ad4d1fe7d446
parent 2504100 97a03c24e865c8826fac62e76cd552c7c1e5d443
child 2514409 7d32f16c841f565534cd2163fba40a319ae679b2
push id459908
push userreviewbot
push dateWed, 04 Dec 2019 12:51:53 +0000
treeherdertry@dbe3fcb3ec01 [default view] [failures only]
reviewersdom-workers-and-storage-reviewers, janv
bugs1598164
milestone72.0a1
Bug 1598164 - Remove uses of already_AddRefed. r=dom-workers-and-storage-reviewers,janv Depends on D54322 Differential Revision: https://phabricator.services.mozilla.com/D55079 Differential Diff: PHID-DIFF-62uhzy37bsukgspbcmuv
dom/indexedDB/IDBTransaction.cpp
dom/indexedDB/IDBTransaction.h
--- a/dom/indexedDB/IDBTransaction.cpp
+++ b/dom/indexedDB/IDBTransaction.cpp
@@ -86,17 +86,18 @@ auto IDBTransaction::DoWithTransactionCh
   return mMode == Mode::VersionChange
              ? aFunc(*mBackgroundActor.mVersionChangeBackgroundActor)
              : aFunc(*mBackgroundActor.mNormalBackgroundActor);
 }
 
 IDBTransaction::IDBTransaction(IDBDatabase* const aDatabase,
                                const nsTArray<nsString>& aObjectStoreNames,
                                const Mode aMode, nsString aFilename,
-                               const uint32_t aLineNo, const uint32_t aColumn)
+                               const uint32_t aLineNo, const uint32_t aColumn,
+                               CreatedFromFactoryFunction /*aDummy*/)
     : DOMEventTargetHelper(aDatabase),
       mDatabase(aDatabase),
       mObjectStoreNames(aObjectStoreNames),
       mLoggingSerialNumber(GetIndexedDBThreadLocal()->NextTransactionSN(aMode)),
       mNextObjectStoreId(0),
       mNextIndexId(0),
       mAbortCode(NS_OK),
       mPendingRequestCount(0),
@@ -163,64 +164,65 @@ IDBTransaction::~IDBTransaction() {
   MOZ_ASSERT(!HasTransactionChild(),
              "SendDeleteMeInternal should have cleared!");
 
   ReleaseWrapper(this);
   mozilla::DropJSObjects(this);
 }
 
 // static
-already_AddRefed<IDBTransaction> IDBTransaction::CreateVersionChange(
+RefPtr<IDBTransaction> IDBTransaction::CreateVersionChange(
     IDBDatabase* const aDatabase,
     BackgroundVersionChangeTransactionChild* const aActor,
     IDBOpenDBRequest* const aOpenRequest, const int64_t aNextObjectStoreId,
     const int64_t aNextIndexId) {
   MOZ_ASSERT(aDatabase);
   aDatabase->AssertIsOnOwningThread();
   MOZ_ASSERT(aActor);
   MOZ_ASSERT(aOpenRequest);
   MOZ_ASSERT(aNextObjectStoreId > 0);
   MOZ_ASSERT(aNextIndexId > 0);
 
-  nsTArray<nsString> emptyObjectStoreNames;
+  const nsTArray<nsString> emptyObjectStoreNames;
 
   nsString filename;
   uint32_t lineNo, column;
   aOpenRequest->GetCallerLocation(filename, &lineNo, &column);
-  RefPtr<IDBTransaction> transaction =
-      new IDBTransaction(aDatabase, emptyObjectStoreNames, Mode::VersionChange,
-                         std::move(filename), lineNo, column);
+  auto transaction = MakeRefPtr<IDBTransaction>(
+      aDatabase, emptyObjectStoreNames, Mode::VersionChange,
+      std::move(filename), lineNo, column, CreatedFromFactoryFunction{});
 
   transaction->NoteActiveTransaction();
 
   transaction->mBackgroundActor.mVersionChangeBackgroundActor = aActor;
   transaction->mNextObjectStoreId = aNextObjectStoreId;
   transaction->mNextIndexId = aNextIndexId;
 
   aDatabase->RegisterTransaction(transaction);
   transaction->mRegistered = true;
 
-  return transaction.forget();
+  return transaction;
 }
 
 // static
-already_AddRefed<IDBTransaction> IDBTransaction::Create(
+RefPtr<IDBTransaction> IDBTransaction::Create(
     JSContext* const aCx, IDBDatabase* const aDatabase,
     const nsTArray<nsString>& aObjectStoreNames, const Mode aMode) {
   MOZ_ASSERT(aDatabase);
   aDatabase->AssertIsOnOwningThread();
   MOZ_ASSERT(!aObjectStoreNames.IsEmpty());
   MOZ_ASSERT(aMode == Mode::ReadOnly || aMode == Mode::ReadWrite ||
              aMode == Mode::ReadWriteFlush || aMode == Mode::Cleanup);
 
   nsString filename;
   uint32_t lineNo, column;
   IDBRequest::CaptureCaller(aCx, filename, &lineNo, &column);
-  RefPtr<IDBTransaction> transaction = new IDBTransaction(
-      aDatabase, aObjectStoreNames, aMode, std::move(filename), lineNo, column);
+  auto transaction = MakeRefPtr<IDBTransaction>(
+      aDatabase, aObjectStoreNames, aMode, std::move(filename), lineNo, column,
+      CreatedFromFactoryFunction{});
 
   if (!NS_IsMainThread()) {
     WorkerPrivate* const workerPrivate = GetCurrentThreadWorkerPrivate();
     MOZ_ASSERT(workerPrivate);
 
     workerPrivate->AssertIsOnWorkerThread();
 
     RefPtr<StrongWorkerRef> workerRef = StrongWorkerRef::Create(
@@ -247,17 +249,17 @@ already_AddRefed<IDBTransaction> IDBTran
   nsCOMPtr<nsIRunnable> runnable = do_QueryObject(transaction);
   nsContentUtils::AddPendingIDBTransaction(runnable.forget());
 
   transaction->mCreating = true;
 
   aDatabase->RegisterTransaction(transaction);
   transaction->mRegistered = true;
 
-  return transaction.forget();
+  return transaction;
 }
 
 // static
 IDBTransaction* IDBTransaction::GetCurrent() {
   using namespace mozilla::ipc;
 
   MOZ_ASSERT(BackgroundChild::GetForCurrentThread());
 
@@ -463,17 +465,17 @@ void IDBTransaction::GetCallerLocation(n
   MOZ_ASSERT(aLineNo);
   MOZ_ASSERT(aColumn);
 
   aFilename = mFilename;
   *aLineNo = mLineNo;
   *aColumn = mColumn;
 }
 
-already_AddRefed<IDBObjectStore> IDBTransaction::CreateObjectStore(
+RefPtr<IDBObjectStore> IDBTransaction::CreateObjectStore(
     const ObjectStoreSpec& aSpec) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(aSpec.metadata().id());
   MOZ_ASSERT(Mode::VersionChange == mMode);
   MOZ_ASSERT(mBackgroundActor.mVersionChangeBackgroundActor);
   MOZ_ASSERT(CanAcceptRequests());
 
 #ifdef DEBUG
@@ -495,17 +497,17 @@ already_AddRefed<IDBObjectStore> IDBTran
       mBackgroundActor.mVersionChangeBackgroundActor->SendCreateObjectStore(
           aSpec.metadata()));
 
   RefPtr<IDBObjectStore> objectStore = IDBObjectStore::Create(this, aSpec);
   MOZ_ASSERT(objectStore);
 
   mObjectStores.AppendElement(objectStore);
 
-  return objectStore.forget();
+  return objectStore;
 }
 
 void IDBTransaction::DeleteObjectStore(const int64_t aObjectStoreId) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(aObjectStoreId);
   MOZ_ASSERT(Mode::VersionChange == mMode);
   MOZ_ASSERT(mBackgroundActor.mVersionChangeBackgroundActor);
   MOZ_ASSERT(CanAcceptRequests());
@@ -583,29 +585,27 @@ void IDBTransaction::RenameIndex(IDBObje
   MOZ_ASSERT(CanAcceptRequests());
 
   MOZ_ALWAYS_TRUE(
       mBackgroundActor.mVersionChangeBackgroundActor->SendRenameIndex(
           aObjectStore->Id(), aIndexId, nsString(aName)));
 }
 
 void IDBTransaction::AbortInternal(const nsresult aAbortCode,
-                                   already_AddRefed<DOMException> aError) {
+                                   RefPtr<DOMException> aError) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(NS_FAILED(aAbortCode));
   MOZ_ASSERT(!IsCommittingOrFinished());
 
-  RefPtr<DOMException> error = aError;
-
   const bool isVersionChange = mMode == Mode::VersionChange;
   const bool needToSendAbort = mReadyState == ReadyState::Active && !mStarted;
 
   mAbortCode = aAbortCode;
   mReadyState = ReadyState::Finished;
-  mError = error.forget();
+  mError = std::move(aError);
 
   if (isVersionChange) {
     // If a version change transaction is aborted, we must revert the world
     // back to its previous state unless we're being invalidated after the
     // transaction already completed.
     if (!mDatabase->IsInvalidated()) {
       mDatabase->RevertToPreviousState();
     }
@@ -671,30 +671,32 @@ void IDBTransaction::Abort(IDBRequest* c
     // Already started (and maybe finished) the commit or abort so there is
     // nothing to do here.
     return;
   }
 
   ErrorResult rv;
   RefPtr<DOMException> error = aRequest->GetError(rv);
 
-  AbortInternal(aRequest->GetErrorCode(), error.forget());
+  // TODO: Do we deliberately ignore rv here? Isn't there a static analysis that
+  // prevents that?
+
+  AbortInternal(aRequest->GetErrorCode(), std::move(error));
 }
 
 void IDBTransaction::Abort(const nsresult aErrorCode) {
   AssertIsOnOwningThread();
 
   if (IsCommittingOrFinished()) {
     // Already started (and maybe finished) the commit or abort so there is
     // nothing to do here.
     return;
   }
 
-  RefPtr<DOMException> error = DOMException::Create(aErrorCode);
-  AbortInternal(aErrorCode, error.forget());
+  AbortInternal(aErrorCode, DOMException::Create(aErrorCode));
 }
 
 void IDBTransaction::Abort(ErrorResult& aRv) {
   AssertIsOnOwningThread();
 
   if (IsCommittingOrFinished()) {
     aRv = NS_ERROR_DOM_INDEXEDDB_NOT_ALLOWED_ERR;
     return;
@@ -840,30 +842,30 @@ IDBTransactionMode IDBTransaction::GetMo
 }
 
 DOMException* IDBTransaction::GetError() const {
   AssertIsOnOwningThread();
 
   return mError;
 }
 
-already_AddRefed<DOMStringList> IDBTransaction::ObjectStoreNames() const {
+RefPtr<DOMStringList> IDBTransaction::ObjectStoreNames() const {
   AssertIsOnOwningThread();
 
   if (mMode == Mode::VersionChange) {
     return mDatabase->ObjectStoreNames();
   }
 
-  RefPtr<DOMStringList> list = new DOMStringList();
+  auto list = MakeRefPtr<DOMStringList>();
   list->StringArray() = mObjectStoreNames;
-  return list.forget();
+  return list;
 }
 
-already_AddRefed<IDBObjectStore> IDBTransaction::ObjectStore(
-    const nsAString& aName, ErrorResult& aRv) {
+RefPtr<IDBObjectStore> IDBTransaction::ObjectStore(const nsAString& aName,
+                                                   ErrorResult& aRv) {
   AssertIsOnOwningThread();
 
   if (IsCommittingOrFinished()) {
     aRv.ThrowDOMException(
         NS_ERROR_DOM_INVALID_STATE_ERR,
         NS_LITERAL_CSTRING("Transaction is already committing or done."));
     return nullptr;
   }
@@ -901,17 +903,17 @@ already_AddRefed<IDBObjectStore> IDBTran
     objectStore = *foundIt;
   } else {
     objectStore = IDBObjectStore::Create(this, *spec);
     MOZ_ASSERT(objectStore);
 
     mObjectStores.AppendElement(objectStore);
   }
 
-  return objectStore.forget();
+  return objectStore;
 }
 
 NS_IMPL_ADDREF_INHERITED(IDBTransaction, DOMEventTargetHelper)
 NS_IMPL_RELEASE_INHERITED(IDBTransaction, DOMEventTargetHelper)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IDBTransaction)
   NS_INTERFACE_MAP_ENTRY(nsIRunnable)
 NS_INTERFACE_MAP_END_INHERITING(DOMEventTargetHelper)
--- a/dom/indexedDB/IDBTransaction.h
+++ b/dom/indexedDB/IDBTransaction.h
@@ -114,23 +114,23 @@ class IDBTransaction final : public DOME
   bool mNotedActiveTransaction;
 
 #ifdef DEBUG
   bool mSentCommitOrAbort;
   bool mFiredCompleteOrAbort;
 #endif
 
  public:
-  static already_AddRefed<IDBTransaction> CreateVersionChange(
+  static MOZ_MUST_USE RefPtr<IDBTransaction> CreateVersionChange(
       IDBDatabase* aDatabase,
       indexedDB::BackgroundVersionChangeTransactionChild* aActor,
       IDBOpenDBRequest* aOpenRequest, int64_t aNextObjectStoreId,
       int64_t aNextIndexId);
 
-  static already_AddRefed<IDBTransaction> Create(
+  static MOZ_MUST_USE RefPtr<IDBTransaction> Create(
       JSContext* aCx, IDBDatabase* aDatabase,
       const nsTArray<nsString>& aObjectStoreNames, Mode aMode);
 
   static IDBTransaction* GetCurrent();
 
   void AssertIsOnOwningThread() const
 #ifdef DEBUG
       ;
@@ -238,17 +238,17 @@ class IDBTransaction final : public DOME
   }
 
   // Only for use by ProfilerHelpers.h
   const nsTArray<nsString>& ObjectStoreNamesInternal() const {
     AssertIsOnOwningThread();
     return mObjectStoreNames;
   }
 
-  already_AddRefed<IDBObjectStore> CreateObjectStore(
+  MOZ_MUST_USE RefPtr<IDBObjectStore> CreateObjectStore(
       const indexedDB::ObjectStoreSpec& aSpec);
 
   void DeleteObjectStore(int64_t aObjectStoreId);
 
   void RenameObjectStore(int64_t aObjectStoreId, const nsAString& aName);
 
   void CreateIndex(IDBObjectStore* aObjectStore,
                    const indexedDB::IndexMetadata& aMetadata);
@@ -292,38 +292,43 @@ class IDBTransaction final : public DOME
 
   // Methods bound via WebIDL.
   IDBDatabase* Db() const { return Database(); }
 
   IDBTransactionMode GetMode(ErrorResult& aRv) const;
 
   DOMException* GetError() const;
 
-  already_AddRefed<IDBObjectStore> ObjectStore(const nsAString& aName,
-                                               ErrorResult& aRv);
+  MOZ_MUST_USE RefPtr<IDBObjectStore> ObjectStore(const nsAString& aName,
+                                                  ErrorResult& aRv);
 
   void Abort(ErrorResult& aRv);
 
   IMPL_EVENT_HANDLER(abort)
   IMPL_EVENT_HANDLER(complete)
   IMPL_EVENT_HANDLER(error)
 
-  already_AddRefed<DOMStringList> ObjectStoreNames() const;
+  MOZ_MUST_USE RefPtr<DOMStringList> ObjectStoreNames() const;
 
   // EventTarget
   void GetEventTargetParent(EventChainPreVisitor& aVisitor) override;
 
  private:
+  struct CreatedFromFactoryFunction {};
+
+ public:
   IDBTransaction(IDBDatabase* aDatabase,
                  const nsTArray<nsString>& aObjectStoreNames, Mode aMode,
-                 nsString aFilename, uint32_t aLineNo, uint32_t aColumn);
+                 nsString aFilename, uint32_t aLineNo, uint32_t aColumn,
+                 CreatedFromFactoryFunction aDummy);
+
+ private:
   ~IDBTransaction();
 
-  void AbortInternal(nsresult aAbortCode,
-                     already_AddRefed<DOMException> aError);
+  void AbortInternal(nsresult aAbortCode, RefPtr<DOMException> aError);
 
   void SendCommit();
 
   void SendAbort(nsresult aResultCode);
 
   void NoteActiveTransaction();
 
   void MaybeNoteInactiveTransaction();