Bug 1372085 - Cache notification permission requests on the window. draft
authorBlake Kaplan <mrbkap@gmail.com>
Tue, 03 Oct 2017 18:20:30 -0700
changeset 809752 4e6602b9aa37aeb8e1027185197c4cf6091b1364
parent 809260 1c33a38da75d550750923358e73d7af2102b9c1d
push id113797
push userbmo:mrbkap@mozilla.com
push dateFri, 22 Jun 2018 21:48:34 +0000
bugs1372085
milestone62.0a1
Bug 1372085 - Cache notification permission requests on the window. MozReview-Commit-ID: 550UcjGx2Ba
dom/base/nsGlobalWindowInner.cpp
dom/base/nsPIDOMWindow.h
dom/notification/Notification.cpp
dom/notification/test/mochitest/test_notification_basics.html
--- a/dom/base/nsGlobalWindowInner.cpp
+++ b/dom/base/nsGlobalWindowInner.cpp
@@ -38,16 +38,17 @@
 #include "nsDOMOfflineResourceList.h"
 #include "nsError.h"
 #include "nsIIdleService.h"
 #include "nsISizeOfEventTarget.h"
 #include "nsDOMJSUtils.h"
 #include "nsArrayUtils.h"
 #include "nsDOMWindowList.h"
 #include "mozilla/dom/WakeLock.h"
+#include "nsIContentPermissionPrompt.h"
 #include "mozilla/dom/power/PowerManagerService.h"
 #include "nsIDocShellTreeOwner.h"
 #include "nsIDocumentLoader.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIPermissionManager.h"
 #include "nsIScriptContext.h"
 #include "nsIScriptTimeoutHandler.h"
 #include "nsITimeoutHandler.h"
@@ -1444,16 +1445,17 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mGamepads)
 
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCacheStorage)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mVRDisplays)
 
   // Traverse stuff from nsPIDOMWindow
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mChromeEventHandler)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParentTarget)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCachedNotificationRequest)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFocusedElement)
 
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMenubar)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mToolbar)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLocationbar)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPersonalbar)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mStatusbar)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScrollbars)
@@ -1532,16 +1534,17 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ns
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mGamepads)
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mCacheStorage)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mVRDisplays)
 
   // Unlink stuff from nsPIDOMWindow
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mChromeEventHandler)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mParentTarget)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mCachedNotificationRequest)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mFocusedElement)
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mMenubar)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mToolbar)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mLocationbar)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mPersonalbar)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mStatusbar)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mScrollbars)
--- a/dom/base/nsPIDOMWindow.h
+++ b/dom/base/nsPIDOMWindow.h
@@ -11,16 +11,17 @@
 #include "mozIDOMWindow.h"
 
 #include "nsCOMPtr.h"
 #include "nsTArray.h"
 #include "mozilla/dom/EventTarget.h"
 #include "mozilla/TaskCategory.h"
 #include "js/TypeDecls.h"
 #include "nsRefPtrHashtable.h"
+#include "nsIContentPermissionPrompt.h"
 
 // Only fired for inner windows.
 #define DOM_WINDOW_DESTROYED_TOPIC "dom-window-destroyed"
 #define DOM_WINDOW_FROZEN_TOPIC "dom-window-frozen"
 #define DOM_WINDOW_THAWED_TOPIC "dom-window-thawed"
 
 class nsDOMWindowList;
 class nsGlobalWindowInner;
@@ -606,16 +607,26 @@ public:
 
   virtual nsresult Focus() = 0;
   virtual nsresult Close() = 0;
 
   mozilla::dom::DocGroup* GetDocGroup() const;
   virtual nsISerialEventTarget*
   EventTargetFor(mozilla::TaskCategory aCategory) const = 0;
 
+  nsIContentPermissionRequest*
+  GetCachedNotificationRequest() {
+    return mCachedNotificationRequest;
+  }
+
+  void
+  SetCachedNotificationRequest(nsIContentPermissionRequest* aRequest) {
+    mCachedNotificationRequest = aRequest;
+  }
+
 protected:
   void CreatePerformanceObjectIfNeeded();
 
   // Lazily instantiate an about:blank document if necessary, and if
   // we have what it takes to do so.
   void MaybeCreateDoc();
 
   void SetChromeEventHandlerInternal(mozilla::dom::EventTarget* aChromeEventHandler) {
@@ -631,16 +642,17 @@ protected:
   // sure you keep them in sync!
   nsCOMPtr<mozilla::dom::EventTarget> mChromeEventHandler; // strong
   nsCOMPtr<nsIDocument> mDoc; // strong
   // Cache the URI when mDoc is cleared.
   nsCOMPtr<nsIURI> mDocumentURI; // strong
   nsCOMPtr<nsIURI> mDocBaseURI; // strong
 
   nsCOMPtr<mozilla::dom::EventTarget> mParentTarget; // strong
+  nsCOMPtr<nsIContentPermissionRequest> mCachedNotificationRequest;
 
   RefPtr<mozilla::dom::Performance> mPerformance;
   mozilla::UniquePtr<mozilla::dom::TimeoutManager> mTimeoutManager;
 
   RefPtr<mozilla::dom::Navigator> mNavigator;
 
   // These variables are only used on inner windows.
   uint32_t mMutationBits;
--- a/dom/notification/Notification.cpp
+++ b/dom/notification/Notification.cpp
@@ -250,28 +250,34 @@ public:
   }
 
   NS_IMETHOD GetName(nsACString& aName) override
   {
     aName.AssignLiteral("NotificationPermissionRequest");
     return NS_OK;
   }
 
+  void ChainRequest(NotificationPermissionRequest* aRequest)
+  {
+    mChainedCallbacks.AppendElement(aRequest);
+  }
+
 protected:
   virtual ~NotificationPermissionRequest() {}
 
   nsresult ResolvePromise();
   nsresult DispatchResolvePromise();
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsCOMPtr<nsPIDOMWindowInner> mWindow;
   NotificationPermission mPermission;
   RefPtr<Promise> mPromise;
   RefPtr<NotificationPermissionCallback> mCallback;
   nsCOMPtr<nsIContentPermissionRequester> mRequester;
   bool mIsHandlingUserInput;
+  nsTArray<RefPtr<NotificationPermissionRequest>> mChainedCallbacks;
 };
 
 namespace {
 class ReleaseNotificationControlRunnable final : public MainThreadWorkerControlRunnable
 {
   Notification* mNotification;
 
 public:
@@ -533,18 +539,21 @@ protected:
   virtual ~NotificationTask() {}
 
   UniquePtr<NotificationRef> mNotificationRef;
   NotificationAction mAction;
 };
 
 uint32_t Notification::sCount = 0;
 
-NS_IMPL_CYCLE_COLLECTION(NotificationPermissionRequest, mWindow, mPromise,
-                                                        mCallback)
+NS_IMPL_CYCLE_COLLECTION(NotificationPermissionRequest,
+                         mWindow,
+                         mPromise,
+                         mCallback,
+                         mChainedCallbacks)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(NotificationPermissionRequest)
   NS_INTERFACE_MAP_ENTRY(nsIContentPermissionRequest)
   NS_INTERFACE_MAP_ENTRY(nsIRunnable)
   NS_INTERFACE_MAP_ENTRY(nsINamed)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIContentPermissionRequest)
 NS_INTERFACE_MAP_END
 
@@ -643,16 +652,20 @@ NotificationPermissionRequest::GetReques
   nsCOMPtr<nsIContentPermissionRequester> requester = mRequester;
   requester.forget(aRequester);
   return NS_OK;
 }
 
 inline nsresult
 NotificationPermissionRequest::DispatchResolvePromise()
 {
+  if (mWindow->GetCachedNotificationRequest() == this) {
+    mWindow->SetCachedNotificationRequest(nullptr);
+  }
+
   nsCOMPtr<nsIRunnable> resolver =
     NewRunnableMethod("NotificationPermissionRequest::DispatchResolvePromise",
                       this, &NotificationPermissionRequest::ResolvePromise);
   if (nsIEventTarget* target = mWindow->EventTargetFor(TaskCategory::Other)) {
     return target->Dispatch(resolver.forget(), nsIEventTarget::DISPATCH_NORMAL);
   }
   return NS_ERROR_FAILURE;
 }
@@ -667,16 +680,21 @@ NotificationPermissionRequest::ResolvePr
     mPermission = Notification::TestPermission(mPrincipal);
   }
   if (mCallback) {
     ErrorResult error;
     mCallback->Call(mPermission, error);
     rv = error.StealNSResult();
   }
   mPromise->MaybeResolve(mPermission);
+
+  for (NotificationPermissionRequest* chained : mChainedCallbacks) {
+    chained->mPermission = mPermission;
+    chained->ResolvePromise();
+  }
   return rv;
 }
 
 NS_IMETHODIMP
 NotificationPermissionRequest::GetTypes(nsIArray** aTypes)
 {
   nsTArray<nsString> emptyOptions;
   return nsContentPermissionUtils::CreatePermissionArray(NS_LITERAL_CSTRING("desktop-notification"),
@@ -1800,21 +1818,27 @@ Notification::RequestPermission(const Gl
   RefPtr<Promise> promise = Promise::Create(window->AsGlobal(), aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
   NotificationPermissionCallback* permissionCallback = nullptr;
   if (aCallback.WasPassed()) {
     permissionCallback = &aCallback.Value();
   }
+
   bool isHandlingUserInput = EventStateManager::IsHandlingUserInput();
-  nsCOMPtr<nsIRunnable> request = new NotificationPermissionRequest(
+  RefPtr<NotificationPermissionRequest> request = new NotificationPermissionRequest(
     principal, isHandlingUserInput, window, promise, permissionCallback);
 
-  window->AsGlobal()->Dispatch(TaskCategory::Other, request.forget());
+  if (nsIContentPermissionRequest* existing = window->GetCachedNotificationRequest()) {
+    static_cast<NotificationPermissionRequest*>(existing)->ChainRequest(request);
+  } else {
+    window->SetCachedNotificationRequest(request);
+    window->AsGlobal()->Dispatch(TaskCategory::Other, request.forget());
+  }
 
   return promise.forget();
 }
 
 // static
 NotificationPermission
 Notification::GetPermission(const GlobalObject& aGlobal, ErrorResult& aRv)
 {
--- a/dom/notification/test/mochitest/test_notification_basics.html
+++ b/dom/notification/test/mochitest/test_notification_basics.html
@@ -22,19 +22,23 @@
     function () {
       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 () {
+    function (done) {
       info("Test requestPermission without callback");
-      Notification.requestPermission();
+      Notification.requestPermission()
+        .then(perm => {
+          is(perm, "granted", "permission was granted");
+          done();
+        });
     },
 
     function (done) {
       info("Test requestPermission deny");
       function assertPermissionDenied(perm) {
         is(perm, "denied", "Permission should be denied.");
         is(Notification.permission, "denied", "Permission should be denied.");
       }