From b0243117bd0d75a9dc339fffe54d3c855829f039 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Thu, 21 Feb 2019 22:54:24 +0000 Subject: [PATCH] Bug 1527505 - Part 7: Compute the default permission in the recursive loop only once; r=nika Depends on D20234 Differential Revision: https://phabricator.services.mozilla.com/D20235 --HG-- extra : moz-landing-system : lando --- extensions/cookie/nsPermissionManager.cpp | 68 +++++++------------- extensions/cookie/nsPermissionManager.h | 77 ++++++++++++++++++----- 2 files changed, 85 insertions(+), 60 deletions(-) diff --git a/extensions/cookie/nsPermissionManager.cpp b/extensions/cookie/nsPermissionManager.cpp index 1d8f802a9442..c9d55d89d662 100644 --- a/extensions/cookie/nsPermissionManager.cpp +++ b/extensions/cookie/nsPermissionManager.cpp @@ -13,7 +13,6 @@ #include "mozilla/Pair.h" #include "mozilla/Services.h" #include "mozilla/SystemGroup.h" -#include "mozilla/Unused.h" #include "nsPermissionManager.h" #include "nsPermission.h" #include "nsCRT.h" @@ -115,25 +114,6 @@ static const char* kPreloadPermissions[] = { USER_INTERACTION_PERM}; -// A list of permissions that can have a fallback default permission -// set under the permissions.default.* pref. -static const char* kPermissionsWithDefaults[] = { - "camera", "microphone", "geo", "desktop-notification", "shortcuts"}; - -// NOTE: nullptr can be passed as aType - if it is this function will return -// "false" unconditionally. -bool HasDefaultPref(const char* aType) { - if (aType) { - for (const char* perm : kPermissionsWithDefaults) { - if (!strcmp(aType, perm)) { - return true; - } - } - } - - return false; -} - // NOTE: nullptr can be passed as aType - if it is this function will return // "false" unconditionally. bool IsPreloadPermission(const char* aType) { @@ -2252,27 +2232,35 @@ nsresult nsPermissionManager::RemoveAllInternal(bool aNotifyObservers) { NS_IMETHODIMP nsPermissionManager::TestExactPermission(nsIURI* aURI, const char* aType, uint32_t* aPermission) { - return CommonTestPermission(aURI, -1, aType, aPermission, true, true); + return CommonTestPermission(aURI, -1, aType, aPermission, + nsIPermissionManager::UNKNOWN_ACTION, false, true, + true); } NS_IMETHODIMP nsPermissionManager::TestExactPermissionFromPrincipal(nsIPrincipal* aPrincipal, const char* aType, uint32_t* aPermission) { - return CommonTestPermission(aPrincipal, -1, aType, aPermission, true, true); + return CommonTestPermission(aPrincipal, -1, aType, aPermission, + nsIPermissionManager::UNKNOWN_ACTION, false, true, + true); } NS_IMETHODIMP nsPermissionManager::TestExactPermanentPermission(nsIPrincipal* aPrincipal, const char* aType, uint32_t* aPermission) { - return CommonTestPermission(aPrincipal, -1, aType, aPermission, true, false); + return CommonTestPermission(aPrincipal, -1, aType, aPermission, + nsIPermissionManager::UNKNOWN_ACTION, false, true, + false); } NS_IMETHODIMP nsPermissionManager::TestPermission(nsIURI* aURI, const char* aType, uint32_t* aPermission) { - return CommonTestPermission(aURI, -1, aType, aPermission, false, true); + return CommonTestPermission(aURI, -1, aType, aPermission, + nsIPermissionManager::UNKNOWN_ACTION, false, + false, true); } NS_IMETHODIMP @@ -2281,17 +2269,18 @@ nsPermissionManager::TestPermissionOriginNoSuffix( uint32_t* aPermission) { // Our caller isn't providing a type index hint, so we just pass -1 to force // CommonPrepareToTestPermission to compute it for us based on aType. - auto preparationResult = - CommonPrepareToTestPermission(nullptr, -1, aType, aPermission); + auto preparationResult = CommonPrepareToTestPermission( + nullptr, -1, aType, aPermission, nsIPermissionManager::UNKNOWN_ACTION, + false); if (preparationResult.mShouldContinue == eDone) { return NS_OK; } MOZ_ASSERT(!preparationResult.mPrincipal); - return CommonTestPermissionInternal(nullptr, nullptr, aOriginNoSuffix, - preparationResult.mTypeIndex, aType, - aPermission, false, true); + return CommonTestPermissionInternal( + nullptr, nullptr, aOriginNoSuffix, preparationResult.mTypeIndex, aType, + aPermission, preparationResult.mDefaultPermission, false, true); } NS_IMETHODIMP @@ -2313,7 +2302,9 @@ NS_IMETHODIMP nsPermissionManager::TestPermissionFromPrincipal(nsIPrincipal* aPrincipal, const char* aType, uint32_t* aPermission) { - return CommonTestPermission(aPrincipal, -1, aType, aPermission, false, true); + return CommonTestPermission(aPrincipal, -1, aType, aPermission, + nsIPermissionManager::UNKNOWN_ACTION, false, + false, true); } NS_IMETHODIMP @@ -2385,27 +2376,13 @@ nsPermissionManager::GetPermissionObject(nsIPrincipal* aPrincipal, nsresult nsPermissionManager::CommonTestPermissionInternal( BasePrincipal* aPrincipal, nsIURI* aURI, const nsACString& aOriginNoSuffix, int32_t aTypeIndex, const char* aType, uint32_t* aPermission, - bool aExactHostMatch, bool aIncludingSession) { + uint32_t aDefaultPermission, bool aExactHostMatch, bool aIncludingSession) { MOZ_ASSERT(aPrincipal || aURI || !aOriginNoSuffix.IsEmpty()); MOZ_ASSERT_IF(aPrincipal, !aURI && aOriginNoSuffix.IsEmpty()); MOZ_ASSERT_IF(aURI, !aPrincipal && aOriginNoSuffix.IsEmpty()); NS_ENSURE_ARG_POINTER(aPrincipal || aURI || !aOriginNoSuffix.IsEmpty()); NS_ENSURE_ARG_POINTER(aType); - // Set the default. - *aPermission = nsIPermissionManager::UNKNOWN_ACTION; - - // For some permissions, query the default from a pref. We want to avoid - // doing this for all permissions so that permissions can opt into having - // the pref lookup overhead on each call. - if (HasDefaultPref(aType)) { - int32_t defaultPermission = nsIPermissionManager::UNKNOWN_ACTION; - nsresult rv = mDefaultPrefBranch->GetIntPref(aType, &defaultPermission); - if (NS_SUCCEEDED(rv)) { - *aPermission = defaultPermission; - } - } - // For expanded principals, we want to iterate over the allowlist and see // if the permission is granted for any of them. if (aPrincipal && aPrincipal->Is()) { @@ -2413,6 +2390,7 @@ nsresult nsPermissionManager::CommonTestPermissionInternal( for (auto& prin : ep->AllowList()) { uint32_t perm; nsresult rv = CommonTestPermission(prin, aTypeIndex, aType, &perm, + aDefaultPermission, true, aExactHostMatch, aIncludingSession); NS_ENSURE_SUCCESS(rv, rv); if (perm == nsIPermissionManager::ALLOW_ACTION) { diff --git a/extensions/cookie/nsPermissionManager.h b/extensions/cookie/nsPermissionManager.h index 9e9256007233..b68eddd88443 100644 --- a/extensions/cookie/nsPermissionManager.h +++ b/extensions/cookie/nsPermissionManager.h @@ -24,6 +24,7 @@ #include "nsRefPtrHashtable.h" #include "mozilla/BasePrincipal.h" #include "mozilla/MozPromise.h" +#include "mozilla/Unused.h" #include "mozilla/Vector.h" namespace mozilla { @@ -269,6 +270,25 @@ class nsPermissionManager final : public nsIPermissionManager, private: virtual ~nsPermissionManager(); + // NOTE: nullptr can be passed as aType - if it is this function will return + // "false" unconditionally. + static bool HasDefaultPref(const char* aType) { + // A list of permissions that can have a fallback default permission + // set under the permissions.default.* pref. + static const char* kPermissionsWithDefaults[] = { + "camera", "microphone", "geo", "desktop-notification", "shortcuts"}; + + if (aType) { + for (const char* perm : kPermissionsWithDefaults) { + if (!strcmp(aType, perm)) { + return true; + } + } + } + + return false; + } + // Returns -1 on failure int32_t GetTypeIndex(const char* aType, bool aAdd) { for (uint32_t i = 0; i < mTypeArray.length(); ++i) { @@ -301,6 +321,7 @@ class nsPermissionManager final : public nsIPermissionManager, struct TestPreparationResult { mozilla::BasePrincipal* mPrincipal; int32_t mTypeIndex; + uint32_t mDefaultPermission; TestPreparationEnum mShouldContinue; }; /** @@ -319,50 +340,74 @@ class nsPermissionManager final : public nsIPermissionManager, * @param aPermission out argument which will be a permission type that we * will return from this function once the function is * done. + * @param aDefaultPermission the default permission to be used if we can't + * determine the result of the permission check. + * @param aDefaultPermissionIsValid whether the previous argument contains a + * valid value. */ - TestPreparationResult CommonPrepareToTestPermission(nsIPrincipal* aPrincipal, - int32_t aTypeIndex, - const char* aType, - uint32_t* aPermission) { + TestPreparationResult CommonPrepareToTestPermission( + nsIPrincipal* aPrincipal, int32_t aTypeIndex, const char* aType, + uint32_t* aPermission, uint32_t aDefaultPermission, + bool aDefaultPermissionIsValid) { auto* basePrin = mozilla::BasePrincipal::Cast(aPrincipal); if (basePrin && basePrin->IsSystemPrincipal()) { - *aPermission = nsIPermissionManager::ALLOW_ACTION; - return {basePrin, -1, eDone}; + *aPermission = ALLOW_ACTION; + return {basePrin, -1, UNKNOWN_ACTION, eDone}; } - *aPermission = nsIPermissionManager::UNKNOWN_ACTION; + // For some permissions, query the default from a pref. We want to avoid + // doing this for all permissions so that permissions can opt into having + // the pref lookup overhead on each call. + int32_t defaultPermission = + aDefaultPermissionIsValid ? aDefaultPermission : UNKNOWN_ACTION; + if (!aDefaultPermissionIsValid && HasDefaultPref(aType)) { + mozilla::Unused << mDefaultPrefBranch->GetIntPref(aType, + &defaultPermission); + } + // Set the default. + *aPermission = defaultPermission; + + // For expanded principals, we want to iterate over the allowlist and see + // if the permission is granted for any of them. int32_t typeIndex = aTypeIndex == -1 ? GetTypeIndex(aType, false) : aTypeIndex; // If type == -1, the type isn't known, just signal that we are done. if (typeIndex == -1) { - return {basePrin, -1, eDone}; + return {basePrin, -1, UNKNOWN_ACTION, NS_OK, eDone}; } - return {basePrin, typeIndex, eContinue}; + return {basePrin, typeIndex, uint32_t(defaultPermission), NS_OK, eContinue}; } // If aTypeIndex is passed -1, we try to inder the type index from aType. nsresult CommonTestPermission(nsIPrincipal* aPrincipal, int32_t aTypeIndex, const char* aType, uint32_t* aPermission, + uint32_t aDefaultPermission, + bool aDefaultPermissionIsValid, bool aExactHostMatch, bool aIncludingSession) { auto preparationResult = CommonPrepareToTestPermission( - aPrincipal, aTypeIndex, aType, aPermission); + aPrincipal, aTypeIndex, aType, aPermission, aDefaultPermission, + aDefaultPermissionIsValid); if (preparationResult.mShouldContinue == eDone) { return NS_OK; } return CommonTestPermissionInternal( preparationResult.mPrincipal, nullptr, EmptyCString(), - preparationResult.mTypeIndex, aType, aPermission, aExactHostMatch, + preparationResult.mTypeIndex, aType, aPermission, + preparationResult.mDefaultPermission, aExactHostMatch, aIncludingSession); } // If aTypeIndex is passed -1, we try to inder the type index from aType. nsresult CommonTestPermission(nsIURI* aURI, int32_t aTypeIndex, const char* aType, uint32_t* aPermission, + uint32_t aDefaultPermission, + bool aDefaultPermissionIsValid, bool aExactHostMatch, bool aIncludingSession) { - auto preparationResult = - CommonPrepareToTestPermission(nullptr, aTypeIndex, aType, aPermission); + auto preparationResult = CommonPrepareToTestPermission( + nullptr, aTypeIndex, aType, aPermission, aDefaultPermission, + aDefaultPermissionIsValid); if (preparationResult.mShouldContinue == eDone) { return NS_OK; } @@ -371,13 +416,15 @@ class nsPermissionManager final : public nsIPermissionManager, return CommonTestPermissionInternal( nullptr, aURI, EmptyCString(), preparationResult.mTypeIndex, aType, - aPermission, aExactHostMatch, aIncludingSession); + aPermission, preparationResult.mDefaultPermission, aExactHostMatch, + aIncludingSession); } // Only one of aPrincipal or aURI is allowed to be passed in. nsresult CommonTestPermissionInternal( mozilla::BasePrincipal* aPrincipal, nsIURI* aURI, const nsACString& aOriginNoSuffix, int32_t aTypeIndex, const char* aType, - uint32_t* aPermission, bool aExactHostMatch, bool aIncludingSession); + uint32_t* aPermission, uint32_t aDefaultPermission, bool aExactHostMatch, + bool aIncludingSession); nsresult OpenDatabase(nsIFile* permissionsFile); nsresult InitDB(bool aRemoveFile);