Bug 1334480 - Don't try to update a nullptr registration. r=asuth a=jcristau
authorBen Kelly <ben@wanderview.com>
Fri, 27 Jan 2017 12:04:24 -0800
changeset 375737 1d9713abc2cfcbbb8e1724dbb55b07efab093e17
parent 375736 79c1118774f22b5d3707ffddd2c978defb7f7eb0
child 375738 0411363a0dc7d350ac731edc047e3c6f3ef51c1d
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth, jcristau
bugs1334480
milestone53.0a2
Bug 1334480 - Don't try to update a nullptr registration. r=asuth a=jcristau
dom/workers/ServiceWorkerPrivate.cpp
--- a/dom/workers/ServiceWorkerPrivate.cpp
+++ b/dom/workers/ServiceWorkerPrivate.cpp
@@ -388,16 +388,17 @@ class RegistrationUpdateRunnable : publi
   const bool mNeedTimeCheck;
 
 public:
   RegistrationUpdateRunnable(nsMainThreadPtrHandle<ServiceWorkerRegistrationInfo>& aRegistration,
                              bool aNeedTimeCheck)
     : mRegistration(aRegistration)
     , mNeedTimeCheck(aNeedTimeCheck)
   {
+    MOZ_DIAGNOSTIC_ASSERT(mRegistration);
   }
 
   NS_IMETHOD
   Run() override
   {
     if (mNeedTimeCheck) {
       mRegistration->MaybeScheduleTimeCheckAndUpdate();
     } else {
@@ -593,19 +594,22 @@ public:
     , mRegistration(aRegistration)
   {
     MOZ_DIAGNOSTIC_ASSERT(aRegistration);
   }
 
   void
   PostRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate, bool aRunResult)
   {
-    nsCOMPtr<nsIRunnable> runnable =
-      new RegistrationUpdateRunnable(mRegistration, true /* time check */);
-    aWorkerPrivate->DispatchToMainThread(runnable.forget());
+    // Sub-class PreRun() or WorkerRun() methods could clear our mRegistration.
+    if (mRegistration) {
+      nsCOMPtr<nsIRunnable> runnable =
+        new RegistrationUpdateRunnable(mRegistration, true /* time check */);
+      aWorkerPrivate->DispatchToMainThread(runnable.forget());
+    }
 
     ExtendableEventWorkerRunnable::PostRun(aCx, aWorkerPrivate, aRunResult);
   }
 };
 
 /*
  * Fires 'install' event on the ServiceWorkerGlobalScope. Modifies busy count
  * since it fires the event. This is ok since there can't be nested
@@ -1327,17 +1331,16 @@ ServiceWorkerPrivate::SendNotificationEv
 namespace {
 
 // Inheriting ExtendableEventWorkerRunnable so that the worker is not terminated
 // while handling the fetch event, though that's very unlikely.
 class FetchEventRunnable : public ExtendableFunctionalEventWorkerRunnable
                          , public nsIHttpHeaderVisitor {
   nsMainThreadPtrHandle<nsIInterceptedChannel> mInterceptedChannel;
   const nsCString mScriptSpec;
-  nsMainThreadPtrHandle<ServiceWorkerRegistrationInfo> mRegistration;
   nsTArray<nsCString> mHeaderNames;
   nsTArray<nsCString> mHeaderValues;
   nsCString mSpec;
   nsCString mFragment;
   nsCString mMethod;
   nsString mClientId;
   bool mIsReload;
   RequestCache mCacheMode;
@@ -1358,17 +1361,16 @@ public:
                      const nsACString& aScriptSpec,
                      nsMainThreadPtrHandle<ServiceWorkerRegistrationInfo>& aRegistration,
                      const nsAString& aDocumentId,
                      bool aIsReload)
     : ExtendableFunctionalEventWorkerRunnable(
         aWorkerPrivate, aKeepAliveToken, aRegistration)
     , mInterceptedChannel(aChannel)
     , mScriptSpec(aScriptSpec)
-    , mRegistration(aRegistration)
     , mClientId(aDocumentId)
     , mIsReload(aIsReload)
     , mCacheMode(RequestCache::Default)
     , mRequestMode(RequestMode::No_cors)
     , mRequestRedirect(RequestRedirect::Follow)
     // By default we set it to same-origin since normal HTTP fetches always
     // send credentials to same-origin websites unless explicitly forbidden.
     , mRequestCredentials(RequestCredentials::Same_origin)