Bug 1256428 P11 Don't coalesce SW jobs after the existing job has already resolved its promise. r=jdm

dom/workers/ServiceWorkerJob.cpp

@@ -32,6 +32,12 @@ ServiceWorkerJob2::Canceled() const
   return mCanceled;
 }
 
+bool
+ServiceWorkerJob2::ResultCallbacksInvoked() const
+{
+  return mResultCallbacksInvoked;
+}
+
 bool
 ServiceWorkerJob2::IsEquivalentTo(ServiceWorkerJob2* aJob) const
 {
@@ -51,7 +57,7 @@ ServiceWorkerJob2::AppendResultCallback(Callback* aCallback)
   MOZ_ASSERT(aCallback);
   MOZ_ASSERT(mFinalCallback != aCallback);
   MOZ_ASSERT(!mResultCallbackList.Contains(aCallback));
-  // TODO: handle the case where InvokeResultCallback() has already been called
+  MOZ_ASSERT(!mResultCallbacksInvoked);
   mResultCallbackList.AppendElement(aCallback);
 }
 
@@ -123,6 +129,7 @@ ServiceWorkerJob2::ServiceWorkerJob2(Type aType,
   , mScriptSpec(aScriptSpec)
   , mState(State::Initial)
   , mCanceled(false)
+  , mResultCallbacksInvoked(false)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(mPrincipal);
@@ -136,6 +143,7 @@ ServiceWorkerJob2::~ServiceWorkerJob2()
   // Jobs must finish or never be started.  Destroying an actively running
   // job is an error.
   MOZ_ASSERT(mState != State::Started);
+  MOZ_ASSERT_IF(mState == State::Finished, mResultCallbacksInvoked);
 }
 
 void
@@ -144,6 +152,9 @@ ServiceWorkerJob2::InvokeResultCallbacks(ErrorResult& aRv)
   AssertIsOnMainThread();
   MOZ_ASSERT(mState == State::Started);
 
+  MOZ_ASSERT(!mResultCallbacksInvoked);
+  mResultCallbacksInvoked = true;
+
   nsTArray<RefPtr<Callback>> callbackList;
   callbackList.SwapElements(mResultCallbackList);
 
@@ -191,7 +202,9 @@ ServiceWorkerJob2::Finish(ErrorResult& aRv)
   // The final callback may drop the last ref to this object.
   RefPtr<ServiceWorkerJob2> kungFuDeathGrip = this;
 
-  InvokeResultCallbacks(aRv);
+  if (!mResultCallbacksInvoked) {
+    InvokeResultCallbacks(aRv);
+  }
 
   mState = State::Finished;
 

dom/workers/ServiceWorkerJob.h

@@ -66,6 +66,11 @@ public:
   bool
   Canceled() const;
 
+  // Determine if the result callbacks have already been called.  This is
+  // equivalent to the spec checked to see if the job promise has settled.
+  bool
+  ResultCallbacksInvoked() const;
+
   bool
   IsEquivalentTo(ServiceWorkerJob2* aJob) const;
 
@@ -137,6 +142,7 @@ private:
   nsTArray<RefPtr<Callback>> mResultCallbackList;
   State mState;
   bool mCanceled;
+  bool mResultCallbacksInvoked;
 
 public:
   NS_INLINE_DECL_REFCOUNTING(ServiceWorkerJob2)

dom/workers/ServiceWorkerJobQueue.cpp

@@ -103,7 +103,7 @@ ServiceWorkerJobQueue2::ScheduleJob(ServiceWorkerJob2* aJob)
   MOZ_ASSERT(mJobList[0]->GetState() == ServiceWorkerJob2::State::Started);
 
   RefPtr<ServiceWorkerJob2>& tailJob = mJobList[mJobList.Length() - 1];
-  if (aJob->IsEquivalentTo(tailJob)) {
+  if (!tailJob->ResultCallbacksInvoked() && aJob->IsEquivalentTo(tailJob)) {
     tailJob->StealResultCallbacksFrom(aJob);
     return;
   }