Bug 1016196 - Patch 1/2: Add Promise support in BluetoothReplyRunnable, r=echou

dom/bluetooth2/BluetoothAdapter.cpp

@@ -157,38 +157,6 @@ public:
   }
 };
 
-class EnableDisableAdapterTask : public BluetoothReplyRunnable
-{
-public:
-  EnableDisableAdapterTask(Promise* aPromise)
-    : BluetoothReplyRunnable(nullptr)
-    , mPromise(aPromise)
-  { }
-
-  bool
-  ParseSuccessfulReply(JS::MutableHandle<JS::Value> aValue)
-  {
-    /*
-     * It is supposed to be Promise<void> according to BluetoothAdapter.webidl,
-     * but we have to pass "true" since it is mandatory to pass an
-     * argument while calling MaybeResolve.
-     */
-    mPromise->MaybeResolve(true);
-    aValue.setUndefined();
-    return true;
-  }
-
-  void
-  ReleaseMembers()
-  {
-    BluetoothReplyRunnable::ReleaseMembers();
-    mPromise = nullptr;
-  }
-
-private:
-  nsRefPtr<Promise> mPromise;
-};
-
 static int kCreatePairedDeviceTimeout = 50000; // unit: msec
 
 BluetoothAdapter::BluetoothAdapter(nsPIDOMWindow* aWindow,
@@ -321,12 +289,8 @@ BluetoothAdapter::SetPropertyByValue(const BluetoothNamedValue& aValue)
     mJsDeviceAddresses = deviceAddresses;
     Root();
   } else {
-#ifdef DEBUG
-    nsCString warningMsg;
-    warningMsg.AssignLiteral("Not handling adapter property: ");
-    warningMsg.Append(NS_ConvertUTF16toUTF8(name));
-    BT_WARNING(warningMsg.get());
-#endif
+    BT_WARNING("Not handling adapter property: %s",
+               NS_ConvertUTF16toUTF8(name).get());
   }
 }
 
@@ -400,12 +364,8 @@ BluetoothAdapter::Notify(const BluetoothSignal& aData)
 
     DispatchTrustedEvent(event);
   } else {
-#ifdef DEBUG
-    nsCString warningMsg;
-    warningMsg.AssignLiteral("Not handling adapter signal: ");
-    warningMsg.Append(NS_ConvertUTF16toUTF8(aData.name()));
-    BT_WARNING(warningMsg.get());
-#endif
+    BT_WARNING("Not handling adapter signal: %s",
+               NS_ConvertUTF16toUTF8(aData.name()).get());
   }
 }
 
@@ -719,7 +679,7 @@ BluetoothAdapter::EnableDisable(bool aEnable)
 
   nsRefPtr<Promise> promise = new Promise(global);
 
-  // Make sure BluetoothService is available before modifying adapter state
+  // Ensure BluetoothService is available before modifying adapter state
   BluetoothService* bs = BluetoothService::Get();
   if (!bs) {
     promise->MaybeReject(NS_ERROR_NOT_AVAILABLE);
@@ -727,21 +687,24 @@ BluetoothAdapter::EnableDisable(bool aEnable)
   }
 
   if (aEnable) {
+    // Enable local adapter
     if (mState != BluetoothAdapterState::Disabled) {
       promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
       return promise.forget();
     }
     mState = BluetoothAdapterState::Enabling;
   } else {
+    // Disable local adapter
     if (mState != BluetoothAdapterState::Enabled) {
       promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
       return promise.forget();
     }
     mState = BluetoothAdapterState::Disabling;
   }
-
   // TODO: Fire attr changed event for this state change
-  nsRefPtr<BluetoothReplyRunnable> result = new EnableDisableAdapterTask(promise);
+
+  nsRefPtr<BluetoothReplyRunnable> result =
+    new BluetoothVoidReplyRunnable(nullptr /* DOMRequest */, promise);
 
   if(NS_FAILED(bs->EnableDisable(aEnable, result))) {
     promise->MaybeReject(NS_ERROR_DOM_OPERATION_ERR);

dom/bluetooth2/BluetoothReplyRunnable.cpp

@@ -8,12 +8,17 @@
 #include "BluetoothReplyRunnable.h"
 #include "DOMRequest.h"
 #include "mozilla/dom/bluetooth/BluetoothTypes.h"
+#include "mozilla/dom/Promise.h"
 #include "nsServiceManagerUtils.h"
 
+using namespace mozilla::dom;
+
 USING_BLUETOOTH_NAMESPACE
 
-BluetoothReplyRunnable::BluetoothReplyRunnable(nsIDOMDOMRequest* aReq)
+BluetoothReplyRunnable::BluetoothReplyRunnable(nsIDOMDOMRequest* aReq,
+                                               Promise* aPromise)
   : mDOMRequest(aReq)
+  , mPromise(aPromise)
 {}
 
 void
@@ -26,35 +31,58 @@ void
 BluetoothReplyRunnable::ReleaseMembers()
 {
   mDOMRequest = nullptr;
+  mPromise = nullptr;
 }
 
 BluetoothReplyRunnable::~BluetoothReplyRunnable()
 {}
 
 nsresult
-BluetoothReplyRunnable::FireReply(JS::Handle<JS::Value> aVal)
+BluetoothReplyRunnable::FireReplySuccess(JS::Handle<JS::Value> aVal)
 {
-  NS_ENSURE_TRUE(mDOMRequest, NS_OK);
+  MOZ_ASSERT(mReply->type() == BluetoothReply::TBluetoothReplySuccess);
 
-  nsCOMPtr<nsIDOMRequestService> rs =
-    do_GetService(DOMREQUEST_SERVICE_CONTRACTID);
-  NS_ENSURE_TRUE(rs, NS_ERROR_FAILURE);
+  // DOMRequest
+  if (mDOMRequest) {
+    nsCOMPtr<nsIDOMRequestService> rs =
+      do_GetService(DOMREQUEST_SERVICE_CONTRACTID);
+    NS_ENSURE_TRUE(rs, NS_ERROR_FAILURE);
 
-  return mReply->type() == BluetoothReply::TBluetoothReplySuccess ?
-    rs->FireSuccessAsync(mDOMRequest, aVal) :
-    rs->FireErrorAsync(mDOMRequest, mReply->get_BluetoothReplyError().error());
+    return rs->FireSuccessAsync(mDOMRequest, aVal);
+  }
+
+  // Promise
+  if (mPromise) {
+    mPromise->MaybeResolve(aVal);
+  }
+
+  return NS_OK;
 }
 
 nsresult
 BluetoothReplyRunnable::FireErrorString()
 {
-  NS_ENSURE_TRUE(mDOMRequest, NS_OK);
+  // DOMRequest
+  if (mDOMRequest) {
+    nsCOMPtr<nsIDOMRequestService> rs =
+      do_GetService(DOMREQUEST_SERVICE_CONTRACTID);
+    NS_ENSURE_TRUE(rs, NS_ERROR_FAILURE);
 
-  nsCOMPtr<nsIDOMRequestService> rs =
-    do_GetService("@mozilla.org/dom/dom-request-service;1");
-  NS_ENSURE_TRUE(rs, NS_ERROR_FAILURE);
+    return rs->FireErrorAsync(mDOMRequest, mErrorString);
+  }
 
-  return rs->FireErrorAsync(mDOMRequest, mErrorString);
+  // Promise
+  if (mPromise) {
+    /**
+     * Always reject with NS_ERROR_DOM_OPERATION_ERR.
+     *
+     * TODO: Return actual error result once bluetooth backend wraps
+     *       nsresult instead of error string.
+     */
+    mPromise->MaybeReject(NS_ERROR_DOM_OPERATION_ERR);
+  }
+
+  return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -63,35 +91,37 @@ BluetoothReplyRunnable::Run()
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mReply);
 
-  nsresult rv;
-
   AutoSafeJSContext cx;
   JS::Rooted<JS::Value> v(cx, JSVAL_VOID);
 
+  nsresult rv;
   if (mReply->type() != BluetoothReply::TBluetoothReplySuccess) {
-    rv = FireReply(v);
+    SetError(mReply->get_BluetoothReplyError().error());
+    rv = FireErrorString();
+  } else if (!ParseSuccessfulReply(&v)) {
+    rv = FireErrorString();
   } else {
-    if (!ParseSuccessfulReply(&v)) {
-      rv = FireErrorString();
-    } else {
-      rv = FireReply(v);
-    }
+    rv = FireReplySuccess(v);
   }
 
   if (NS_FAILED(rv)) {
-    BT_WARNING("Could not fire DOMRequest!");
+    BT_WARNING("Could not fire DOMRequest/Promise!");
   }
 
   ReleaseMembers();
   MOZ_ASSERT(!mDOMRequest,
-             "mDOMRequest still alive! Deriving class should call "
+             "mDOMRequest is still alive! Deriving class should call "
+             "BluetoothReplyRunnable::ReleaseMembers()!");
+  MOZ_ASSERT(!mPromise,
+             "mPromise is still alive! Deriving class should call "
              "BluetoothReplyRunnable::ReleaseMembers()!");
 
   return rv;
 }
 
-BluetoothVoidReplyRunnable::BluetoothVoidReplyRunnable(nsIDOMDOMRequest* aReq)
-  : BluetoothReplyRunnable(aReq)
+BluetoothVoidReplyRunnable::BluetoothVoidReplyRunnable(nsIDOMDOMRequest* aReq,
+                                                       Promise* aPromise)
+  : BluetoothReplyRunnable(aReq, aPromise)
 {}
 
 BluetoothVoidReplyRunnable::~BluetoothVoidReplyRunnable()

dom/bluetooth2/BluetoothReplyRunnable.h

@@ -14,6 +14,12 @@
 
 class nsIDOMDOMRequest;
 
+namespace mozilla {
+namespace dom {
+class Promise;
+}
+}
+
 BEGIN_BLUETOOTH_NAMESPACE
 
 class BluetoothReply;
@@ -23,7 +29,8 @@ class BluetoothReplyRunnable : public nsRunnable
 public:
   NS_DECL_NSIRUNNABLE
 
-  BluetoothReplyRunnable(nsIDOMDOMRequest* aReq);
+  BluetoothReplyRunnable(nsIDOMDOMRequest* aReq,
+                         Promise* aPromise = nullptr);
 
   void SetReply(BluetoothReply* aReply);
 
@@ -45,25 +52,33 @@ protected:
   nsAutoPtr<BluetoothReply> mReply;
 
 private:
-  nsresult FireReply(JS::Handle<JS::Value> aVal);
+  nsresult FireReplySuccess(JS::Handle<JS::Value> aVal);
   nsresult FireErrorString();
 
   /**
-   * mDOMRequest is nullptr for internal IPC that require no DOMRequest,
-   * e.g., GetAdaptersTask triggered by BluetoothManager
+   * Either mDOMRequest or mPromise is not nullptr to reply applications
+   * success or error string. One special case is internal IPC that require
+   * neither mDOMRequest nor mPromise to reply applications.
+   * E.g., GetAdaptersTask triggered by BluetoothManager
+   *
+   * TODO: remove mDOMRequest once all methods adopt Promise.
    */
   nsCOMPtr<nsIDOMDOMRequest> mDOMRequest;
+  nsRefPtr<Promise> mPromise;
+
   nsString mErrorString;
 };
 
 class BluetoothVoidReplyRunnable : public BluetoothReplyRunnable
 {
 public:
-  BluetoothVoidReplyRunnable(nsIDOMDOMRequest* aReq);
+  BluetoothVoidReplyRunnable(nsIDOMDOMRequest* aReq,
+                             Promise* aPromise = nullptr);
  ~BluetoothVoidReplyRunnable();
 
 protected:
-  virtual bool ParseSuccessfulReply(JS::MutableHandle<JS::Value> aValue) MOZ_OVERRIDE
+  virtual bool
+  ParseSuccessfulReply(JS::MutableHandle<JS::Value> aValue) MOZ_OVERRIDE
   {
     aValue.setUndefined();
     return true;