Bug 984048 - Patch 4 - Handle parse and uncaught errors. r=khuey
☠☠ backed out by b969a37d51aa ☠ ☠
authorNikhil Marathe <nsm.nikhil@gmail.com>
Mon, 30 Jun 2014 13:31:02 -0700
changeset 191586 7d12b5d7446fcae7c64c75d2942998cadae8caa9
parent 191493 cddd70c049644abf1c0d1fca3502d16e88df6981
child 191587 bacfdc83424a680dc34ad10ebe645ec4e101cba1
push id27055
push usercbook@mozilla.com
push dateTue, 01 Jul 2014 12:01:46 +0000
treeherdermozilla-central@4a9353b5762d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs984048
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 984048 - Patch 4 - Handle parse and uncaught errors. r=khuey
dom/promise/Promise.h
dom/promise/PromiseCallback.cpp
dom/workers/ServiceWorkerManager.cpp
dom/workers/ServiceWorkerManager.h
dom/workers/WorkerPrivate.cpp
dom/workers/test/serviceworkers/mochitest.ini
dom/workers/test/serviceworkers/parse_error_worker.js
dom/workers/test/serviceworkers/test_installation_simple.html
js/src/jsapi-tests/testUncaughtError.cpp
js/src/jsapi.h
js/src/jsexn.cpp
--- a/dom/promise/Promise.h
+++ b/dom/promise/Promise.h
@@ -22,16 +22,17 @@
 
 class nsIGlobalObject;
 
 namespace mozilla {
 namespace dom {
 
 class AnyCallback;
 class DOMError;
+class ErrorEvent;
 class PromiseCallback;
 class PromiseInit;
 class PromiseNativeHandler;
 
 class Promise;
 class PromiseReportRejectFeature : public workers::WorkerFeature
 {
   // The Promise that owns this feature.
@@ -89,16 +90,20 @@ public:
   void MaybeResolve(const T& aArg) {
     MaybeSomething(aArg, &Promise::MaybeResolve);
   }
 
   inline void MaybeReject(nsresult aArg) {
     MOZ_ASSERT(NS_FAILED(aArg));
     MaybeSomething(aArg, &Promise::MaybeReject);
   }
+
+  inline void MaybeReject(ErrorEvent* aArg) {
+    MaybeSomething(aArg, &Promise::MaybeReject);
+  }
   // DO NOT USE MaybeRejectBrokenly with in new code.  Promises should be
   // rejected with Error instances.
   // Note: MaybeRejectBrokenly is a template so we can use it with DOMError
   // without instantiating the DOMError specialization of MaybeSomething in
   // every translation unit that includes this header, because that would
   // require use to include DOMError.h either here or in all those translation
   // units.
   template<typename T>
--- a/dom/promise/PromiseCallback.cpp
+++ b/dom/promise/PromiseCallback.cpp
@@ -273,18 +273,18 @@ WrapperPromiseCallback::Call(JSContext* 
           "then() cannot return same Promise that it resolves."));
       if (!message) {
         // Out of memory. Promise will stay unresolved.
         JS_ClearPendingException(aCx);
         return;
       }
 
       JS::Rooted<JS::Value> typeError(aCx);
-      if (!JS::CreateTypeError(aCx, stack, fn, lineNumber, 0,
-                               nullptr, message, &typeError)) {
+      if (!JS::CreateError(aCx, JSEXN_TYPEERR, stack, fn, lineNumber, 0,
+                           nullptr, message, &typeError)) {
         // Out of memory. Promise will stay unresolved.
         JS_ClearPendingException(aCx);
         return;
       }
 
       mNextPromise->RejectInternal(aCx, typeError, Promise::SyncTask);
       return;
     }
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -7,16 +7,17 @@
 #include "nsIDocument.h"
 #include "nsPIDOMWindow.h"
 
 #include "jsapi.h"
 
 #include "mozilla/Preferences.h"
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/DOMError.h"
+#include "mozilla/dom/ErrorEvent.h"
 
 #include "nsContentUtils.h"
 #include "nsCxPusher.h"
 #include "nsNetUtil.h"
 #include "nsProxyRelease.h"
 #include "nsTArray.h"
 
 #include "RuntimeService.h"
@@ -107,16 +108,59 @@ UpdatePromise::RejectAllPromises(nsresul
         do_QueryInterface(pendingPromise->GetParentObject());
       MOZ_ASSERT(window);
       nsRefPtr<DOMError> domError = new DOMError(window, aRv);
       pendingPromise->MaybeRejectBrokenly(domError);
     }
   }
 }
 
+void
+UpdatePromise::RejectAllPromises(const ErrorEventInit& aErrorDesc)
+{
+  MOZ_ASSERT(mState == Pending);
+  mState = Rejected;
+
+  nsTArray<nsTWeakRef<Promise>> array;
+  array.SwapElements(mPromises);
+  for (uint32_t i = 0; i < array.Length(); ++i) {
+    nsTWeakRef<Promise>& pendingPromise = array.ElementAt(i);
+    if (pendingPromise) {
+      // Since ServiceWorkerContainer is only exposed to windows we can be
+      // certain about this cast.
+      nsCOMPtr<nsIGlobalObject> go = do_QueryInterface(pendingPromise->GetParentObject());
+      MOZ_ASSERT(go);
+
+      AutoJSAPI jsapi;
+      jsapi.Init(go);
+
+      JSContext* cx = jsapi.cx();
+
+      JS::Rooted<JSString*> stack(cx, JS_GetEmptyString(JS_GetRuntime(cx)));
+
+      JS::Rooted<JS::Value> fnval(cx);
+      ToJSValue(cx, aErrorDesc.mFilename, &fnval);
+      JS::Rooted<JSString*> fn(cx, fnval.toString());
+
+      JS::Rooted<JS::Value> msgval(cx);
+      ToJSValue(cx, aErrorDesc.mMessage, &msgval);
+      JS::Rooted<JSString*> msg(cx, msgval.toString());
+
+      JS::Rooted<JS::Value> error(cx);
+      if (!JS::CreateError(cx, JSEXN_ERR, stack, fn, aErrorDesc.mLineno,
+                           aErrorDesc.mColno, nullptr, msg, &error)) {
+        pendingPromise->MaybeReject(NS_ERROR_FAILURE);
+        continue;
+      }
+
+      pendingPromise->MaybeReject(cx, error);
+    }
+  }
+}
+
 class FinishFetchOnMainThreadRunnable : public nsRunnable
 {
   nsMainThreadPtrHandle<ServiceWorkerUpdateInstance> mUpdateInstance;
 public:
   FinishFetchOnMainThreadRunnable
     (const nsMainThreadPtrHandle<ServiceWorkerUpdateInstance>& aUpdateInstance)
     : mUpdateInstance(aUpdateInstance)
   { }
@@ -173,16 +217,22 @@ public:
       // Capture the current script spec in case register() gets called.
       mScriptSpec(aRegistration->mScriptSpec),
       mWindow(aWindow),
       mAborted(false)
   {
     AssertIsOnMainThread();
   }
 
+  const nsCString&
+  GetScriptSpec() const
+  {
+    return mScriptSpec;
+  }
+
   void
   Abort()
   {
     MOZ_ASSERT(!mAborted);
     mAborted = true;
   }
 
   void
@@ -470,16 +520,26 @@ ServiceWorkerManager::RejectUpdatePromis
                                                    nsresult aRv)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(aRegistration->HasUpdatePromise());
   aRegistration->mUpdatePromise->RejectAllPromises(aRv);
   aRegistration->mUpdatePromise = nullptr;
 }
 
+void
+ServiceWorkerManager::RejectUpdatePromiseObservers(ServiceWorkerRegistration* aRegistration,
+                                                   const ErrorEventInit& aErrorDesc)
+{
+  AssertIsOnMainThread();
+  MOZ_ASSERT(aRegistration->HasUpdatePromise());
+  aRegistration->mUpdatePromise->RejectAllPromises(aErrorDesc);
+  aRegistration->mUpdatePromise = nullptr;
+}
+
 /*
  * Update() does not return the Promise that the spec says it should. Callers
  * may access the registration's (new) Promise after calling this method.
  */
 NS_IMETHODIMP
 ServiceWorkerManager::Update(ServiceWorkerRegistration* aRegistration,
                              nsPIDOMWindow* aWindow)
 {
@@ -585,16 +645,75 @@ ServiceWorkerManager::FinishFetch(Servic
 
   ResolveRegisterPromises(aRegistration, aRegistration->mScriptSpec);
 
   ServiceWorkerInfo info(aRegistration->mScriptSpec);
   Install(aRegistration, info);
 }
 
 void
+ServiceWorkerManager::HandleError(JSContext* aCx,
+                                  const nsACString& aScope,
+                                  const nsAString& aWorkerURL,
+                                  nsString aMessage,
+                                  nsString aFilename,
+                                  nsString aLine,
+                                  uint32_t aLineNumber,
+                                  uint32_t aColumnNumber,
+                                  uint32_t aFlags)
+{
+  AssertIsOnMainThread();
+
+  nsCOMPtr<nsIURI> uri;
+  nsresult rv = NS_NewURI(getter_AddRefs(uri), aScope, nullptr, nullptr);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+
+  nsCString domain;
+  rv = uri->GetHost(domain);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+
+  ServiceWorkerDomainInfo* domainInfo;
+  if (!mDomainMap.Get(domain, &domainInfo)) {
+    return;
+  }
+
+  nsCString scope;
+  scope.Assign(aScope);
+  nsRefPtr<ServiceWorkerRegistration> registration = domainInfo->GetRegistration(scope);
+  MOZ_ASSERT(registration);
+
+  ErrorEventInit init;
+  init.mMessage = aMessage;
+  init.mFilename = aFilename;
+  init.mLineno = aLineNumber;
+  init.mColno = aColumnNumber;
+
+  // If the worker was the one undergoing registration, we reject the promises,
+  // otherwise we fire events on the ServiceWorker instances.
+
+  // If there is an update in progress and the worker that errored is the same one
+  // that is being updated, it is a sufficient test for 'this worker is being
+  // registered'.
+  // FIXME(nsm): Except the case where an update is found for a worker, in
+  // which case we'll need some other association than simply the URL.
+  if (registration->mUpdateInstance &&
+      registration->mUpdateInstance->GetScriptSpec().Equals(NS_ConvertUTF16toUTF8(aWorkerURL))) {
+    RejectUpdatePromiseObservers(registration, init);
+    // We don't need to abort here since the worker has already run.
+    registration->mUpdateInstance = nullptr;
+  } else {
+    // FIXME(nsm): Bug 983497 Fire 'error' on ServiceWorkerContainers.
+  }
+}
+
+void
 ServiceWorkerManager::Install(ServiceWorkerRegistration* aRegistration,
                               ServiceWorkerInfo aServiceWorkerInfo)
 {
   // FIXME(nsm): Same bug, different patch.
 }
 
 NS_IMETHODIMP
 ServiceWorkerManager::CreateServiceWorkerForWindow(nsPIDOMWindow* aWindow,
--- a/dom/workers/ServiceWorkerManager.h
+++ b/dom/workers/ServiceWorkerManager.h
@@ -13,16 +13,18 @@
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/Promise.h"
 #include "nsClassHashtable.h"
 #include "nsDataHashtable.h"
 #include "nsRefPtrHashtable.h"
 #include "nsTArrayForwardDeclare.h"
 #include "nsTWeakRef.h"
 
+class nsIScriptError;
+
 namespace mozilla {
 namespace dom {
 namespace workers {
 
 class ServiceWorker;
 class ServiceWorkerUpdateInstance;
 
 /**
@@ -38,16 +40,17 @@ class UpdatePromise MOZ_FINAL
 {
 public:
   UpdatePromise();
   ~UpdatePromise();
 
   void AddPromise(Promise* aPromise);
   void ResolveAllPromises(const nsACString& aScriptSpec, const nsACString& aScope);
   void RejectAllPromises(nsresult aRv);
+  void RejectAllPromises(const ErrorEventInit& aErrorDesc);
 
   bool
   IsRejected() const
   {
     return mState == Rejected;
   }
 
 private:
@@ -226,19 +229,34 @@ public:
   ResolveRegisterPromises(ServiceWorkerRegistration* aRegistration,
                           const nsACString& aWorkerScriptSpec);
 
   void
   RejectUpdatePromiseObservers(ServiceWorkerRegistration* aRegistration,
                                nsresult aResult);
 
   void
+  RejectUpdatePromiseObservers(ServiceWorkerRegistration* aRegistration,
+                               const ErrorEventInit& aErrorDesc);
+
+  void
   FinishFetch(ServiceWorkerRegistration* aRegistration,
               nsPIDOMWindow* aWindow);
 
+  void
+  HandleError(JSContext* aCx,
+              const nsACString& aScope,
+              const nsAString& aWorkerURL,
+              nsString aMessage,
+              nsString aFilename,
+              nsString aLine,
+              uint32_t aLineNumber,
+              uint32_t aColumnNumber,
+              uint32_t aFlags);
+
   static already_AddRefed<ServiceWorkerManager>
   GetInstance();
 
 private:
   ServiceWorkerManager();
   ~ServiceWorkerManager();
 
   NS_IMETHOD
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -72,16 +72,17 @@
 #endif
 
 #include "File.h"
 #include "MessagePort.h"
 #include "Navigator.h"
 #include "Principal.h"
 #include "RuntimeService.h"
 #include "ScriptLoader.h"
+#include "ServiceWorkerManager.h"
 #include "SharedWorker.h"
 #include "WorkerFeature.h"
 #include "WorkerRunnable.h"
 #include "WorkerScope.h"
 
 // JS_MaybeGC will run once every second during normal execution.
 #define PERIODIC_GC_TIMER_DELAY_SEC 1
 
@@ -1317,17 +1318,25 @@ private:
     else {
       AssertIsOnMainThread();
 
       if (aWorkerPrivate->IsSuspended()) {
         aWorkerPrivate->QueueRunnable(this);
         return true;
       }
 
-      if (aWorkerPrivate->IsSharedWorker() || aWorkerPrivate->IsServiceWorker()) {
+      if (aWorkerPrivate->IsServiceWorker()) {
+        nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+        MOZ_ASSERT(swm);
+        swm->HandleError(aCx, aWorkerPrivate->SharedWorkerName(),
+                         aWorkerPrivate->ScriptURL(),
+                         mMessage,
+                         mFilename, mLine, mLineNumber, mColumnNumber, mFlags);
+        return true;
+      } else if (aWorkerPrivate->IsSharedWorker()) {
         aWorkerPrivate->BroadcastErrorToSharedWorkers(aCx, mMessage, mFilename,
                                                       mLine, mLineNumber,
                                                       mColumnNumber, mFlags);
         return true;
       }
 
       aWorkerPrivate->AssertInnerWindowIsCorrect();
 
--- a/dom/workers/test/serviceworkers/mochitest.ini
+++ b/dom/workers/test/serviceworkers/mochitest.ini
@@ -1,8 +1,9 @@
 [DEFAULT]
 support-files =
   worker.js
   worker2.js
   worker3.js
+  parse_error_worker.js
 
 [test_installation_simple.html]
 [test_navigator.html]
new file mode 100644
--- /dev/null
+++ b/dom/workers/test/serviceworkers/parse_error_worker.js
@@ -0,0 +1,2 @@
+// intentional parse error.
+var foo = {;
--- a/dom/workers/test/serviceworkers/test_installation_simple.html
+++ b/dom/workers/test/serviceworkers/test_installation_simple.html
@@ -58,18 +58,18 @@
   function realWorker() {
     var p = navigator.serviceWorker.register("worker.js");
     return p.then(function(w) {
       ok(w instanceof ServiceWorker, "Register a ServiceWorker");
       info(w.scope);
       ok(w.scope == (new URL("/*", document.baseURI)).href, "Scope should match");
       ok(w.url == (new URL("worker.js", document.baseURI)).href, "URL should be of the worker");
     }, function(e) {
-      info(e.name);
-      ok(false, "Registration should have succeeded!");
+      info("Error: " + e.name);
+      ok(false, "realWorker Registration should have succeeded!");
     });
   }
 
   function abortPrevious() {
     var p = navigator.serviceWorker.register("worker2.js", { scope: "foo/*" });
     var q = navigator.serviceWorker.register("worker3.js", { scope: "foo/*" });
 
     return Promise.all([
@@ -90,25 +90,37 @@
   function networkError404() {
     return navigator.serviceWorker.register("404.js").then(function(w) {
         todo(false, "Should fail with NetworkError");
       }, function(e) {
         todo(e.name === "NetworkError", "Should fail with NetworkError");
       });
   }
 
+  function parseError() {
+    var p = navigator.serviceWorker.register("parse_error_worker.js");
+    return p.then(function(w) {
+      ok(false, "Registration should fail with parse error");
+    }, function(e) {
+    info("NSM " + e.name);
+      ok(e instanceof Error, "Registration should fail with parse error");
+    });
+  }
+
+  // FIXME(nsm): test for parse error when Update step doesn't happen (directly from register).
+
   function runTest() {
     simpleRegister()
       .then(sameOriginWorker)
       .then(sameOriginScope)
       .then(httpsOnly)
       .then(realWorker)
       .then(abortPrevious)
-      // FIXME(nsm): Uncomment once we have the error trapping patch from Bug 984048.
-      // .then(networkError404)
+      .then(networkError404)
+      .then(parseError)
       // put more tests here.
       .then(function() {
         SimpleTest.finish();
       }).catch(function(e) {
         ok(false, "Some test failed with error " + e);
         SimpleTest.finish();
       });
   }
--- a/js/src/jsapi-tests/testUncaughtError.cpp
+++ b/js/src/jsapi-tests/testUncaughtError.cpp
@@ -1,15 +1,15 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "jsapi-tests/tests.h"
 
-using JS::CreateTypeError;
+using JS::CreateError;
 using JS::Rooted;
 using JS::ObjectValue;
 using JS::Value;
 
 static size_t uncaughtCount = 0;
 
 BEGIN_TEST(testUncaughtError)
 {
@@ -17,17 +17,17 @@ BEGIN_TEST(testUncaughtError)
 
     CHECK(uncaughtCount == 0);
 
     Rooted<JSString*> empty(cx, JS_GetEmptyString(JS_GetRuntime(cx)));
     if (!empty)
         return false;
 
     Rooted<Value> err(cx);
-    if (!CreateTypeError(cx, empty, empty, 0, 0, nullptr, empty, &err))
+    if (!CreateError(cx, JSEXN_TYPEERR, empty, empty, 0, 0, nullptr, empty, &err))
         return false;
 
     Rooted<JSObject*> errObj(cx, &err.toObject());
     if (!JS_SetProperty(cx, errObj, "fileName", err))
         return false;
     if (!JS_SetProperty(cx, errObj, "lineNumber", err))
         return false;
     if (!JS_SetProperty(cx, errObj, "columnNumber", err))
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -4601,19 +4601,19 @@ extern JS_PUBLIC_API(JSErrorReporter)
 JS_GetErrorReporter(JSContext *cx);
 
 extern JS_PUBLIC_API(JSErrorReporter)
 JS_SetErrorReporter(JSContext *cx, JSErrorReporter er);
 
 namespace JS {
 
 extern JS_PUBLIC_API(bool)
-CreateTypeError(JSContext *cx, HandleString stack, HandleString fileName,
-                uint32_t lineNumber, uint32_t columnNumber, JSErrorReport *report,
-                HandleString message, MutableHandleValue rval);
+CreateError(JSContext *cx, JSExnType type, HandleString stack,
+            HandleString fileName, uint32_t lineNumber, uint32_t columnNumber,
+            JSErrorReport *report, HandleString message, MutableHandleValue rval);
 
 /************************************************************************/
 
 /*
  * Weak Maps.
  */
 
 extern JS_PUBLIC_API(JSObject *)
--- a/js/src/jsexn.cpp
+++ b/js/src/jsexn.cpp
@@ -910,26 +910,26 @@ js_CopyErrorObject(JSContext *cx, Handle
     JSExnType errorType = err->type();
 
     // Create the Error object.
     return ErrorObject::create(cx, errorType, stack, fileName,
                                lineNumber, columnNumber, &copyReport, message);
 }
 
 JS_PUBLIC_API(bool)
-JS::CreateTypeError(JSContext *cx, HandleString stack, HandleString fileName,
+JS::CreateError(JSContext *cx, JSExnType type, HandleString stack, HandleString fileName,
                     uint32_t lineNumber, uint32_t columnNumber, JSErrorReport *report,
                     HandleString message, MutableHandleValue rval)
 {
     assertSameCompartment(cx, stack, fileName, message);
     js::ScopedJSFreePtr<JSErrorReport> rep;
     if (report)
         rep = CopyErrorReport(cx, report);
 
     RootedObject obj(cx,
-        js::ErrorObject::create(cx, JSEXN_TYPEERR, stack, fileName,
+        js::ErrorObject::create(cx, type, stack, fileName,
                                 lineNumber, columnNumber, &rep, message));
     if (!obj)
         return false;
 
     rval.setObject(*obj);
     return true;
 }