Bug 967005 - Report rejected promises when worker stops running. r=bent
authorNikhil Marathe <nsm.nikhil@gmail.com>
Wed, 12 Mar 2014 07:31:03 -0700
changeset 190390 30b7c90dca14e05f4adb63e16adf797f2b14f284
parent 190389 dcd632ee9a3e98997c40b3087b57ee1961bd162a
child 190391 fedc1d467cf319282e31233cd6026655182a6f3f
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent
bugs967005
milestone30.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 967005 - Report rejected promises when worker stops running. r=bent
dom/promise/Promise.cpp
dom/promise/Promise.h
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -180,21 +180,20 @@ public:
   }
 };
 
 // Promise
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(Promise)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Promise)
-  tmp->MaybeReportRejected();
+  tmp->MaybeReportRejectedOnce();
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mGlobal)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mResolveCallbacks);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mRejectCallbacks);
-  tmp->mResult = JS::UndefinedValue();
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(Promise)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mGlobal)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mResolveCallbacks);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRejectCallbacks);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
@@ -224,18 +223,17 @@ Promise::Promise(nsIGlobalObject* aGloba
   MOZ_ASSERT(mGlobal);
 
   mozilla::HoldJSObjects(this);
   SetIsDOMBinding();
 }
 
 Promise::~Promise()
 {
-  MaybeReportRejected();
-  mResult = JS::UndefinedValue();
+  MaybeReportRejectedOnce();
   mozilla::DropJSObjects(this);
 }
 
 JSObject*
 Promise::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aScope)
 {
   return PromiseBinding::Wrap(aCx, aScope, this);
 }
@@ -817,16 +815,19 @@ Promise::AppendCallbacks(PromiseCallback
 {
   if (aResolveCallback) {
     mResolveCallbacks.AppendElement(aResolveCallback);
   }
 
   if (aRejectCallback) {
     mHadRejectCallback = true;
     mRejectCallbacks.AppendElement(aRejectCallback);
+
+    // Now that there is a callback, we don't need to report anymore.
+    RemoveFeature();
   }
 
   // If promise's state is resolved, queue a task to process our resolve
   // callbacks with promise's result. If promise's state is rejected, queue a
   // task to process our reject callbacks with promise's result.
   if (mState != Pending && !mTaskPending) {
     if (MOZ_LIKELY(NS_IsMainThread())) {
       nsRefPtr<PromiseTask> task = new PromiseTask(this);
@@ -1053,19 +1054,57 @@ Promise::RunResolveTask(JS::Handle<JS::V
   // Resolve/RejectInternal rather than using the Maybe... forms. Stop SetState
   // from asserting.
   if (mState != Pending) {
     return;
   }
 
   SetResult(aValue);
   SetState(aState);
+
+  // If the Promise was rejected, and there is no reject handler already setup,
+  // watch for thread shutdown.
+  if (aState == PromiseState::Rejected &&
+      !mHadRejectCallback &&
+      !NS_IsMainThread()) {
+    WorkerPrivate* worker = GetCurrentThreadWorkerPrivate();
+    MOZ_ASSERT(worker);
+    worker->AssertIsOnWorkerThread();
+
+    mFeature = new PromiseReportRejectFeature(this);
+    if (NS_WARN_IF(!worker->AddFeature(worker->GetJSContext(), mFeature))) {
+      // Worker is shutting down, report rejection immediately since it is
+      // unlikely that reject callbacks will be added after this point.
+      MaybeReportRejected();
+    }
+  }
+
   RunTask();
 }
 
+void
+Promise::RemoveFeature()
+{
+  if (mFeature) {
+    WorkerPrivate* worker = GetCurrentThreadWorkerPrivate();
+    MOZ_ASSERT(worker);
+    worker->RemoveFeature(worker->GetJSContext(), mFeature);
+    mFeature = nullptr;
+  }
+}
+
+bool
+PromiseReportRejectFeature::Notify(JSContext* aCx, workers::Status aStatus)
+{
+  MOZ_ASSERT(aStatus > workers::Running);
+  mPromise->MaybeReportRejectedOnce();
+  // After this point, `this` has been deleted by RemoveFeature!
+  return true;
+}
+
 bool
 Promise::ArgumentToJSValue(const nsAString& aArgument,
                            JSContext* aCx,
                            JSObject* aScope,
                            JS::MutableHandle<JS::Value> aValue)
 {
   // XXXkhuey I'd love to use xpc::NonVoidStringToJsval here, but it requires
   // a non-const nsAString for silly reasons.
--- a/dom/promise/Promise.h
+++ b/dom/promise/Promise.h
@@ -13,33 +13,53 @@
 #include "mozilla/dom/BindingDeclarations.h"
 #include "nsCycleCollectionParticipant.h"
 #include "mozilla/dom/PromiseBinding.h"
 #include "mozilla/dom/TypedArray.h"
 #include "nsWrapperCache.h"
 #include "nsAutoPtr.h"
 #include "js/TypeDecls.h"
 
+#include "mozilla/dom/workers/bindings/WorkerFeature.h"
+
 class nsIGlobalObject;
 
 namespace mozilla {
 namespace dom {
 
 class AnyCallback;
 class PromiseCallback;
 class PromiseInit;
 class PromiseNativeHandler;
 
+class Promise;
+class PromiseReportRejectFeature : public workers::WorkerFeature
+{
+  // The Promise that owns this feature.
+  Promise* mPromise;
+
+public:
+  PromiseReportRejectFeature(Promise* aPromise)
+    : mPromise(aPromise)
+  {
+    MOZ_ASSERT(mPromise);
+  }
+
+  virtual bool
+  Notify(JSContext* aCx, workers::Status aStatus) MOZ_OVERRIDE;
+};
+
 class Promise MOZ_FINAL : public nsISupports,
                           public nsWrapperCache
 {
   friend class NativePromiseCallback;
   friend class PromiseResolverMixin;
   friend class PromiseResolverTask;
   friend class PromiseTask;
+  friend class PromiseReportRejectFeature;
   friend class RejectPromiseCallback;
   friend class ResolvePromiseCallback;
   friend class WorkerPromiseResolverTask;
   friend class WorkerPromiseTask;
   friend class WrapperPromiseCallback;
 
   ~Promise();
 
@@ -152,18 +172,25 @@ private:
                       Promise::PromiseState aState,
                       PromiseTaskSync aAsynchronous);
 
   void AppendCallbacks(PromiseCallback* aResolveCallback,
                        PromiseCallback* aRejectCallback);
 
   // If we have been rejected and our mResult is a JS exception,
   // report it to the error console.
+  // Use MaybeReportRejectedOnce() for actual calls.
   void MaybeReportRejected();
 
+  void MaybeReportRejectedOnce() {
+    MaybeReportRejected();
+    RemoveFeature();
+    mResult = JS::UndefinedValue();
+  }
+
   void MaybeResolveInternal(JSContext* aCx,
                             JS::Handle<JS::Value> aValue,
                             PromiseTaskSync aSync = AsyncTask);
   void MaybeRejectInternal(JSContext* aCx,
                            JS::Handle<JS::Value> aValue,
                            PromiseTaskSync aSync = AsyncTask);
 
   void ResolveInternal(JSContext* aCx,
@@ -266,25 +293,33 @@ private:
   CreateFunction(JSContext* aCx, JSObject* aParent, Promise* aPromise,
                 int32_t aTask);
 
   static JSObject*
   CreateThenableFunction(JSContext* aCx, Promise* aPromise, uint32_t aTask);
 
   void HandleException(JSContext* aCx);
 
+  void RemoveFeature();
+
   nsRefPtr<nsIGlobalObject> mGlobal;
 
   nsTArray<nsRefPtr<PromiseCallback> > mResolveCallbacks;
   nsTArray<nsRefPtr<PromiseCallback> > mRejectCallbacks;
 
   JS::Heap<JS::Value> mResult;
   PromiseState mState;
   bool mTaskPending;
   bool mHadRejectCallback;
 
   bool mResolvePending;
+
+  // If a rejected promise on a worker has no reject callbacks attached, it
+  // needs to know when the worker is shutting down, to report the error on the
+  // console before the worker's context is deleted. This feature is used for
+  // that purpose.
+  nsAutoPtr<PromiseReportRejectFeature> mFeature;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_Promise_h