Bug 1217909 P3 Refactor service worker register()/update() to reject only with SecurityErr or TypeErr. r=catalinb a=ritu
authorBen Kelly <ben@wanderview.com>
Wed, 18 Nov 2015 13:07:42 -0800
changeset 305545 9277b3f25b27473cd47c7c25cafd2f8dbd858ae8
parent 305544 cc73196f7906936efc95e55ad771555d1e6ff1e2
child 305546 ad71beeffbf075a853285488ed09a356a2e3c73d
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscatalinb, ritu
bugs1217909
milestone44.0a2
Bug 1217909 P3 Refactor service worker register()/update() to reject only with SecurityErr or TypeErr. r=catalinb a=ritu * * * Bug 1217909 P3 interdiff 001 fix try build
dom/bindings/Errors.msg
dom/workers/ServiceWorkerManager.cpp
dom/workers/ServiceWorkerManager.h
dom/workers/ServiceWorkerRegistration.cpp
--- a/dom/bindings/Errors.msg
+++ b/dom/bindings/Errors.msg
@@ -78,8 +78,10 @@ MSG_DEF(MSG_INVALID_URL_SCHEME, 2, JSEXN
 MSG_DEF(MSG_RESPONSE_URL_IS_NULL, 0, JSEXN_TYPEERR, "Cannot set Response.finalURL when Response.url is null.")
 MSG_DEF(MSG_RESPONSE_HAS_VARY_STAR, 0, JSEXN_TYPEERR, "Invalid Response object with a 'Vary: *' header.")
 MSG_DEF(MSG_BAD_FORMDATA, 0, JSEXN_TYPEERR, "Could not parse content as FormData.")
 MSG_DEF(MSG_NO_ACTIVE_WORKER, 1, JSEXN_TYPEERR, "No active worker for scope {0}.")
 MSG_DEF(MSG_NOTIFICATION_PERMISSION_DENIED, 0, JSEXN_TYPEERR, "Permission to show Notification denied.")
 MSG_DEF(MSG_NOTIFICATION_NO_CONSTRUCTOR_IN_SERVICEWORKER, 0, JSEXN_TYPEERR, "Notification constructor cannot be used in ServiceWorkerGlobalScope. Use registration.showNotification() instead.")
 MSG_DEF(MSG_INVALID_SCOPE, 2, JSEXN_TYPEERR, "Invalid scope trying to resolve {0} with base URL {1}.")
 MSG_DEF(MSG_INVALID_KEYFRAME_OFFSETS, 0, JSEXN_TYPEERR, "Keyframes with specified offsets must be in order and all be in the range [0, 1].")
+MSG_DEF(MSG_SW_INSTALL_ERROR, 2, JSEXN_TYPEERR, "ServiceWorker script at {0} for scope {1} encountered an error during installation.")
+MSG_DEF(MSG_SW_SCRIPT_THREW, 2, JSEXN_TYPEERR, "ServiceWorker script at {0} for scope {1} threw an exception during script evaluation.")
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -549,56 +549,20 @@ public:
   {
     RefPtr<ServiceWorkerRegistrationMainThread> swr =
       new ServiceWorkerRegistrationMainThread(mWindow,
                                               NS_ConvertUTF8toUTF16(aInfo->mScope));
     mPromise->MaybeResolve(swr);
   }
 
   void
-  UpdateFailed(nsresult aStatus) override
+  UpdateFailed(ErrorResult& aStatus) override
   {
     mPromise->MaybeReject(aStatus);
   }
-
-  void
-  UpdateFailed(JSExnType aExnType, const ErrorEventInit& aErrorDesc) override
-  {
-    AutoJSAPI jsapi;
-    jsapi.Init(mWindow);
-
-    JSContext* cx = jsapi.cx();
-
-    JS::Rooted<JS::Value> fnval(cx);
-    if (!ToJSValue(cx, aErrorDesc.mFilename, &fnval)) {
-      JS_ClearPendingException(cx);
-      mPromise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
-      return;
-    }
-    JS::Rooted<JSString*> fn(cx, fnval.toString());
-
-    JS::Rooted<JS::Value> msgval(cx);
-    if (!ToJSValue(cx, aErrorDesc.mMessage, &msgval)) {
-      JS_ClearPendingException(cx);
-      mPromise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
-      return;
-    }
-    JS::Rooted<JSString*> msg(cx, msgval.toString());
-
-    JS::Rooted<JS::Value> error(cx);
-    if ((aExnType < JSEXN_ERR) ||
-        !JS::CreateError(cx, aExnType, nullptr, fn, aErrorDesc.mLineno,
-                         aErrorDesc.mColno, nullptr, msg, &error)) {
-      JS_ClearPendingException(cx);
-      mPromise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
-      return;
-    }
-
-    mPromise->MaybeReject(cx, error);
-  }
 };
 
 class ContinueUpdateRunnable final : public nsRunnable
 {
   nsMainThreadPtrHandle<nsISupports> mJob;
 public:
   explicit ContinueUpdateRunnable(const nsMainThreadPtrHandle<nsISupports> aJob)
     : mJob(aJob)
@@ -928,26 +892,22 @@ public:
 
   void
   ComparisonResult(nsresult aStatus, bool aInCacheAndEqual,
                    const nsAString& aNewCacheName,
                    const nsACString& aMaxScope) override
   {
     RefPtr<ServiceWorkerRegisterJob> kungFuDeathGrip = this;
     if (NS_WARN_IF(mCanceled)) {
-      Fail(NS_ERROR_DOM_TYPE_ERR);
+      Fail(NS_ERROR_DOM_ABORT_ERR);
       return;
     }
 
     if (NS_WARN_IF(NS_FAILED(aStatus))) {
-      if (aStatus == NS_ERROR_DOM_SECURITY_ERR) {
-        Fail(aStatus);
-      } else {
-        Fail(NS_ERROR_DOM_TYPE_ERR);
-      }
+      Fail(aStatus);
       return;
     }
 
     if (aInCacheAndEqual) {
       Succeed();
       Done(NS_OK);
       return;
     }
@@ -1043,31 +1003,83 @@ public:
     MOZ_ASSERT(aSwm);
     ServiceWorkerManager::RegistrationDataPerPrincipal* data;
     if (aSwm->mRegistrationInfos.Get(aScopeKey, &data)) {
       data->mSetOfScopesBeingUpdated.Remove(aScopeKey);
     }
     Fail(NS_ERROR_DOM_ABORT_ERR);
   }
 
-  // Public so our error handling code can use it.
+  // This MUST only be called when the job is still performing actions related
+  // to registration or update. After the spec resolves the update promise, use
+  // Done() with the failure code instead.
   // Callers MUST hold a strong ref before calling this!
   void
-  Fail(JSExnType aExnType, const ErrorEventInit& aError)
+  Fail(ErrorResult& aRv)
   {
     MOZ_ASSERT(mCallback);
     RefPtr<ServiceWorkerUpdateFinishCallback> callback = mCallback.forget();
     // With cancellation support, we may only be running with one reference
     // from another object like a stream loader or something.
-    // UpdateFailed may do something with that, so hold a ref to ourself since
-    // FailCommon relies on it.
-    // FailCommon does check for cancellation, but let's be safe here.
     RefPtr<ServiceWorkerRegisterJob> kungFuDeathGrip = this;
-    callback->UpdateFailed(aExnType, aError);
-    FailCommon(NS_ERROR_DOM_JS_EXCEPTION);
+
+    // Save off the plain error code to pass to Done() where its logged to
+    // stderr as a warning.
+    nsresult origStatus = static_cast<nsresult>(aRv.ErrorCodeAsInt());
+
+    // Ensure that we only surface SecurityErr or TypeErr to script.
+    if (aRv.Failed() && !aRv.ErrorCodeIs(NS_ERROR_DOM_SECURITY_ERR) &&
+                        !aRv.ErrorCodeIs(NS_ERROR_DOM_TYPE_ERR)) {
+
+      // Remove the old error code so we can replace it with a TypeError.
+      aRv.SuppressException();
+
+      // Depending on how the job was created and where we are in the
+      // state machine the spec and scope may be stored in different ways.
+      // Extract the current scope and script spec.
+      nsString scriptSpec;
+      nsString scope;
+      if (mRegistration) {
+        CopyUTF8toUTF16(mRegistration->mScriptSpec, scriptSpec);
+        CopyUTF8toUTF16(mRegistration->mScope, scope);
+      } else {
+        CopyUTF8toUTF16(mScriptSpec, scriptSpec);
+        CopyUTF8toUTF16(mScope, scope);
+      }
+
+      // Throw the type error with a generic error message.
+      aRv.ThrowTypeError<MSG_SW_INSTALL_ERROR>(&scriptSpec, &scope);
+    }
+
+    callback->UpdateFailed(aRv);
+
+    // In case the callback does not consume the exception
+    aRv.SuppressException();
+
+    mUpdateAndInstallInfo = nullptr;
+    if (mRegistration->mInstallingWorker) {
+      nsresult rv = serviceWorkerScriptCache::PurgeCache(mRegistration->mPrincipal,
+                                                         mRegistration->mInstallingWorker->CacheName());
+      if (NS_FAILED(rv)) {
+        NS_WARNING("Failed to purge the installing worker cache.");
+      }
+    }
+
+    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+    swm->MaybeRemoveRegistration(mRegistration);
+    // Ensures that the job can't do anything useful from this point on.
+    mRegistration = nullptr;
+    Done(origStatus);
+  }
+
+  void
+  Fail(nsresult aRv)
+  {
+    ErrorResult rv(aRv);
+    Fail(rv);
   }
 
   // Public so our error handling code can continue with a successful worker.
   void
   ContinueInstall()
   {
     // mRegistration will be null if we have already Fail()ed.
     if (!mRegistration) {
@@ -1196,55 +1208,16 @@ private:
   Succeed()
   {
     MOZ_ASSERT(mCallback);
     mCallback->UpdateSucceeded(mRegistration);
     mCallback = nullptr;
   }
 
   void
-  FailCommon(nsresult aRv)
-  {
-    mUpdateAndInstallInfo = nullptr;
-    if (mRegistration->mInstallingWorker) {
-      nsresult rv = serviceWorkerScriptCache::PurgeCache(mRegistration->mPrincipal,
-                                                         mRegistration->mInstallingWorker->CacheName());
-      if (NS_FAILED(rv)) {
-        NS_WARNING("Failed to purge the installing worker cache.");
-      }
-    }
-
-    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-    swm->MaybeRemoveRegistration(mRegistration);
-    // Ensures that the job can't do anything useful from this point on.
-    mRegistration = nullptr;
-    unused << NS_WARN_IF(NS_FAILED(aRv));
-    Done(aRv);
-  }
-
-  // This MUST only be called when the job is still performing actions related
-  // to registration or update. After the spec resolves the update promise, use
-  // Done() with the failure code instead.
-  // Callers MUST hold a strong ref before calling this!
-  void
-  Fail(nsresult aRv)
-  {
-    MOZ_ASSERT(mCallback);
-    RefPtr<ServiceWorkerUpdateFinishCallback> callback = mCallback.forget();
-    // With cancellation support, we may only be running with one reference
-    // from another object like a stream loader or something.
-    // UpdateFailed may do something with that, so hold a ref to ourself since
-    // FailCommon relies on it.
-    // FailCommon does check for cancellation, but let's be safe here.
-    RefPtr<ServiceWorkerRegisterJob> kungFuDeathGrip = this;
-    callback->UpdateFailed(aRv);
-    FailCommon(aRv);
-  }
-
-  void
   ContinueAfterInstallEvent(bool aInstallEventSuccess)
   {
     if (mCanceled) {
       return Done(NS_ERROR_DOM_ABORT_ERR);
     }
 
     if (!mRegistration->mInstallingWorker) {
       NS_WARNING("mInstallingWorker was null.");
@@ -2424,42 +2397,39 @@ ServiceWorkerManager::HandleError(JSCont
   }
 
   ServiceWorkerManager::RegistrationDataPerPrincipal* data;
   if (NS_WARN_IF(!mRegistrationInfos.Get(scopeKey, &data))) {
     return;
   }
 
   // If this is a failure, we may need to cancel an in-progress registration.
-  if (!JSREPORT_IS_WARNING(aFlags)) {
-    ServiceWorkerJob* job = nullptr;
-
-    if (data->mSetOfScopesBeingUpdated.Contains(aScope)) {
-      data->mSetOfScopesBeingUpdated.Remove(aScope);
-
-      ServiceWorkerJobQueue* queue = data->mJobQueues.Get(aScope);
-      MOZ_ASSERT(queue);
-      job = queue->Peek();
-    }
-
+  if (!JSREPORT_IS_WARNING(aFlags) &&
+      data->mSetOfScopesBeingUpdated.Contains(aScope)) {
+
+    data->mSetOfScopesBeingUpdated.Remove(aScope);
+
+    ServiceWorkerJobQueue* queue = data->mJobQueues.Get(aScope);
+    MOZ_ASSERT(queue);
+
+    ServiceWorkerJob* job = queue->Peek();
     if (job) {
       MOZ_ASSERT(job->IsRegisterJob());
       RefPtr<ServiceWorkerRegisterJob> regJob =
         static_cast<ServiceWorkerRegisterJob*>(job);
 
-      RootedDictionary<ErrorEventInit> init(aCx);
-      init.mMessage = aMessage;
-      init.mFilename = aFilename;
-      init.mLineno = aLineNumber;
-      init.mColno = aColumnNumber;
-
-      regJob->Fail(aExnType, init);
+      ErrorResult rv;
+      NS_ConvertUTF8toUTF16 scope(aScope);
+      rv.ThrowTypeError<MSG_SW_SCRIPT_THREW>(&aWorkerURL, &scope);
+      regJob->Fail(rv);
     }
   }
 
+  // Always report any uncaught exceptions or errors to the console of
+  // each client.
   ReportToAllClients(aScope, aMessage, aFilename, aLine, aLineNumber,
                      aColumnNumber, aFlags);
 }
 
 void
 ServiceWorkerRegistrationInfo::FinishActivate(bool aSuccess)
 {
   if (mPendingUninstall || !mActiveWorker) {
@@ -3356,16 +3326,33 @@ ServiceWorkerManager::SoftUpdate(const O
                                  const nsACString& aScope,
                                  ServiceWorkerUpdateFinishCallback* aCallback)
 {
   nsAutoCString scopeKey;
   aOriginAttributes.CreateSuffix(scopeKey);
   SoftUpdate(scopeKey, aScope, aCallback);
 }
 
+namespace {
+
+// Empty callback.  Only use when you really want to ignore errors.
+class EmptyUpdateFinishCallback final : public ServiceWorkerUpdateFinishCallback
+{
+public:
+  void
+  UpdateSucceeded(ServiceWorkerRegistrationInfo* aInfo) override
+  { }
+
+  void
+  UpdateFailed(ErrorResult& aStatus) override
+  { }
+};
+
+} // anonymous namespace
+
 void
 ServiceWorkerManager::SoftUpdate(const nsACString& aScopeKey,
                                  const nsACString& aScope,
                                  ServiceWorkerUpdateFinishCallback* aCallback)
 {
   RefPtr<ServiceWorkerRegistrationInfo> registration =
     GetRegistration(aScopeKey, aScope);
   if (NS_WARN_IF(!registration)) {
@@ -3394,17 +3381,17 @@ ServiceWorkerManager::SoftUpdate(const n
   registration->mScriptSpec = newest->ScriptSpec();
 
   ServiceWorkerJobQueue* queue =
     GetOrCreateJobQueue(aScopeKey, aScope);
   MOZ_ASSERT(queue);
 
   RefPtr<ServiceWorkerUpdateFinishCallback> cb(aCallback);
   if (!cb) {
-    cb = new ServiceWorkerUpdateFinishCallback();
+    cb = new EmptyUpdateFinishCallback();
   }
 
   // "Invoke Update algorithm, or its equivalent, with client, registration as
   // its argument."
   RefPtr<ServiceWorkerRegisterJob> job =
     new ServiceWorkerRegisterJob(queue, registration, cb);
   queue->Append(job);
 }
--- a/dom/workers/ServiceWorkerManager.h
+++ b/dom/workers/ServiceWorkerManager.h
@@ -146,26 +146,20 @@ class ServiceWorkerUpdateFinishCallback
 protected:
   virtual ~ServiceWorkerUpdateFinishCallback()
   { }
 
 public:
   NS_INLINE_DECL_REFCOUNTING(ServiceWorkerUpdateFinishCallback)
 
   virtual
-  void UpdateSucceeded(ServiceWorkerRegistrationInfo* aInfo)
-  { }
+  void UpdateSucceeded(ServiceWorkerRegistrationInfo* aInfo) = 0;
 
   virtual
-  void UpdateFailed(nsresult aStatus)
-  { }
-
-  virtual
-  void UpdateFailed(JSExnType aExnType, const ErrorEventInit& aDesc)
-  { }
+  void UpdateFailed(ErrorResult& aStatus) = 0;
 };
 
 /*
  * Wherever the spec treats a worker instance and a description of said worker
  * as the same thing; i.e. "Resolve foo with
  * _GetNewestWorker(serviceWorkerRegistration)", we represent the description
  * by this class and spawn a ServiceWorker in the right global when required.
  */
--- a/dom/workers/ServiceWorkerRegistration.cpp
+++ b/dom/workers/ServiceWorkerRegistration.cpp
@@ -285,49 +285,48 @@ public:
   }
 
   void
   UpdateSucceeded(ServiceWorkerRegistrationInfo* aRegistration) override
   {
     mPromise->MaybeResolve(JS::UndefinedHandleValue);
   }
 
-  using ServiceWorkerUpdateFinishCallback::UpdateFailed;
-
   void
-  UpdateFailed(nsresult aStatus) override
+  UpdateFailed(ErrorResult& aStatus) override
   {
     mPromise->MaybeReject(aStatus);
   }
 };
 
 class UpdateResultRunnable final : public WorkerRunnable
 {
   RefPtr<PromiseWorkerProxy> mPromiseProxy;
-  nsresult mStatus;
+  ErrorResult mStatus;
 
   ~UpdateResultRunnable()
   {}
 
 public:
-  UpdateResultRunnable(PromiseWorkerProxy* aPromiseProxy, nsresult aStatus)
+  UpdateResultRunnable(PromiseWorkerProxy* aPromiseProxy, ErrorResult& aStatus)
     : WorkerRunnable(aPromiseProxy->GetWorkerPrivate(), WorkerThreadModifyBusyCount)
     , mPromiseProxy(aPromiseProxy)
-    , mStatus(aStatus)
+    , mStatus(Move(aStatus))
   { }
 
   bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
     Promise* promise = mPromiseProxy->WorkerPromise();
-    if (NS_SUCCEEDED(mStatus)) {
-      promise->MaybeResolve(JS::UndefinedHandleValue);
+    if (mStatus.Failed()) {
+      promise->MaybeReject(mStatus);
     } else {
-      promise->MaybeReject(mStatus);
+      promise->MaybeResolve(JS::UndefinedHandleValue);
     }
+    mStatus.SuppressException();
     mPromiseProxy->CleanUp(aCx);
     return true;
   }
 };
 
 class WorkerThreadUpdateCallback final : public ServiceWorkerUpdateFinishCallback
 {
   RefPtr<PromiseWorkerProxy> mPromiseProxy;
@@ -341,29 +340,28 @@ public:
     : mPromiseProxy(aPromiseProxy)
   {
     AssertIsOnMainThread();
   }
 
   void
   UpdateSucceeded(ServiceWorkerRegistrationInfo* aRegistration) override
   {
-    Finish(NS_OK);
+    ErrorResult rv(NS_OK);
+    Finish(rv);
   }
 
-  using ServiceWorkerUpdateFinishCallback::UpdateFailed;
-
   void
-  UpdateFailed(nsresult aStatus) override
+  UpdateFailed(ErrorResult& aStatus) override
   {
     Finish(aStatus);
   }
 
   void
-  Finish(nsresult aStatus)
+  Finish(ErrorResult& aStatus)
   {
     if (!mPromiseProxy) {
       return;
     }
 
     RefPtr<PromiseWorkerProxy> proxy = mPromiseProxy.forget();
 
     MutexAutoLock lock(proxy->Lock());