Bug 1241278 - Change Notification.requestPermission() to return a promise. r=baku

--HG--
extra : commitid : Kl0YRQWwLkN
extra : rebase_source : ab9d18bad5b288f1c38ee2e1c7592d41f4dbaf7b

dom/notification/Notification.cpp

@@ -244,23 +244,27 @@ public:
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(NotificationPermissionRequest,
                                            nsIContentPermissionRequest)
 
-  NotificationPermissionRequest(nsIPrincipal* aPrincipal, nsPIDOMWindow* aWindow,
+  NotificationPermissionRequest(nsIPrincipal* aPrincipal,
+                                nsPIDOMWindow* aWindow, Promise* aPromise,
                                 NotificationPermissionCallback* aCallback)
     : mPrincipal(aPrincipal), mWindow(aWindow),
       mPermission(NotificationPermission::Default),
+      mPromise(aPromise),
       mCallback(aCallback)
   {
+    MOZ_ASSERT(aPromise);
     mRequester = new nsContentPermissionRequester(mWindow);
   }
 
 protected:
   virtual ~NotificationPermissionRequest() {}
 
-  nsresult CallCallback();
-  nsresult DispatchCallback();
+  nsresult ResolvePromise();
+  nsresult DispatchResolvePromise();
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsCOMPtr<nsPIDOMWindow> mWindow;
   NotificationPermission mPermission;
+  RefPtr<Promise> mPromise;
   RefPtr<NotificationPermissionCallback> mCallback;
   nsCOMPtr<nsIContentPermissionRequester> mRequester;
 };
@@ -541,7 +545,7 @@ protected:
 
 uint32_t Notification::sCount = 0;
 
-NS_IMPL_CYCLE_COLLECTION(NotificationPermissionRequest, mWindow)
+NS_IMPL_CYCLE_COLLECTION(NotificationPermissionRequest, mWindow, mPromise)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(NotificationPermissionRequest)
   NS_INTERFACE_MAP_ENTRY(nsIContentPermissionRequest)
@@ -581,7 +585,7 @@ NotificationPermissionRequest::Run()
   }
 
   if (mPermission != NotificationPermission::Default) {
-    return DispatchCallback();
+    return DispatchResolvePromise();
   }
 
   return nsContentPermissionUtils::AskPermission(this, mWindow);
@@ -613,7 +617,7 @@ NS_IMETHODIMP
 NotificationPermissionRequest::Cancel()
 {
   mPermission = NotificationPermission::Denied;
-  return DispatchCallback();
+  return DispatchResolvePromise();
 }
 
 NS_IMETHODIMP
@@ -622,7 +626,7 @@ NotificationPermissionRequest::Allow(JS::HandleValue aChoices)
   MOZ_ASSERT(aChoices.isUndefined());
 
   mPermission = NotificationPermission::Granted;
-  return DispatchCallback();
+  return DispatchResolvePromise();
 }
 
 NS_IMETHODIMP
@@ -636,23 +640,24 @@ NotificationPermissionRequest::GetRequester(nsIContentPermissionRequester** aReq
 }
 
 inline nsresult
-NotificationPermissionRequest::DispatchCallback()
+NotificationPermissionRequest::DispatchResolvePromise()
 {
-  if (!mCallback) {
-    return NS_OK;
-  }
-
-  nsCOMPtr<nsIRunnable> callbackRunnable = NS_NewRunnableMethod(this,
-    &NotificationPermissionRequest::CallCallback);
-  return NS_DispatchToMainThread(callbackRunnable);
+  nsCOMPtr<nsIRunnable> resolveRunnable = NS_NewRunnableMethod(this,
+    &NotificationPermissionRequest::ResolvePromise);
+  return NS_DispatchToMainThread(resolveRunnable);
 }
 
 nsresult
-NotificationPermissionRequest::CallCallback()
+NotificationPermissionRequest::ResolvePromise()
 {
-  ErrorResult rv;
-  mCallback->Call(mPermission, rv);
-  return rv.StealNSResult();
+  nsresult rv = NS_OK;
+  if (mCallback) {
+    ErrorResult error;
+    mCallback->Call(mPermission, error);
+    rv = error.StealNSResult();
+  }
+  mPromise->MaybeResolve(mPermission);
+  return rv;
 }
 
 NS_IMETHODIMP
@@ -1835,7 +1840,7 @@ Notification::RequestPermissionEnabledForScope(JSContext* aCx, JSObject* /* unus
   return NS_IsMainThread();
 }
 
-void
+already_AddRefed<Promise>
 Notification::RequestPermission(const GlobalObject& aGlobal,
                                 const Optional<OwningNonNull<NotificationPermissionCallback> >& aCallback,
                                 ErrorResult& aRv)
@@ -1845,18 +1850,24 @@ Notification::RequestPermission(const GlobalObject& aGlobal,
   nsCOMPtr<nsIScriptObjectPrincipal> sop = do_QueryInterface(aGlobal.GetAsSupports());
   if (!sop) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
-    return;
+    return nullptr;
   }
   nsCOMPtr<nsIPrincipal> principal = sop->GetPrincipal();
 
+  nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(window);
+  RefPtr<Promise> promise = Promise::Create(global, aRv);
+  if (aRv.Failed()) {
+    return nullptr;
+  }
   NotificationPermissionCallback* permissionCallback = nullptr;
   if (aCallback.WasPassed()) {
     permissionCallback = &aCallback.Value();
   }
   nsCOMPtr<nsIRunnable> request =
-    new NotificationPermissionRequest(principal, window, permissionCallback);
+    new NotificationPermissionRequest(principal, window, promise, permissionCallback);
 
   NS_DispatchToMainThread(request);
+  return promise.forget();
 }
 
 // static

dom/notification/Notification.h

@@ -235,9 +235,10 @@ public:
 
   static bool RequestPermissionEnabledForScope(JSContext* aCx, JSObject* /* unused */);
 
-  static void RequestPermission(const GlobalObject& aGlobal,
-                                const Optional<OwningNonNull<NotificationPermissionCallback> >& aCallback,
-                                ErrorResult& aRv);
+  static already_AddRefed<Promise>
+  RequestPermission(const GlobalObject& aGlobal,
+                    const Optional<OwningNonNull<NotificationPermissionCallback> >& aCallback,
+                    ErrorResult& aRv);
 
   static NotificationPermission GetPermission(const GlobalObject& aGlobal,
                                               ErrorResult& aRv);

dom/tests/mochitest/notification/test_notification_basics.html

@@ -28,38 +28,51 @@
     },
 
     function () {
-      info("Test blank requestPermission");
+      info("Test requestPermission without callback");
       Notification.requestPermission();
     },
 
     function (done) {
       info("Test requestPermission deny");
-      NotificationTest.denyNotifications();
-      Notification.requestPermission(function(perm) {
+      function assertPermissionDenied(perm) {
         is(perm, "denied", "Permission should be denied.");
         is(Notification.permission, "denied", "Permission should be denied.");
-        done();
-      });
+      }
+      NotificationTest.denyNotifications();
+      Notification.requestPermission()
+        .then(assertPermissionDenied)
+        .then(_ => Notification.requestPermission(assertPermissionDenied))
+        .catch(err => {
+          ok(!err, "requestPermission should not reject promise");
+        })
+        .then(done);
     },
 
     function (done) {
       info("Test requestPermission grant");
-      NotificationTest.allowNotifications();
-      Notification.requestPermission(function (perm) {
+      function assertPermissionGranted(perm) {
         is(perm, "granted", "Permission should be granted.");
         is(Notification.permission, "granted", "Permission should be granted");
-        done();
-      });
+      }
+      NotificationTest.allowNotifications();
+      Notification.requestPermission()
+        .then(assertPermissionGranted)
+        .then(_ => Notification.requestPermission(assertPermissionGranted))
+        .catch(err => {
+          ok(!err, "requestPermission should not reject promise");
+        })
+        .then(done);
     },
 
-    function () {
+    function (done) {
       info("Test invalid requestPermission");
-      try {
-        Notification.requestPermission({});
-        ok(false, "Non callable arg to requestPermission should throw");
-      } catch (e) {
-        ok(true, "Non callable arg to requestPermission should throw");
-      }
+      Notification.requestPermission({})
+        .then(_ => {
+          ok(false, "Non callable arg to requestPermission should reject promise");
+        }, err => {
+          ok(true, "Non callable arg to requestPermission should reject promise");
+        })
+        .then(done);
     },
 
     function (done) {

dom/webidl/Notification.webidl

@@ -20,7 +20,7 @@ interface Notification : EventTarget {
   static readonly attribute NotificationPermission permission;
 
   [Throws, Func="mozilla::dom::Notification::RequestPermissionEnabledForScope"]
-  static void requestPermission(optional NotificationPermissionCallback permissionCallback);
+  static Promise<NotificationPermission> requestPermission(optional NotificationPermissionCallback permissionCallback);
 
   [Throws, Func="mozilla::dom::Notification::IsGetEnabled"]
   static Promise<sequence<Notification>> get(optional GetNotificationOptions filter);

testing/web-platform/tests/notifications/interfaces.html

@@ -20,7 +20,7 @@ typedef EventHandlerNonNull? EventHandler;
 [Constructor(DOMString title, optional NotificationOptions options)]
 interface Notification : EventTarget {
   static readonly attribute NotificationPermission permission;
-  static void requestPermission(optional NotificationPermissionCallback callback);
+  static Promise<NotificationPermission> requestPermission(optional NotificationPermissionCallback callback);
 
   attribute EventHandler onclick;
   attribute EventHandler onshow;