Bug 1330693 - Try to handle SW job double-completion better. r=ehsan a=gchang
authorBen Kelly <ben@wanderview.com>
Thu, 12 Jan 2017 09:24:00 +0100
changeset 353601 f987327e5c36f94610120cd97fed376e0447893f
parent 353600 6d03505155dd3d0f53f4679418f803712dd0f0b7
child 353602 d506e0d4d610a7c982aa71fbe28ade712ccdecdf
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan, gchang
bugs1330693
milestone52.0a2
Bug 1330693 - Try to handle SW job double-completion better. r=ehsan a=gchang
dom/workers/ServiceWorkerJob.cpp
--- a/dom/workers/ServiceWorkerJob.cpp
+++ b/dom/workers/ServiceWorkerJob.cpp
@@ -48,21 +48,21 @@ ServiceWorkerJob::IsEquivalentTo(Service
          mScriptSpec.Equals(aJob->mScriptSpec) &&
          mPrincipal->Equals(aJob->mPrincipal);
 }
 
 void
 ServiceWorkerJob::AppendResultCallback(Callback* aCallback)
 {
   AssertIsOnMainThread();
-  MOZ_ASSERT(mState != State::Finished);
-  MOZ_ASSERT(aCallback);
-  MOZ_ASSERT(mFinalCallback != aCallback);
+  MOZ_DIAGNOSTIC_ASSERT(mState != State::Finished);
+  MOZ_DIAGNOSTIC_ASSERT(aCallback);
+  MOZ_DIAGNOSTIC_ASSERT(mFinalCallback != aCallback);
   MOZ_ASSERT(!mResultCallbackList.Contains(aCallback));
-  MOZ_ASSERT(!mResultCallbacksInvoked);
+  MOZ_DIAGNOSTIC_ASSERT(!mResultCallbacksInvoked);
   mResultCallbackList.AppendElement(aCallback);
 }
 
 void
 ServiceWorkerJob::StealResultCallbacksFrom(ServiceWorkerJob* aJob)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(aJob);
@@ -79,24 +79,24 @@ ServiceWorkerJob::StealResultCallbacksFr
     AppendResultCallback(callback);
   }
 }
 
 void
 ServiceWorkerJob::Start(Callback* aFinalCallback)
 {
   AssertIsOnMainThread();
-  MOZ_ASSERT(!mCanceled);
+  MOZ_DIAGNOSTIC_ASSERT(!mCanceled);
 
-  MOZ_ASSERT(aFinalCallback);
-  MOZ_ASSERT(!mFinalCallback);
+  MOZ_DIAGNOSTIC_ASSERT(aFinalCallback);
+  MOZ_DIAGNOSTIC_ASSERT(!mFinalCallback);
   MOZ_ASSERT(!mResultCallbackList.Contains(aFinalCallback));
   mFinalCallback = aFinalCallback;
 
-  MOZ_ASSERT(mState == State::Initial);
+  MOZ_DIAGNOSTIC_ASSERT(mState == State::Initial);
   mState = State::Started;
 
   nsCOMPtr<nsIRunnable> runnable =
     NewRunnableMethod(this, &ServiceWorkerJob::AsyncExecute);
 
   // We may have to wait for the PBackground actor to be initialized
   // before proceeding.  We should always be able to get a ServiceWorkerManager,
   // however, since Start() should not be called during shutdown.
@@ -150,19 +150,19 @@ ServiceWorkerJob::~ServiceWorkerJob()
   MOZ_ASSERT(mState != State::Started);
   MOZ_ASSERT_IF(mState == State::Finished, mResultCallbacksInvoked);
 }
 
 void
 ServiceWorkerJob::InvokeResultCallbacks(ErrorResult& aRv)
 {
   AssertIsOnMainThread();
-  MOZ_ASSERT(mState == State::Started);
+  MOZ_DIAGNOSTIC_ASSERT(mState == State::Started);
 
-  MOZ_ASSERT(!mResultCallbacksInvoked);
+  MOZ_DIAGNOSTIC_ASSERT(!mResultCallbacksInvoked);
   mResultCallbacksInvoked = true;
 
   nsTArray<RefPtr<Callback>> callbackList;
   callbackList.SwapElements(mResultCallbackList);
 
   for (RefPtr<Callback>& callback : callbackList) {
     // The callback might consume an exception on the ErrorResult, so we need
     // to clone in order to maintain the error for the next callback.
@@ -182,17 +182,24 @@ ServiceWorkerJob::InvokeResultCallbacks(
   ErrorResult converted(aRv);
   InvokeResultCallbacks(converted);
 }
 
 void
 ServiceWorkerJob::Finish(ErrorResult& aRv)
 {
   AssertIsOnMainThread();
-  MOZ_ASSERT(mState == State::Started);
+
+  // Avoid double-completion because it can result on operating on cleaned
+  // up data.  This should not happen, though, so also assert to try to
+  // narrow down the causes.
+  MOZ_DIAGNOSTIC_ASSERT(mState == State::Started);
+  if (mState != State::Started) {
+    return;
+  }
 
   // Ensure that we only surface SecurityErr, TypeErr or InvalidStateErr to script.
   if (aRv.Failed() && !aRv.ErrorCodeIs(NS_ERROR_DOM_SECURITY_ERR) &&
                       !aRv.ErrorCodeIs(NS_ERROR_DOM_TYPE_ERR) &&
                       !aRv.ErrorCodeIs(NS_ERROR_DOM_INVALID_STATE_ERR)) {
 
     // Remove the old error code so we can replace it with a TypeError.
     aRv.SuppressException();
@@ -208,18 +215,21 @@ ServiceWorkerJob::Finish(ErrorResult& aR
   RefPtr<ServiceWorkerJob> kungFuDeathGrip = this;
 
   if (!mResultCallbacksInvoked) {
     InvokeResultCallbacks(aRv);
   }
 
   mState = State::Finished;
 
-  mFinalCallback->JobFinished(this, aRv);
-  mFinalCallback = nullptr;
+  MOZ_DIAGNOSTIC_ASSERT(mFinalCallback);
+  if (mFinalCallback) {
+    mFinalCallback->JobFinished(this, aRv);
+    mFinalCallback = nullptr;
+  }
 
   // The callback might not consume the error.
   aRv.SuppressException();
 
   // Async release this object to ensure that our caller methods complete
   // as well.
   NS_ReleaseOnMainThread(kungFuDeathGrip.forget(), true /* always proxy */);
 }