Bug 1355608 - Part 1: Add tools to nsPermissionManager to await permissions becoming avaliable, r=baku
authorMichael Layzell <michael@thelayzells.com>
Tue, 11 Apr 2017 16:36:37 -0400
changeset 355465 eb249c8b3e6010cc3ae9a12432ee5498d598f8c5
parent 355464 e9e4b447bf87b183cc7bb6007e35c1931828c319
child 355466 0334bfc886c8b6bef2df8c9a130cde364bd5d816
push id31727
push usercbook@mozilla.com
push dateFri, 28 Apr 2017 08:36:40 +0000
treeherdermozilla-central@8f2b930cd028 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1355608
milestone55.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 1355608 - Part 1: Add tools to nsPermissionManager to await permissions becoming avaliable, r=baku MozReview-Commit-ID: 1HDS8zw6dpF
extensions/cookie/nsPermissionManager.cpp
extensions/cookie/nsPermissionManager.h
netwerk/base/nsIPermissionManager.idl
--- a/extensions/cookie/nsPermissionManager.cpp
+++ b/extensions/cookie/nsPermissionManager.cpp
@@ -35,21 +35,22 @@
 #include "mozilla/Preferences.h"
 #include "nsReadLine.h"
 #include "mozilla/Telemetry.h"
 #include "nsIConsoleService.h"
 #include "nsINavHistoryService.h"
 #include "nsToolkitCompsCID.h"
 #include "nsIObserverService.h"
 #include "nsPrintfCString.h"
+#include "mozilla/AbstractThread.h"
 
 static nsPermissionManager *gPermissionManager = nullptr;
 
-using mozilla::dom::ContentParent;
-using mozilla::Unused; // ha!
+using namespace mozilla;
+using namespace mozilla::dom;
 
 static bool
 IsChildProcess()
 {
   return XRE_IsContentProcess();
 }
 
 static void
@@ -843,16 +844,25 @@ nsPermissionManager::nsPermissionManager
  : mMemoryOnlyDB(false)
  , mLargestID(0)
  , mIsShuttingDown(false)
 {
 }
 
 nsPermissionManager::~nsPermissionManager()
 {
+  // NOTE: Make sure to reject each of the promises in mPermissionKeyPromiseMap
+  // before destroying.
+  for (auto iter = mPermissionKeyPromiseMap.Iter(); !iter.Done(); iter.Next()) {
+    if (iter.Data()) {
+      iter.Data()->Reject(NS_ERROR_FAILURE, __func__);
+    }
+  }
+  mPermissionKeyPromiseMap.Clear();
+
   RemoveAllFromMemory();
   gPermissionManager = nullptr;
 }
 
 // static
 nsIPermissionManager*
 nsPermissionManager::GetXPCOMSingleton()
 {
@@ -3039,23 +3049,30 @@ nsPermissionManager::GetPermissionsWithK
 NS_IMETHODIMP
 nsPermissionManager::SetPermissionsWithKey(const nsACString& aPermissionKey,
                                            nsTArray<IPC::Permission>& aPerms)
 {
   if (NS_WARN_IF(XRE_IsParentProcess())) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  // Record that we have seen the permissions with the given permission key.
-  if (NS_WARN_IF(mAvailablePermissionKeys.Contains(aPermissionKey))) {
+  RefPtr<GenericPromise::Private> promise;
+  bool foundKey = mPermissionKeyPromiseMap.Get(aPermissionKey, getter_AddRefs(promise));
+  if (promise) {
+    MOZ_ASSERT(foundKey);
+    // NOTE: This will resolve asynchronously, so we can mark it as resolved
+    // now, and be confident that we will have filled in the database before any
+    // callbacks run.
+    promise->Resolve(true, __func__);
+  } else if (foundKey) {
     // NOTE: We shouldn't be sent two InitializePermissionsWithKey for the same
     // key, but it's possible.
     return NS_OK;
   }
-  mAvailablePermissionKeys.PutEntry(aPermissionKey);
+  mPermissionKeyPromiseMap.Put(aPermissionKey, nullptr);
 
   // Add the permissions locally to our process
   for (IPC::Permission& perm : aPerms) {
     nsCOMPtr<nsIPrincipal> principal;
     nsresult rv = GetPrincipalFromOrigin(perm.origin, getter_AddRefs(principal));
     if (NS_WARN_IF(NS_FAILED(rv))) {
       continue;
     }
@@ -3166,18 +3183,68 @@ nsPermissionManager::BroadcastPermission
 
 bool
 nsPermissionManager::PermissionAvaliable(nsIPrincipal* aPrincipal, const char* aType)
 {
   if (XRE_IsContentProcess()) {
     nsAutoCString permissionKey;
     // NOTE: GetKeyForPermission accepts a null aType.
     GetKeyForPermission(aPrincipal, aType, permissionKey);
-    if (!mAvailablePermissionKeys.Contains(permissionKey)) {
+
+    // If we have a pending promise for the permission key in question, we don't
+    // have the permission avaliable, so report a warning and return false.
+    RefPtr<GenericPromise::Private> promise;
+    if (!mPermissionKeyPromiseMap.Get(permissionKey, getter_AddRefs(promise)) || promise) {
       // Emit a useful diagnostic warning with the permissionKey for the process
       // which hasn't received permissions yet.
       NS_WARNING(nsPrintfCString("This content process hasn't received the "
                                  "permissions for %s yet", permissionKey.get()).get());
       return false;
     }
   }
   return true;
 }
+
+NS_IMETHODIMP
+nsPermissionManager::WhenPermissionsAvailable(nsIPrincipal* aPrincipal,
+                                              nsIRunnable* aRunnable)
+{
+  MOZ_ASSERT(aRunnable);
+
+  if (!XRE_IsContentProcess()) {
+    aRunnable->Run();
+    return NS_OK;
+  }
+
+  nsTArray<RefPtr<GenericPromise>> promises;
+  for (auto& key : GetAllKeysForPrincipal(aPrincipal)) {
+    RefPtr<GenericPromise::Private> promise;
+    if (!mPermissionKeyPromiseMap.Get(key, getter_AddRefs(promise))) {
+      // In this case we have found a permission which isn't avaliable in the
+      // content process and hasn't been requested yet. We need to create a new
+      // promise, and send the request to the parent (if we have not already
+      // done so).
+      promise = new GenericPromise::Private(__func__);
+      mPermissionKeyPromiseMap.Put(key, RefPtr<GenericPromise::Private>(promise).forget());
+    }
+
+    if (promise) {
+      promises.AppendElement(Move(promise));
+    }
+  }
+
+  // If all of our permissions are avaliable, immediately run the runnable. This
+  // avoids any extra overhead during fetch interception which is performance
+  // sensitive.
+  if (promises.IsEmpty()) {
+    aRunnable->Run();
+    return NS_OK;
+  }
+
+  RefPtr<nsIRunnable> runnable = aRunnable;
+  GenericPromise::All(AbstractThread::GetCurrent(), promises)->Then(
+    AbstractThread::GetCurrent(), __func__,
+    [runnable] () { runnable->Run(); },
+    [] () {
+      NS_WARNING("nsPermissionManager permission promise rejected. We're probably shutting down.");
+    });
+  return NS_OK;
+}
--- a/extensions/cookie/nsPermissionManager.h
+++ b/extensions/cookie/nsPermissionManager.h
@@ -13,16 +13,19 @@
 #include "nsIInputStream.h"
 #include "nsTHashtable.h"
 #include "nsTArray.h"
 #include "nsString.h"
 #include "nsPermission.h"
 #include "nsHashKeys.h"
 #include "nsCOMArray.h"
 #include "nsDataHashtable.h"
+#include "nsIRunnable.h"
+#include "nsRefPtrHashtable.h"
+#include "mozilla/MozPromise.h"
 
 namespace mozilla {
 class OriginAttributesPattern;
 }
 
 class nsIPermission;
 class mozIStorageConnection;
 class mozIStorageAsyncStatement;
@@ -318,33 +321,32 @@ private:
    * Returns false if this permission manager wouldn't have the permission
    * requested avaliable.
    *
    * If aType is nullptr, checks that the permission manager would have all
    * permissions avaliable for the given principal.
    */
   bool PermissionAvaliable(nsIPrincipal* aPrincipal, const char* aType);
 
+  nsRefPtrHashtable<nsCStringHashKey, mozilla::GenericPromise::Private> mPermissionKeyPromiseMap;
+
   nsCOMPtr<mozIStorageConnection> mDBConn;
   nsCOMPtr<mozIStorageAsyncStatement> mStmtInsert;
   nsCOMPtr<mozIStorageAsyncStatement> mStmtDelete;
   nsCOMPtr<mozIStorageAsyncStatement> mStmtUpdate;
 
   bool mMemoryOnlyDB;
 
   nsTHashtable<PermissionHashKey> mPermissionTable;
   // a unique, monotonically increasing id used to identify each database entry
   int64_t                      mLargestID;
 
   // An array to store the strings identifying the different types.
   nsTArray<nsCString>          mTypeArray;
 
-  // The base domains which have their permissions loaded in the current process.
-  nsTHashtable<nsCStringHashKey> mAvailablePermissionKeys;
-
   // Initially, |false|. Set to |true| once shutdown has started, to avoid
   // reopening the database.
   bool mIsShuttingDown;
 
   friend class DeleteFromMozHostListener;
   friend class CloseDatabaseListener;
 };
 
--- a/netwerk/base/nsIPermissionManager.idl
+++ b/netwerk/base/nsIPermissionManager.idl
@@ -31,16 +31,17 @@
 #include "nsISupports.idl"
 
 interface nsIURI;
 interface nsIObserver;
 interface nsIPrincipal;
 interface mozIDOMWindow;
 interface nsIPermission;
 interface nsISimpleEnumerator;
+interface nsIRunnable;
 
 %{ C++
 namespace IPC {
 struct Permission;
 }
 #include "nsTArrayForwardDeclare.h"
 %}
 [ref] native IPCPermissionArrayRef(nsTArray<IPC::Permission>);
@@ -321,15 +322,31 @@ interface nsIPermissionManager : nsISupp
    *
    * DO NOT USE THIS METHOD if you can avoid it. It was added in bug XXX to
    * handle the current temporary implementation of ServiceWorker debugging. It
    * will be removed when service worker debugging is fixed.
    *
    * @param aPrincipal The principal to broadcast permissions for.
    */
   void broadcastPermissionsForPrincipalToAllContentProcesses(in nsIPrincipal aPrincipal);
+
+  /**
+   * Add a callback which should be run when all permissions are available for
+   * the given nsIPrincipal. This method invokes the callback runnable
+   * synchronously when the permissions are already available.
+   *
+   * NOTE: This method will not request the permissions be sent by the parent
+   * process. This should only be used to wait for permissions which may not
+   * have arrived yet in order to ensure they are present.
+   *
+   * @param aPrincipal The principal to wait for permissions to be available for.
+   * @param aRunnable  The runnable to run when permissions are available for the
+   *                   given principal.
+   */
+  void whenPermissionsAvailable(in nsIPrincipal aPrincipal,
+                                in nsIRunnable aRunnable);
 };
 
 %{ C++
 #define NS_PERMISSIONMANAGER_CONTRACTID "@mozilla.org/permissionmanager;1"
 
 #define PERM_CHANGE_NOTIFICATION "perm-changed"
 %}