Bug 1542541 - FactoryOp::ActorDestroy needs to do cleanup if the actor was waiting for the PermissionRetry message; r=asuth a=pascalc
authorJan Varga <jan.varga@gmail.com>
Sat, 06 Apr 2019 23:27:07 +0200
changeset 526196 095f7046554c6134f2ecff417634301f6413e1da
parent 526195 5d798451164267c5f4a4b69684361bc028d3d07a
child 526197 09e4cb94ed9e2acde522fcd81f867ad25eaf2149
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth, pascalc
bugs1542541
milestone67.0
Bug 1542541 - FactoryOp::ActorDestroy needs to do cleanup if the actor was waiting for the PermissionRetry message; r=asuth a=pascalc Differential Revision: https://phabricator.services.mozilla.com/D26433
dom/indexedDB/ActorsParent.cpp
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@@ -6538,16 +6538,17 @@ class FactoryOp : public DatabaseOperati
 
   const CommonFactoryRequestParams mCommonParams;
   nsCString mSuffix;
   nsCString mGroup;
   nsCString mOrigin;
   nsCString mDatabaseId;
   nsString mDatabaseFilePath;
   State mState;
+  bool mWaitingForPermissionRetry;
   bool mEnforcingQuota;
   const bool mDeleting;
   bool mChromeWriteAccessAllowed;
   bool mFileHandleDisabled;
 
  public:
   void NoteDatabaseBlocked(Database* aDatabase);
 
@@ -6580,16 +6581,18 @@ class FactoryOp : public DatabaseOperati
     MOZ_ASSERT_IF(OperationMayProceed(),
                   mState == State::Initial || mState == State::Completed);
   }
 
   nsresult Open();
 
   nsresult ChallengePermission();
 
+  void PermissionRetry();
+
   nsresult RetryCheckPermission();
 
   nsresult DirectoryOpen();
 
   nsresult SendToIOThread();
 
   void WaitForTransactions();
 
@@ -18986,16 +18989,17 @@ FactoryOp::FactoryOp(Factory* aFactory,
                      const CommonFactoryRequestParams& aCommonParams,
                      bool aDeleting)
     : DatabaseOperationBase(aFactory->GetLoggingInfo()->Id(),
                             aFactory->GetLoggingInfo()->NextRequestSN()),
       mFactory(aFactory),
       mContentParent(std::move(aContentParent)),
       mCommonParams(aCommonParams),
       mState(State::Initial),
+      mWaitingForPermissionRetry(false),
       mEnforcingQuota(true),
       mDeleting(aDeleting),
       mChromeWriteAccessAllowed(false),
       mFileHandleDisabled(false) {
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aFactory);
   MOZ_ASSERT(!QuotaClient::IsShuttingDownOnBackgroundThread());
 }
@@ -19072,17 +19076,32 @@ nsresult FactoryOp::ChallengePermission(
 
   const PrincipalInfo& principalInfo = mCommonParams.principalInfo();
   MOZ_ASSERT(principalInfo.type() == PrincipalInfo::TContentPrincipalInfo);
 
   if (NS_WARN_IF(!SendPermissionChallenge(principalInfo))) {
     return NS_ERROR_FAILURE;
   }
 
-  return NS_OK;
+  mWaitingForPermissionRetry = true;
+
+  return NS_OK;
+}
+
+void FactoryOp::PermissionRetry() {
+  AssertIsOnOwningThread();
+  MOZ_ASSERT(mState == State::PermissionChallenge);
+
+  mContentParent = BackgroundParent::GetContentParent(Manager()->Manager());
+
+  mState = State::PermissionRetry;
+
+  mWaitingForPermissionRetry = false;
+
+  MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread(this));
 }
 
 nsresult FactoryOp::RetryCheckPermission() {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mState == State::PermissionRetry);
   MOZ_ASSERT(mCommonParams.principalInfo().type() ==
              PrincipalInfo::TContentPrincipalInfo);
 
@@ -19686,27 +19705,40 @@ void FactoryOp::DirectoryLockFailed() {
   mState = State::SendingResults;
   MOZ_ALWAYS_SUCCEEDS(Run());
 }
 
 void FactoryOp::ActorDestroy(ActorDestroyReason aWhy) {
   AssertIsOnBackgroundThread();
 
   NoteActorDestroyed();
+
+  // Assume ActorDestroy can happen at any time, so we can't probe the current
+  // state since mState can be modified on any thread (only one thread at a time
+  // based on the state machine).  However we can use mWaitingForPermissionRetry
+  // which is only touched on the owning thread.  If mWaitingForPermissionRetry
+  // is true, we can also modify mState since we are guaranteed that there are
+  // no pending runnables which would probe mState to decide what code needs to
+  // run (there shouldn't be any running runnables on other threads either).
+
+  if (mWaitingForPermissionRetry) {
+    PermissionRetry();
+  }
+
+  // We don't have to handle the case when mWaitingForPermissionRetry is not
+  // true since it means that either nothing has been initialized yet, so
+  // nothing to cleanup or there are pending runnables that will detect that the
+  // actor has been destroyed and cleanup accordingly.
 }
 
 mozilla::ipc::IPCResult FactoryOp::RecvPermissionRetry() {
   AssertIsOnOwningThread();
   MOZ_ASSERT(!IsActorDestroyed());
-  MOZ_ASSERT(mState == State::PermissionChallenge);
-
-  mContentParent = BackgroundParent::GetContentParent(Manager()->Manager());
-
-  mState = State::PermissionRetry;
-  MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread(this));
+
+  PermissionRetry();
 
   return IPC_OK();
 }
 
 OpenDatabaseOp::OpenDatabaseOp(Factory* aFactory,
                                already_AddRefed<ContentParent> aContentParent,
                                const CommonFactoryRequestParams& aParams)
     : FactoryOp(aFactory, std::move(aContentParent), aParams,