Bug 1410420 - Clear database actor's strong reference to IDBDatabase when opening of a database fails; r=asuth

dom/indexedDB/ActorsChild.cpp

@@ -1755,6 +1755,7 @@ BackgroundFactoryRequestChild::BackgroundFactoryRequestChild(
                                                uint64_t aRequestedVersion)
   : BackgroundRequestChildBase(aOpenRequest)
   , mFactory(aFactory)
+  , mDatabaseActor(nullptr)
   , mRequestedVersion(aRequestedVersion)
   , mIsDeleteOp(aIsDeleteOp)
 {
@@ -1779,6 +1780,15 @@ BackgroundFactoryRequestChild::GetOpenDBRequest() const
   return static_cast<IDBOpenDBRequest*>(mRequest.get());
 }
 
+void
+BackgroundFactoryRequestChild::SetDatabaseActor(BackgroundDatabaseChild* aActor)
+{
+  AssertIsOnOwningThread();
+  MOZ_ASSERT(!aActor || !mDatabaseActor);
+
+  mDatabaseActor = aActor;
+}
+
 bool
 BackgroundFactoryRequestChild::HandleResponse(nsresult aResponse)
 {
@@ -1790,6 +1800,11 @@ BackgroundFactoryRequestChild::HandleResponse(nsresult aResponse)
 
   DispatchErrorEvent(mRequest, aResponse);
 
+  if (mDatabaseActor) {
+    mDatabaseActor->ReleaseDOMObject();
+    MOZ_ASSERT(!mDatabaseActor);
+  }
+
   return true;
 }
 
@@ -1808,6 +1823,7 @@ BackgroundFactoryRequestChild::HandleResponse(
   IDBDatabase* database = databaseActor->GetDOMObject();
   if (!database) {
     databaseActor->EnsureDOMObject();
+    MOZ_ASSERT(mDatabaseActor);
 
     database = databaseActor->GetDOMObject();
     MOZ_ASSERT(database);
@@ -1815,6 +1831,8 @@ BackgroundFactoryRequestChild::HandleResponse(
     MOZ_ASSERT(!database->IsClosed());
   }
 
+  MOZ_ASSERT(mDatabaseActor == databaseActor);
+
   if (database->IsClosed()) {
     // If the database was closed already, which is only possible if we fired an
     // "upgradeneeded" event, then we shouldn't fire a "success" event here.
@@ -1827,6 +1845,7 @@ BackgroundFactoryRequestChild::HandleResponse(
   }
 
   databaseActor->ReleaseDOMObject();
+  MOZ_ASSERT(!mDatabaseActor);
 
   return true;
 }
@@ -1847,6 +1866,8 @@ BackgroundFactoryRequestChild::HandleResponse(
 
   DispatchSuccessEvent(&helper, successEvent);
 
+  MOZ_ASSERT(!mDatabaseActor);
+
   return true;
 }
 
@@ -2095,6 +2116,8 @@ BackgroundDatabaseChild::EnsureDOMObject()
 
   mDatabase = mTemporaryStrongDatabase;
   mSpec.forget();
+
+  mOpenRequestActor->SetDatabaseActor(this);
 }
 
 void
@@ -2106,6 +2129,8 @@ BackgroundDatabaseChild::ReleaseDOMObject()
   MOZ_ASSERT(mOpenRequestActor);
   MOZ_ASSERT(mDatabase == mTemporaryStrongDatabase);
 
+  mOpenRequestActor->SetDatabaseActor(nullptr);
+
   mOpenRequestActor = nullptr;
 
   // This may be the final reference to the IDBDatabase object so we may end up

dom/indexedDB/ActorsChild.h

@@ -253,6 +253,20 @@ class BackgroundFactoryRequestChild final
   friend class PermissionRequestParent;
 
   RefPtr<IDBFactory> mFactory;
+
+  // Normally when opening of a database is successful, we receive a database
+  // actor in request response, so we can use it to call ReleaseDOMObject()
+  // which clears temporary strong reference to IDBDatabase.
+  // However, when there's an error, we don't receive a database actor and
+  // IDBRequest::mTransaction is already cleared (must be). So the only way how
+  // to call ReleaseDOMObject() is to have a back-reference to database actor.
+  // This creates a weak ref cycle between
+  // BackgroundFactoryRequestChild (using mDatabaseActor member) and
+  // BackgroundDatabaseChild actor (using mOpenRequestActor member).
+  // mDatabaseActor is set in EnsureDOMObject() and cleared in
+  // ReleaseDOMObject().
+  BackgroundDatabaseChild* mDatabaseActor;
+
   const uint64_t mRequestedVersion;
   const bool mIsDeleteOp;
 
@@ -270,6 +284,9 @@ private:
   // Only destroyed by BackgroundFactoryChild.
   ~BackgroundFactoryRequestChild();
 
+  void
+  SetDatabaseActor(BackgroundDatabaseChild* aActor);
+
   bool
   HandleResponse(nsresult aResponse);