Bug 1288768 - Better error reporting for network errors in workers, r=bz
authorAndrea Marchesini <amarchesini@mozilla.com>
Sat, 28 Jan 2017 15:39:24 +0100
changeset 331925 7cf28e24da9d2e3d6c2eb1bdf2c0e0d545e2e01b
parent 331924 114e9708ec62893bb43fd54663433318ec92cac4
child 331926 11b1bbf8ba948c12bd26ba250afcc2e6b04fc615
push id86399
push useramarchesini@mozilla.com
push dateWed, 01 Feb 2017 10:45:57 +0000
treeherdermozilla-inbound@11b1bbf8ba94 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1288768
milestone54.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 1288768 - Better error reporting for network errors in workers, r=bz
dom/events/Event.cpp
dom/events/Event.h
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
testing/web-platform/meta/fetch/nosniff/worker.html.ini
--- a/dom/events/Event.cpp
+++ b/dom/events/Event.cpp
@@ -397,18 +397,27 @@ Event::Init(mozilla::dom::EventTarget* a
 // static
 already_AddRefed<Event>
 Event::Constructor(const GlobalObject& aGlobal,
                    const nsAString& aType,
                    const EventInit& aParam,
                    ErrorResult& aRv)
 {
   nsCOMPtr<mozilla::dom::EventTarget> t = do_QueryInterface(aGlobal.GetAsSupports());
-  RefPtr<Event> e = new Event(t, nullptr, nullptr);
-  bool trusted = e->Init(t);
+  return Constructor(t, aType, aParam);
+}
+
+// static
+already_AddRefed<Event>
+Event::Constructor(EventTarget* aEventTarget,
+                   const nsAString& aType,
+                   const EventInit& aParam)
+{
+  RefPtr<Event> e = new Event(aEventTarget, nullptr, nullptr);
+  bool trusted = e->Init(aEventTarget);
   e->InitEvent(aType, aParam.mBubbles, aParam.mCancelable);
   e->SetTrusted(trusted);
   e->SetComposed(aParam.mComposed);
   return e.forget();
 }
 
 uint16_t
 Event::EventPhase() const
@@ -1202,17 +1211,17 @@ Event::Deserialize(const IPC::Message* a
   InitEvent(type, bubbles, cancelable);
   SetTrusted(trusted);
   SetComposed(composed);
 
   return true;
 }
 
 NS_IMETHODIMP_(void)
-Event::SetOwner(mozilla::dom::EventTarget* aOwner)
+Event::SetOwner(EventTarget* aOwner)
 {
   mOwner = nullptr;
 
   if (!aOwner) {
     return;
   }
 
   nsCOMPtr<nsINode> n = do_QueryInterface(aOwner);
--- a/dom/events/Event.h
+++ b/dom/events/Event.h
@@ -137,16 +137,20 @@ public:
   static CSSIntPoint GetScreenCoords(nsPresContext* aPresContext,
                                      WidgetEvent* aEvent,
                                      LayoutDeviceIntPoint aPoint);
   static CSSIntPoint GetOffsetCoords(nsPresContext* aPresContext,
                                      WidgetEvent* aEvent,
                                      LayoutDeviceIntPoint aPoint,
                                      CSSIntPoint aDefaultPoint);
 
+  static already_AddRefed<Event> Constructor(EventTarget* aEventTarget,
+                                             const nsAString& aType,
+                                             const EventInit& aParam);
+
   static already_AddRefed<Event> Constructor(const GlobalObject& aGlobal,
                                              const nsAString& aType,
                                              const EventInit& aParam,
                                              ErrorResult& aRv);
 
   // Implemented as xpidl method
   // void GetType(nsString& aRetval) {}
 
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -489,16 +489,92 @@ private:
       WorkerControlRunnable::PostRun(aCx, aWorkerPrivate, aRunResult);
       return;
     }
     // Don't do anything here as it's possible that aWorkerPrivate has been
     // deleted.
   }
 };
 
+class ReportCompileErrorRunnable final : public WorkerRunnable
+{
+public:
+  static void
+  CreateAndDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
+  {
+    MOZ_ASSERT(aWorkerPrivate);
+    aWorkerPrivate->AssertIsOnWorkerThread();
+
+    RefPtr<ReportCompileErrorRunnable> runnable =
+      new ReportCompileErrorRunnable(aCx, aWorkerPrivate);
+    runnable->Dispatch();
+  }
+
+private:
+  ReportCompileErrorRunnable(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
+    : WorkerRunnable(aWorkerPrivate, ParentThreadUnchangedBusyCount)
+  {
+    aWorkerPrivate->AssertIsOnWorkerThread();
+  }
+
+  void
+  PostDispatch(WorkerPrivate* aWorkerPrivate, bool aDispatchResult) override
+  {
+    aWorkerPrivate->AssertIsOnWorkerThread();
+
+    // Dispatch may fail if the worker was canceled, no need to report that as
+    // an error, so don't call base class PostDispatch.
+  }
+
+  bool
+  WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  {
+    if (aWorkerPrivate->IsFrozen() ||
+        aWorkerPrivate->IsParentWindowPaused()) {
+      MOZ_ASSERT(!IsDebuggerRunnable());
+      aWorkerPrivate->QueueRunnable(this);
+      return true;
+    }
+
+    if (aWorkerPrivate->IsSharedWorker()) {
+      aWorkerPrivate->BroadcastErrorToSharedWorkers(aCx, EmptyString(),
+                                                    EmptyString(),
+                                                    EmptyString(), 0, 0,
+                                                    JSREPORT_ERROR,
+                                                    /* isErrorEvent */ false);
+      return true;
+    }
+
+    if (aWorkerPrivate->IsServiceWorker()) {
+      RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+      if (swm) {
+        swm->HandleError(aCx, aWorkerPrivate->GetPrincipal(),
+                         aWorkerPrivate->WorkerName(),
+                         aWorkerPrivate->ScriptURL(),
+                         EmptyString(), EmptyString(), EmptyString(),
+                         0, 0, JSREPORT_ERROR, JSEXN_ERR);
+      }
+      return true;
+    }
+
+    if (!aWorkerPrivate->IsAcceptingEvents()) {
+      return true;
+    }
+
+    RefPtr<Event> event =
+      Event::Constructor(aWorkerPrivate, NS_LITERAL_STRING("error"),
+                         EventInit());
+    event->SetTrusted(true);
+
+    nsEventStatus status = nsEventStatus_eIgnore;
+    aWorkerPrivate->DispatchDOMEvent(nullptr, event, nullptr, &status);
+    return true;
+  }
+};
+
 class CompileScriptRunnable final : public WorkerRunnable
 {
   nsString mScriptURL;
 
 public:
   explicit CompileScriptRunnable(WorkerPrivate* aWorkerPrivate,
                                  const nsAString& aScriptURL)
   : WorkerRunnable(aWorkerPrivate),
@@ -532,19 +608,25 @@ private:
       // We never got as far as calling GetOrCreateGlobalScope, or it failed.
       // We have no way to enter a compartment, hence no sane way to report this
       // error.  :(
       rv.SuppressException();
       return false;
     }
 
     // Make sure to propagate exceptions from rv onto aCx, so that they will get
-    // reported after we return.  We do this for all failures on rv, because now
-    // we're using rv to track all the state we care about.
-    //
+    // reported after we return.  We want to propagate just JS exceptions,
+    // because all the other errors are handled when the script is loaded.
+    // See: https://dom.spec.whatwg.org/#concept-event-fire
+    if (rv.Failed() && !rv.IsJSException()) {
+      ReportCompileErrorRunnable::CreateAndDispatch(aCx, aWorkerPrivate);
+      rv.SuppressException();
+      return false;
+    }
+
     // This is a little dumb, but aCx is in the null compartment here because we
     // set it up that way in our Run(), since we had not created the global at
     // that point yet.  So we need to enter the compartment of our global,
     // because setting a pending exception on aCx involves wrapping into its
     // current compartment.  Luckily we have a global now.
     JSAutoCompartment ac(aCx, globalScope->GetGlobalJSObject());
     if (rv.MaybeSetPendingException(aCx)) {
       return false;
@@ -1112,17 +1194,18 @@ private:
         MOZ_ASSERT(!IsDebuggerRunnable());
         aWorkerPrivate->QueueRunnable(this);
         return true;
       }
 
       if (aWorkerPrivate->IsSharedWorker()) {
         aWorkerPrivate->BroadcastErrorToSharedWorkers(aCx, mMessage, mFilename,
                                                       mLine, mLineNumber,
-                                                      mColumnNumber, mFlags);
+                                                      mColumnNumber, mFlags,
+                                                      /* isErrorEvent */ true);
         return true;
       }
 
       // Service workers do not have a main thread parent global, so normal
       // worker error reporting will crash.  Instead, pass the error to
       // the ServiceWorkerManager to report on any controlled documents.
       if (aWorkerPrivate->IsServiceWorker()) {
         RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
@@ -3286,17 +3369,18 @@ template <class Derived>
 void
 WorkerPrivateParent<Derived>::BroadcastErrorToSharedWorkers(
                                                     JSContext* aCx,
                                                     const nsAString& aMessage,
                                                     const nsAString& aFilename,
                                                     const nsAString& aLine,
                                                     uint32_t aLineNumber,
                                                     uint32_t aColumnNumber,
-                                                    uint32_t aFlags)
+                                                    uint32_t aFlags,
+                                                    bool aIsErrorEvent)
 {
   AssertIsOnMainThread();
 
   if (JSREPORT_IS_WARNING(aFlags)) {
     // Don't fire any events anywhere.  Just log to console.
     // XXXbz should we log to all the consoles of all the relevant windows?
     LogErrorToConsole(aMessage, aFilename, aLine, aLineNumber, aColumnNumber,
                       aFlags, 0);
@@ -3317,41 +3401,52 @@ WorkerPrivateParent<Derived>::BroadcastE
   // multiple objects in a single window as well as objects in different
   // windows.
   for (size_t index = 0; index < sharedWorkers.Length(); index++) {
     RefPtr<SharedWorker>& sharedWorker = sharedWorkers[index];
 
     // May be null.
     nsPIDOMWindowInner* window = sharedWorker->GetOwner();
 
-    RootedDictionary<ErrorEventInit> errorInit(aCx);
-    errorInit.mBubbles = false;
-    errorInit.mCancelable = true;
-    errorInit.mMessage = aMessage;
-    errorInit.mFilename = aFilename;
-    errorInit.mLineno = aLineNumber;
-    errorInit.mColno = aColumnNumber;
-
-    RefPtr<ErrorEvent> errorEvent =
-      ErrorEvent::Constructor(sharedWorker, NS_LITERAL_STRING("error"),
-                              errorInit);
-    if (!errorEvent) {
+    RefPtr<Event> event;
+
+    if (aIsErrorEvent) {
+      RootedDictionary<ErrorEventInit> errorInit(aCx);
+      errorInit.mBubbles = false;
+      errorInit.mCancelable = true;
+      errorInit.mMessage = aMessage;
+      errorInit.mFilename = aFilename;
+      errorInit.mLineno = aLineNumber;
+      errorInit.mColno = aColumnNumber;
+
+      event = ErrorEvent::Constructor(sharedWorker, NS_LITERAL_STRING("error"),
+                                      errorInit);
+    } else {
+      event = Event::Constructor(sharedWorker, NS_LITERAL_STRING("error"),
+                                 EventInit());
+    }
+
+    if (!event) {
       ThrowAndReport(window, NS_ERROR_UNEXPECTED);
       continue;
     }
 
-    errorEvent->SetTrusted(true);
+    event->SetTrusted(true);
 
     bool defaultActionEnabled;
-    nsresult rv = sharedWorker->DispatchEvent(errorEvent, &defaultActionEnabled);
+    nsresult rv = sharedWorker->DispatchEvent(event, &defaultActionEnabled);
     if (NS_FAILED(rv)) {
       ThrowAndReport(window, rv);
       continue;
     }
 
+    if (!aIsErrorEvent) {
+      continue;
+    }
+
     if (defaultActionEnabled) {
       // Add the owning window to our list so that we will fire an error event
       // at it later.
       if (!windowActions.Contains(window)) {
         windowActions.AppendElement(WindowAction(window));
       }
     } else {
       size_t actionsIndex = windowActions.LastIndexOf(WindowAction(window));
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -398,17 +398,18 @@ public:
 
   void
   BroadcastErrorToSharedWorkers(JSContext* aCx,
                                 const nsAString& aMessage,
                                 const nsAString& aFilename,
                                 const nsAString& aLine,
                                 uint32_t aLineNumber,
                                 uint32_t aColumnNumber,
-                                uint32_t aFlags);
+                                uint32_t aFlags,
+                                bool aIsErrorEvent);
 
   void
   WorkerScriptLoaded();
 
   void
   QueueRunnable(nsIRunnable* aRunnable)
   {
     AssertIsOnParentThread();
deleted file mode 100644
--- a/testing/web-platform/meta/fetch/nosniff/worker.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[worker.html]
-  type: testharness
-  expected: ERROR