Bug 1225470 - Report a message to the console when a service worker waitUntil() is rejected. r=baku, a=ritu
authorBen Kelly <ben@wanderview.com>
Thu, 19 Nov 2015 13:15:17 -0800
changeset 305654 3493d5fba50e5cc68c34e868200f5e1d312457f4
parent 305653 d574b73912fcebd71b66b1117bc44465a212561d
child 305655 b02dc2a2305ef53488f5d6204da185863b1b0d36
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)
reviewersbaku, ritu
bugs1225470
milestone44.0a2
Bug 1225470 - Report a message to the console when a service worker waitUntil() is rejected. r=baku, a=ritu
dom/bindings/Bindings.conf
dom/workers/ServiceWorkerEvents.cpp
dom/workers/ServiceWorkerEvents.h
dom/workers/ServiceWorkerPrivate.cpp
--- a/dom/bindings/Bindings.conf
+++ b/dom/bindings/Bindings.conf
@@ -482,16 +482,17 @@ DOMInterfaces = {
     'binaryNames': {
         'message': 'messageMoz',
     },
 },
 
 'ExtendableEvent': {
     'headerFile': 'mozilla/dom/ServiceWorkerEvents.h',
     'nativeType': 'mozilla::dom::workers::ExtendableEvent',
+    'implicitJSContext': [ 'waitUntil' ],
 },
 
 'FetchEvent': {
     'headerFile': 'ServiceWorkerEvents.h',
     'nativeType': 'mozilla::dom::workers::FetchEvent',
     'binaryNames': {
         'request': 'request_'
     },
--- a/dom/workers/ServiceWorkerEvents.cpp
+++ b/dom/workers/ServiceWorkerEvents.cpp
@@ -688,17 +688,19 @@ FetchEvent::RespondWith(JSContext* aCx, 
   mWaitToRespond = true;
   RefPtr<RespondWithHandler> handler =
     new RespondWithHandler(mChannel, mRequest->Mode(), ir->IsClientRequest(),
                            ir->IsNavigationRequest(), mScriptSpec,
                            NS_ConvertUTF8toUTF16(requestURL),
                            spec, line, column);
   aArg.AppendNativeHandler(handler);
 
-  WaitUntil(aArg, aRv);
+  // Append directly to the lifecycle promises array.  Don't call WaitUntil()
+  // because that will lead to double-reporting any errors.
+  mPromises.AppendElement(&aArg);
 }
 
 void
 FetchEvent::PreventDefault(JSContext* aCx)
 {
   MOZ_ASSERT(aCx);
 
   if (mPreventDefaultScriptSpec.IsEmpty()) {
@@ -730,39 +732,133 @@ FetchEvent::ReportCanceled()
   //nsString requestURL;
   //CopyUTF8toUTF16(url, requestURL);
 
   ::AsyncLog(mChannel.get(), mPreventDefaultScriptSpec,
              mPreventDefaultLineNumber, mPreventDefaultColumnNumber,
              NS_LITERAL_CSTRING("InterceptionCanceledWithURL"), &requestURL);
 }
 
+namespace {
+
+class WaitUntilHandler final : public PromiseNativeHandler
+{
+  WorkerPrivate* mWorkerPrivate;
+  const nsCString mScope;
+  nsCString mSourceSpec;
+  uint32_t mLine;
+  uint32_t mColumn;
+  nsString mRejectValue;
+
+  ~WaitUntilHandler()
+  {
+  }
+
+public:
+  NS_DECL_THREADSAFE_ISUPPORTS
+
+  WaitUntilHandler(WorkerPrivate* aWorkerPrivate, JSContext* aCx)
+    : mWorkerPrivate(aWorkerPrivate)
+    , mScope(mWorkerPrivate->WorkerName())
+    , mLine(0)
+    , mColumn(0)
+  {
+    mWorkerPrivate->AssertIsOnWorkerThread();
+
+    // Save the location of the waitUntil() call itself as a fallback
+    // in case the rejection value does not contain any location info.
+    nsJSUtils::GetCallingLocation(aCx, mSourceSpec, &mLine, &mColumn);
+  }
+
+  void ResolvedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override
+  {
+    // do nothing, we are only here to report errors
+  }
+
+  void RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override
+  {
+    mWorkerPrivate->AssertIsOnWorkerThread();
+
+    nsCString spec;
+    uint32_t line = 0;
+    uint32_t column = 0;
+    ExtractErrorValues(aCx, aValue, spec, &line, &column, mRejectValue);
+
+    // only use the extracted location if we found one
+    if (!spec.IsEmpty()) {
+      mSourceSpec = spec;
+      mLine = line;
+      mColumn = column;
+    }
+
+    nsCOMPtr<nsIRunnable> runnable =
+      NS_NewRunnableMethod(this, &WaitUntilHandler::ReportOnMainThread);
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
+      NS_DispatchToMainThread(runnable.forget())));
+  }
+
+  void
+  ReportOnMainThread()
+  {
+    AssertIsOnMainThread();
+    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+
+    // TODO: Make the error message a localized string. (bug 1222720)
+    nsString message;
+    message.AppendLiteral("Service worker event waitUntil() was passed a "
+                          "promise that rejected with '");
+    message.Append(mRejectValue);
+    message.AppendLiteral("'.");
+
+    // Note, there is a corner case where this won't report to the window
+    // that triggered the error.  Consider a navigation fetch event that
+    // rejects waitUntil() without holding respondWith() open.  In this case
+    // there is no controlling document yet, the window did call .register()
+    // because there is no documeny yet, and the navigation is no longer
+    // being intercepted.
+
+    swm->ReportToAllClients(mScope, message, NS_ConvertUTF8toUTF16(mSourceSpec),
+                            EmptyString(), mLine, mColumn,
+                            nsIScriptError::errorFlag);
+  }
+};
+
+NS_IMPL_ISUPPORTS0(WaitUntilHandler)
+
+} // anonymous namespace
+
 NS_IMPL_ADDREF_INHERITED(FetchEvent, ExtendableEvent)
 NS_IMPL_RELEASE_INHERITED(FetchEvent, ExtendableEvent)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(FetchEvent)
 NS_INTERFACE_MAP_END_INHERITING(ExtendableEvent)
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(FetchEvent, ExtendableEvent, mRequest)
 
 ExtendableEvent::ExtendableEvent(EventTarget* aOwner)
   : Event(aOwner, nullptr, nullptr)
 {
 }
 
 void
-ExtendableEvent::WaitUntil(Promise& aPromise, ErrorResult& aRv)
+ExtendableEvent::WaitUntil(JSContext* aCx, Promise& aPromise, ErrorResult& aRv)
 {
   MOZ_ASSERT(!NS_IsMainThread());
 
   if (EventPhase() == nsIDOMEvent::NONE) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
+  // Append our handler to each waitUntil promise separately so we
+  // can record the location in script where waitUntil was called.
+  RefPtr<WaitUntilHandler> handler =
+    new WaitUntilHandler(GetCurrentThreadWorkerPrivate(), aCx);
+  aPromise.AppendNativeHandler(handler);
+
   mPromises.AppendElement(&aPromise);
 }
 
 already_AddRefed<Promise>
 ExtendableEvent::GetPromise()
 {
   WorkerPrivate* worker = GetCurrentThreadWorkerPrivate();
   MOZ_ASSERT(worker);
--- a/dom/workers/ServiceWorkerEvents.h
+++ b/dom/workers/ServiceWorkerEvents.h
@@ -43,19 +43,19 @@ public:
   CancelChannelRunnable(nsMainThreadPtrHandle<nsIInterceptedChannel>& aChannel,
                         nsresult aStatus);
 
   NS_IMETHOD Run() override;
 };
 
 class ExtendableEvent : public Event
 {
+protected:
   nsTArray<RefPtr<Promise>> mPromises;
 
-protected:
   explicit ExtendableEvent(mozilla::dom::EventTarget* aOwner);
   ~ExtendableEvent() {}
 
 public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(ExtendableEvent, Event)
   NS_FORWARD_TO_EVENT
 
@@ -82,17 +82,17 @@ public:
               const EventInit& aOptions,
               ErrorResult& aRv)
   {
     nsCOMPtr<EventTarget> target = do_QueryInterface(aGlobal.GetAsSupports());
     return Constructor(target, aType, aOptions);
   }
 
   void
-  WaitUntil(Promise& aPromise, ErrorResult& aRv);
+  WaitUntil(JSContext* aCx, Promise& aPromise, ErrorResult& aRv);
 
   already_AddRefed<Promise>
   GetPromise();
 
   virtual ExtendableEvent* AsExtendableEvent() override
   {
     return this;
   }
--- a/dom/workers/ServiceWorkerPrivate.cpp
+++ b/dom/workers/ServiceWorkerPrivate.cpp
@@ -401,31 +401,20 @@ public:
     workerPrivate->AssertIsOnWorkerThread();
 
     mCallback->SetResult(false);
     nsresult rv = NS_DispatchToMainThread(mCallback);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       NS_RUNTIMEABORT("Failed to dispatch life cycle event handler.");
     }
 
-    JS::Rooted<JSObject*> obj(aCx, workerPrivate->GlobalScope()->GetWrapper());
-    JS::ExposeValueToActiveJS(aValue);
-
-    js::ErrorReport report(aCx);
-    if (NS_WARN_IF(!report.init(aCx, aValue))) {
-      JS_ClearPendingException(aCx);
-      return;
-    }
-
-    RefPtr<xpc::ErrorReport> xpcReport = new xpc::ErrorReport();
-    xpcReport->Init(report.report(), report.message(),
-                    /* aIsChrome = */ false, workerPrivate->WindowID());
-
-    RefPtr<AsyncErrorReporter> aer = new AsyncErrorReporter(xpcReport);
-    NS_DispatchToMainThread(aer);
+    // Note, all WaitUntil() rejections are reported to client consoles
+    // by the WaitUntilHandler in ServiceWorkerEvents.  This ensures that
+    // errors in non-lifecycle events like FetchEvent and PushEvent are
+    // reported properly.
   }
 };
 
 NS_IMPL_ISUPPORTS0(LifecycleEventPromiseHandler)
 
 bool
 LifecycleEventWorkerRunnable::DispatchLifecycleEvent(JSContext* aCx,
                                                      WorkerPrivate* aWorkerPrivate)