Bug 984048 - Patch 4 - Handle parse and uncaught errors. r=khuey

dom/promise/Promise.h

@@ -27,6 +27,7 @@ namespace dom {
 
 class AnyCallback;
 class DOMError;
+class ErrorEvent;
 class PromiseCallback;
 class PromiseInit;
 class PromiseNativeHandler;
@@ -94,6 +95,10 @@ public:
     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

dom/promise/PromiseCallback.cpp

@@ -278,8 +278,8 @@ WrapperPromiseCallback::Call(JSContext* aCx,
       }
 
       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;

dom/workers/ServiceWorkerManager.cpp

@@ -12,6 +12,7 @@
 #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"
@@ -112,6 +113,49 @@ UpdatePromise::RejectAllPromises(nsresult aRv)
   }
 }
 
+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;
@@ -178,6 +222,12 @@ public:
     AssertIsOnMainThread();
   }
 
+  const nsCString&
+  GetScriptSpec() const
+  {
+    return mScriptSpec;
+  }
+
   void
   Abort()
   {
@@ -475,6 +525,16 @@ ServiceWorkerManager::RejectUpdatePromiseObservers(ServiceWorkerRegistration* aR
   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.
@@ -589,6 +649,65 @@ ServiceWorkerManager::FinishFetch(ServiceWorkerRegistration* aRegistration,
   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)

dom/workers/ServiceWorkerManager.h

@@ -18,6 +18,8 @@
 #include "nsTArrayForwardDeclare.h"
 #include "nsTWeakRef.h"
 
+class nsIScriptError;
+
 namespace mozilla {
 namespace dom {
 namespace workers {
@@ -43,6 +45,7 @@ public:
   void AddPromise(Promise* aPromise);
   void ResolveAllPromises(const nsACString& aScriptSpec, const nsACString& aScope);
   void RejectAllPromises(nsresult aRv);
+  void RejectAllPromises(const ErrorEventInit& aErrorDesc);
 
   bool
   IsRejected() const
@@ -230,10 +233,25 @@ public:
   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();
 

dom/workers/WorkerPrivate.cpp

@@ -77,6 +77,7 @@
 #include "Principal.h"
 #include "RuntimeService.h"
 #include "ScriptLoader.h"
+#include "ServiceWorkerManager.h"
 #include "SharedWorker.h"
 #include "WorkerFeature.h"
 #include "WorkerRunnable.h"
@@ -1333,7 +1334,15 @@ private:
         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);

dom/workers/test/serviceworkers/mochitest.ini

@@ -3,6 +3,7 @@ support-files =
   worker.js
   worker2.js
   worker3.js
+  parse_error_worker.js
 
 [test_installation_simple.html]
 [test_navigator.html]

dom/workers/test/serviceworkers/parse_error_worker.js

@@ -0,0 +1,2 @@
+// intentional parse error.
+var foo = {;

dom/workers/test/serviceworkers/test_installation_simple.html

@@ -63,8 +63,8 @@
       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!");
     });
   }
 
@@ -95,6 +95,18 @@
       });
   }
 
+  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)
@@ -102,8 +114,8 @@
       .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();

js/src/jsapi-tests/testUncaughtError.cpp

@@ -4,7 +4,7 @@
 
 #include "jsapi-tests/tests.h"
 
-using JS::CreateTypeError;
+using JS::CreateError;
 using JS::Rooted;
 using JS::ObjectValue;
 using JS::Value;
@@ -22,7 +22,7 @@ BEGIN_TEST(testUncaughtError)
         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());

js/src/jsapi.h

@@ -4624,9 +4624,9 @@ 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);
 
 /************************************************************************/
 

js/src/jsexn.cpp

@@ -915,7 +915,7 @@ js_CopyErrorObject(JSContext *cx, Handle<ErrorObject*> err, HandleObject scope)
 }
 
 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)
 {
@@ -925,7 +925,7 @@ JS::CreateTypeError(JSContext *cx, HandleString stack, HandleString fileName,
         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;