Bug 1016560. Remove the footgun of rejecting promises with arbitrary objects. r=khuey
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 03 Jun 2014 11:38:38 -0400
changeset 205958 f23221132fe90c54d550fe26d6a7108c6a8f1cc7
parent 205957 43e4fecc8251545ac5f719d0559c0c19fe71a0b8
child 205959 62c85e7fc6adcc7da216f5e65c573569305a800d
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs1016560
milestone32.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 1016560. Remove the footgun of rejecting promises with arbitrary objects. r=khuey
dom/base/Navigator.cpp
dom/datastore/DataStoreService.cpp
dom/filesystem/CreateDirectoryTask.cpp
dom/filesystem/CreateFileTask.cpp
dom/filesystem/GetFileOrDirectoryTask.cpp
dom/filesystem/RemoveTask.cpp
dom/ipc/ContentChild.cpp
dom/promise/Promise.cpp
dom/promise/Promise.h
dom/telephony/Telephony.cpp
--- a/dom/base/Navigator.cpp
+++ b/dom/base/Navigator.cpp
@@ -1492,17 +1492,17 @@ Navigator::GetFeature(const nsAString& a
   nsRefPtr<Promise> p = new Promise(go);
 
 #if defined(XP_LINUX)
   if (aName.EqualsLiteral("hardware.memory")) {
     // with seccomp enabled, fopen() should be in a non-sandboxed process
     if (XRE_GetProcessType() == GeckoProcessType_Default) {
       uint32_t memLevel = mozilla::hal::GetTotalSystemMemoryLevel();
       if (memLevel == 0) {
-        p->MaybeReject(NS_LITERAL_STRING("Abnormal"));
+        p->MaybeReject(NS_ERROR_NOT_AVAILABLE);
         return p.forget();
       }
       p->MaybeResolve((int)memLevel);
     } else {
       mozilla::dom::ContentChild* cc =
         mozilla::dom::ContentChild::GetSingleton();
       nsRefPtr<Promise> ipcRef(p);
       cc->SendGetSystemMemory(reinterpret_cast<uint64_t>(ipcRef.forget().take()));
--- a/dom/datastore/DataStoreService.cpp
+++ b/dom/datastore/DataStoreService.cpp
@@ -145,17 +145,17 @@ RejectPromise(nsPIDOMWindow* aWindow, Pr
   if (aRv == NS_ERROR_DOM_SECURITY_ERR) {
     error = new DOMError(aWindow, NS_LITERAL_STRING("SecurityError"),
                          NS_LITERAL_STRING("Access denied"));
   } else {
     error = new DOMError(aWindow, NS_LITERAL_STRING("InternalError"),
                          NS_LITERAL_STRING("An error occurred"));
   }
 
-  aPromise->MaybeReject(error);
+  aPromise->MaybeRejectBrokenly(error);
 }
 
 void
 DeleteDatabase(const nsAString& aName,
                const nsAString& aManifestURL)
 {
   AssertIsInMainProcess();
   MOZ_ASSERT(NS_IsMainThread());
--- a/dom/filesystem/CreateDirectoryTask.cpp
+++ b/dom/filesystem/CreateDirectoryTask.cpp
@@ -117,17 +117,17 @@ CreateDirectoryTask::HandlerCallback()
   if (mFileSystem->IsShutdown()) {
     mPromise = nullptr;
     return;
   }
 
   if (HasError()) {
     nsRefPtr<DOMError> domError = new DOMError(mFileSystem->GetWindow(),
       mErrorValue);
-    mPromise->MaybeReject(domError);
+    mPromise->MaybeRejectBrokenly(domError);
     mPromise = nullptr;
     return;
   }
   nsRefPtr<Directory> dir = new Directory(mFileSystem, mTargetRealPath);
   mPromise->MaybeResolve(dir);
   mPromise = nullptr;
 }
 
--- a/dom/filesystem/CreateFileTask.cpp
+++ b/dom/filesystem/CreateFileTask.cpp
@@ -282,17 +282,17 @@ CreateFileTask::HandlerCallback()
   if (mFileSystem->IsShutdown()) {
     mPromise = nullptr;
     return;
   }
 
   if (HasError()) {
     nsRefPtr<DOMError> domError = new DOMError(mFileSystem->GetWindow(),
       mErrorValue);
-    mPromise->MaybeReject(domError);
+    mPromise->MaybeRejectBrokenly(domError);
     mPromise = nullptr;
     return;
   }
 
   mPromise->MaybeResolve(mTargetFile);
   mPromise = nullptr;
 }
 
--- a/dom/filesystem/GetFileOrDirectoryTask.cpp
+++ b/dom/filesystem/GetFileOrDirectoryTask.cpp
@@ -192,17 +192,17 @@ GetFileOrDirectoryTask::HandlerCallback(
   if (mFileSystem->IsShutdown()) {
     mPromise = nullptr;
     return;
   }
 
   if (HasError()) {
     nsRefPtr<DOMError> domError = new DOMError(mFileSystem->GetWindow(),
       mErrorValue);
-    mPromise->MaybeReject(domError);
+    mPromise->MaybeRejectBrokenly(domError);
     mPromise = nullptr;
     return;
   }
 
   if (mIsDirectory) {
     nsRefPtr<Directory> dir = new Directory(mFileSystem, mTargetRealPath);
     mPromise->MaybeResolve(dir);
     mPromise = nullptr;
--- a/dom/filesystem/RemoveTask.cpp
+++ b/dom/filesystem/RemoveTask.cpp
@@ -180,17 +180,17 @@ RemoveTask::HandlerCallback()
   if (mFileSystem->IsShutdown()) {
     mPromise = nullptr;
     return;
   }
 
   if (HasError()) {
     nsRefPtr<DOMError> domError = new DOMError(mFileSystem->GetWindow(),
       mErrorValue);
-    mPromise->MaybeReject(domError);
+    mPromise->MaybeRejectBrokenly(domError);
     mPromise = nullptr;
     return;
   }
 
   mPromise->MaybeResolve(mReturnValue);
   mPromise = nullptr;
 }
 
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -1468,17 +1468,17 @@ ContentChild::AddRemoteAlertObserver(con
 
 bool
 ContentChild::RecvSystemMemoryAvailable(const uint64_t& aGetterId,
                                         const uint32_t& aMemoryAvailable)
 {
     nsRefPtr<Promise> p = dont_AddRef(reinterpret_cast<Promise*>(aGetterId));
 
     if (!aMemoryAvailable) {
-        p->MaybeReject(NS_LITERAL_STRING("Abnormal"));
+        p->MaybeReject(NS_ERROR_NOT_AVAILABLE);
         return true;
     }
 
     p->MaybeResolve((int)aMemoryAvailable);
     return true;
 }
 
 bool
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -2,16 +2,17 @@
 /* vim: set ts=2 et sw=2 tw=80: */
 /* 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 "mozilla/dom/Promise.h"
 
 #include "jsfriendapi.h"
+#include "mozilla/dom/DOMError.h"
 #include "mozilla/dom/OwningNonNull.h"
 #include "mozilla/dom/PromiseBinding.h"
 #include "mozilla/CycleCollectedJSRuntime.h"
 #include "mozilla/Preferences.h"
 #include "PromiseCallback.h"
 #include "PromiseNativeHandler.h"
 #include "PromiseWorkerProxy.h"
 #include "nsContentUtils.h"
@@ -1426,10 +1427,20 @@ PromiseWorkerProxy::CleanUp(JSContext* a
   // Release the Promise and remove the PromiseWorkerProxy from the features of
   // the worker thread since the Promise has been resolved/rejected or the
   // worker thread has been cancelled.
   mWorkerPromise = nullptr;
   mWorkerPrivate->RemoveFeature(aCx, this);
   mCleanedUp = true;
 }
 
+// Specializations of MaybeRejectBrokenly we actually support.
+template<>
+void Promise::MaybeRejectBrokenly(const nsRefPtr<DOMError>& aArg) {
+  MaybeSomething(aArg, &Promise::MaybeReject);
+}
+template<>
+void Promise::MaybeRejectBrokenly(const nsAString& aArg) {
+  MaybeSomething(aArg, &Promise::MaybeReject);
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/promise/Promise.h
+++ b/dom/promise/Promise.h
@@ -21,16 +21,17 @@
 #include "mozilla/dom/workers/bindings/WorkerFeature.h"
 
 class nsIGlobalObject;
 
 namespace mozilla {
 namespace dom {
 
 class AnyCallback;
+class DOMError;
 class PromiseCallback;
 class PromiseInit;
 class PromiseNativeHandler;
 
 class Promise;
 class PromiseReportRejectFeature : public workers::WorkerFeature
 {
   // The Promise that owns this feature.
@@ -84,21 +85,31 @@ public:
   // Most DOM objects are handled already.  To add a new type T, add a
   // ToJSValue overload in ToJSValue.h.
   // aArg is a const reference so we can pass rvalues like integer constants
   template <typename T>
   void MaybeResolve(const T& aArg) {
     MaybeSomething(aArg, &Promise::MaybeResolve);
   }
 
-  // aArg is a const reference so we can pass rvalues like NS_ERROR_*
-  template <typename T>
-  void MaybeReject(const T& aArg) {
+  inline void MaybeReject(nsresult aArg) {
+    MOZ_ASSERT(NS_FAILED(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>
+  void MaybeRejectBrokenly(const T& aArg); // Not implemented by default; see
+                                           // specializations in the .cpp for
+                                           // the T values we support.
 
   // WebIDL
 
   nsIGlobalObject* GetParentObject() const
   {
     return mGlobal;
   }
 
--- a/dom/telephony/Telephony.cpp
+++ b/dom/telephony/Telephony.cpp
@@ -71,17 +71,17 @@ public:
     MOZ_ASSERT(mTelephony);
   }
 
   virtual ~Callback() {}
 
   NS_IMETHODIMP
   NotifyDialError(const nsAString& aError)
   {
-    mPromise->MaybeReject(aError);
+    mPromise->MaybeRejectBrokenly(aError);
     return NS_OK;
   }
 
   NS_IMETHODIMP
   NotifyDialSuccess(uint32_t aCallIndex)
   {
     nsRefPtr<TelephonyCall> call =
       mTelephony->CreateNewDialingCall(mServiceId, mNumber, aCallIndex);
@@ -250,31 +250,31 @@ Telephony::DialInternal(uint32_t aServic
   nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(GetOwner());
   if (!global) {
     return nullptr;
   }
 
   nsRefPtr<Promise> promise = new Promise(global);
 
   if (!IsValidNumber(aNumber) || !IsValidServiceId(aServiceId)) {
-    promise->MaybeReject(NS_LITERAL_STRING("InvalidAccessError"));
+    promise->MaybeReject(NS_ERROR_DOM_INVALID_ACCESS_ERR);
     return promise.forget();
   }
 
   // We only support one outgoing call at a time.
   if (HasDialingCall()) {
-    promise->MaybeReject(NS_LITERAL_STRING("InvalidStateError"));
+    promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
     return promise.forget();
   }
 
   nsCOMPtr<nsITelephonyCallback> callback =
     new Callback(this, promise, aServiceId, aNumber);
   nsresult rv = mService->Dial(aServiceId, aNumber, aIsEmergency, callback);
   if (NS_FAILED(rv)) {
-    promise->MaybeReject(NS_LITERAL_STRING("InvalidStateError"));
+    promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
     return promise.forget();
   }
 
   return promise.forget();
 }
 
 already_AddRefed<TelephonyCall>
 Telephony::CreateNewDialingCall(uint32_t aServiceId, const nsAString& aNumber,