Bug 1241278 - Change `Notification.requestPermission()` to return a promise. r=baku draft
authorKit Cambridge <kcambridge@mozilla.com>
Wed, 20 Jan 2016 14:23:04 -0800
changeset 323991 e3aa97921a653ab6b73c54bad8b0b429c88ee865
parent 323526 6764bc656c1d146962d53710d734c2ac87c2306f
child 323992 26f5c01a4e7ace7516b908939c535fcb7acfcbb0
child 323993 8675e52991797a4832c2022a4802df5ed6c5b1a3
push id9817
push userkcambridge@mozilla.com
push dateThu, 21 Jan 2016 17:19:17 +0000
reviewersbaku
bugs1241278
milestone46.0a1
Bug 1241278 - Change `Notification.requestPermission()` to return a promise. r=baku
dom/notification/Notification.cpp
dom/notification/Notification.h
dom/tests/mochitest/notification/test_notification_basics.html
dom/webidl/Notification.webidl
testing/web-platform/tests/notifications/interfaces.html
--- a/dom/notification/Notification.cpp
+++ b/dom/notification/Notification.cpp
@@ -239,33 +239,37 @@ class NotificationPermissionRequest : pu
 {
 public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_NSICONTENTPERMISSIONREQUEST
   NS_DECL_NSIRUNNABLE
   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;
 };
 
 namespace {
 class ReleaseNotificationControlRunnable final : public MainThreadWorkerControlRunnable
 {
   Notification* mNotification;
@@ -536,17 +540,17 @@ protected:
   virtual ~NotificationTask() {}
 
   UniquePtr<NotificationRef> mNotificationRef;
   NotificationAction mAction;
 };
 
 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)
   NS_INTERFACE_MAP_ENTRY(nsIRunnable)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIContentPermissionRequest)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(NotificationPermissionRequest)
@@ -576,17 +580,17 @@ NotificationPermissionRequest::Run()
     if (Preferences::GetBool("notification.prompt.testing.allow", true)) {
       mPermission = NotificationPermission::Granted;
     } else {
       mPermission = NotificationPermission::Denied;
     }
   }
 
   if (mPermission != NotificationPermission::Default) {
-    return DispatchCallback();
+    return DispatchResolvePromise();
   }
 
   return nsContentPermissionUtils::AskPermission(this, mWindow);
 }
 
 NS_IMETHODIMP
 NotificationPermissionRequest::GetPrincipal(nsIPrincipal** aRequestingPrincipal)
 {
@@ -608,56 +612,57 @@ NotificationPermissionRequest::GetElemen
   *aElement = nullptr;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 NotificationPermissionRequest::Cancel()
 {
   mPermission = NotificationPermission::Denied;
-  return DispatchCallback();
+  return DispatchResolvePromise();
 }
 
 NS_IMETHODIMP
 NotificationPermissionRequest::Allow(JS::HandleValue aChoices)
 {
   MOZ_ASSERT(aChoices.isUndefined());
 
   mPermission = NotificationPermission::Granted;
-  return DispatchCallback();
+  return DispatchResolvePromise();
 }
 
 NS_IMETHODIMP
 NotificationPermissionRequest::GetRequester(nsIContentPermissionRequester** aRequester)
 {
   NS_ENSURE_ARG_POINTER(aRequester);
 
   nsCOMPtr<nsIContentPermissionRequester> requester = mRequester;
   requester.forget(aRequester);
   return NS_OK;
 }
 
 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
 NotificationPermissionRequest::GetTypes(nsIArray** aTypes)
 {
   nsTArray<nsString> emptyOptions;
   return nsContentPermissionUtils::CreatePermissionArray(NS_LITERAL_CSTRING("desktop-notification"),
                                                          NS_LITERAL_CSTRING("unused"),
@@ -1830,38 +1835,44 @@ Notification::RequestPermissionEnabledFo
 {
   // requestPermission() is not allowed on workers. The calling page should ask
   // for permission on the worker's behalf. This is to prevent 'which window
   // should show the browser pop-up'. See discussion:
   // http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2013-October/041272.html
   return NS_IsMainThread();
 }
 
-void
+already_AddRefed<Promise>
 Notification::RequestPermission(const GlobalObject& aGlobal,
                                 const Optional<OwningNonNull<NotificationPermissionCallback> >& aCallback,
                                 ErrorResult& aRv)
 {
   // Get principal from global to make permission request for notifications.
   nsCOMPtr<nsPIDOMWindow> window = do_QueryInterface(aGlobal.GetAsSupports());
   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
 NotificationPermission
 Notification::GetPermission(const GlobalObject& aGlobal, ErrorResult& aRv)
 {
   nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(aGlobal.GetAsSupports());
   return GetPermission(global, aRv);
--- a/dom/notification/Notification.h
+++ b/dom/notification/Notification.h
@@ -230,19 +230,20 @@ public:
 
   bool IsStored()
   {
     return mIsStored;
   }
 
   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);
 
   static already_AddRefed<Promise>
   Get(nsPIDOMWindow* aWindow,
       const GetNotificationOptions& aFilter,
       const nsAString& aScope,
--- a/dom/tests/mochitest/notification/test_notification_basics.html
+++ b/dom/tests/mochitest/notification/test_notification_basics.html
@@ -23,48 +23,61 @@
       info("Test notification spec");
       ok(Notification, "Notification constructor exists");
       ok(Notification.permission, "Notification.permission exists");
       ok(Notification.requestPermission, "Notification.requestPermission exists");
       ok(Notification.get, "Notification.get exists");
     },
 
     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) {
       info("Test create notification");
 
       options = NotificationTest.payload;
 
       var notification = new Notification("This is a title", options);
--- a/dom/webidl/Notification.webidl
+++ b/dom/webidl/Notification.webidl
@@ -15,17 +15,17 @@
  Exposed=(Window,Worker),
  Func="mozilla::dom::Notification::PrefEnabled",
  UnsafeInPrerendering]
 interface Notification : EventTarget {
   [GetterThrows]
   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);
 
   attribute EventHandler onclick;
 
   attribute EventHandler onshow;
 
--- a/testing/web-platform/tests/notifications/interfaces.html
+++ b/testing/web-platform/tests/notifications/interfaces.html
@@ -15,17 +15,17 @@ interface EventTarget {
 [TreatNonCallableAsNull]
 callback EventHandlerNonNull = any (Event event);
 typedef EventHandlerNonNull? EventHandler;
 </script>
 <script type=text/plain>
 [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;
   attribute EventHandler onerror;
   attribute EventHandler onclose;
 
   readonly attribute DOMString title;
   readonly attribute NotificationDirection dir;