Bug 1572240 - Part 6: Remove nsIPermissionManager.testPermissionOriginNoSuffix; r=baku

This is now dead code which will be kept alive by the vtable,
and introduces needless overhead inside the permission manager.

Differential Revision: https://phabricator.services.mozilla.com/D42207

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Ehsan Akhgari 2019-08-19 14:38:47 +00:00
parent 05feeeb79b
commit c771ae8334
4 changed files with 23 additions and 202 deletions

View File

@ -770,12 +770,6 @@ nsPermissionManager::PermissionKey::CreateFromURI(nsIURI* aURI,
return new PermissionKey(origin);
}
nsPermissionManager::PermissionKey*
nsPermissionManager::PermissionKey::CreateFromOriginNoSuffix(
const nsACString& aOriginNoSuffix) {
return new PermissionKey(aOriginNoSuffix);
}
/**
* Simple callback used by |AsyncClose| to trigger a treatment once
* the database is closed.
@ -2255,24 +2249,6 @@ nsPermissionManager::TestPermission(nsIURI* aURI, const nsACString& aType,
false, true);
}
NS_IMETHODIMP
nsPermissionManager::TestPermissionOriginNoSuffix(
const nsACString& aOriginNoSuffix, const nsACString& aType,
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, nsIPermissionManager::UNKNOWN_ACTION,
false, false, true);
if (preparationResult.is<nsresult>()) {
return preparationResult.as<nsresult>();
}
return CommonTestPermissionInternal(nullptr, nullptr, aOriginNoSuffix,
preparationResult.as<int32_t>(), aType,
aPermission, false, true);
}
NS_IMETHODIMP
nsPermissionManager::TestPermissionFromWindow(mozIDOMWindow* aWindow,
const nsACString& aType,
@ -2364,13 +2340,12 @@ nsPermissionManager::GetPermissionObject(nsIPrincipal* aPrincipal,
}
nsresult nsPermissionManager::CommonTestPermissionInternal(
nsIPrincipal* aPrincipal, nsIURI* aURI, const nsACString& aOriginNoSuffix,
int32_t aTypeIndex, const nsACString& aType, uint32_t* aPermission,
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());
nsIPrincipal* aPrincipal, nsIURI* aURI, int32_t aTypeIndex,
const nsACString& aType, uint32_t* aPermission, bool aExactHostMatch,
bool aIncludingSession) {
MOZ_ASSERT(aPrincipal || aURI);
MOZ_ASSERT_IF(aPrincipal, !aURI);
NS_ENSURE_ARG_POINTER(aPrincipal || aURI);
#ifdef DEBUG
{
@ -2379,8 +2354,6 @@ nsresult nsPermissionManager::CommonTestPermissionInternal(
if (aURI) {
prin = mozilla::BasePrincipal::CreateContentPrincipal(
aURI, OriginAttributes());
} else if (!aOriginNoSuffix.IsEmpty()) {
prin = mozilla::BasePrincipal::CreateContentPrincipal(aOriginNoSuffix);
}
}
MOZ_ASSERT(prin);
@ -2390,8 +2363,7 @@ nsresult nsPermissionManager::CommonTestPermissionInternal(
PermissionHashKey* entry =
aPrincipal ? GetPermissionHashKey(aPrincipal, aTypeIndex, aExactHostMatch)
: GetPermissionHashKey(aURI, aOriginNoSuffix, aTypeIndex,
aExactHostMatch);
: GetPermissionHashKey(aURI, aTypeIndex, aExactHostMatch);
if (!entry || (!aIncludingSession &&
entry->GetPermission(aTypeIndex).mNonSessionExpireType ==
nsIPermissionManager::EXPIRE_SESSION)) {
@ -2464,12 +2436,9 @@ nsPermissionManager::GetPermissionHashKey(nsIPrincipal* aPrincipal,
// accepts host on the format "<foo>". This will perform an exact match lookup
// as the string doesn't contain any dots.
nsPermissionManager::PermissionHashKey*
nsPermissionManager::GetPermissionHashKey(nsIURI* aURI,
const nsACString& aOriginNoSuffix,
uint32_t aType,
nsPermissionManager::GetPermissionHashKey(nsIURI* aURI, uint32_t aType,
bool aExactHostMatch) {
MOZ_ASSERT(aURI || !aOriginNoSuffix.IsEmpty());
MOZ_ASSERT_IF(aURI, aOriginNoSuffix.IsEmpty());
MOZ_ASSERT(aURI);
#ifdef DEBUG
{
@ -2477,9 +2446,6 @@ nsPermissionManager::GetPermissionHashKey(nsIURI* aURI,
nsresult rv = NS_OK;
if (aURI) {
rv = GetPrincipal(aURI, getter_AddRefs(principal));
} else {
principal =
mozilla::BasePrincipal::CreateContentPrincipal(aOriginNoSuffix);
}
MOZ_ASSERT_IF(NS_SUCCEEDED(rv),
PermissionAvailable(principal, mTypeArray[aType]));
@ -2488,8 +2454,7 @@ nsPermissionManager::GetPermissionHashKey(nsIURI* aURI,
nsresult rv;
RefPtr<PermissionKey> key =
aURI ? PermissionKey::CreateFromURI(aURI, rv)
: PermissionKey::CreateFromOriginNoSuffix(aOriginNoSuffix);
aURI ? PermissionKey::CreateFromURI(aURI, rv) : nullptr;
if (!key) {
return nullptr;
}
@ -2514,9 +2479,6 @@ nsPermissionManager::GetPermissionHashKey(nsIURI* aURI,
if (NS_WARN_IF(NS_FAILED(rv))) {
return nullptr;
}
} else {
principal =
mozilla::BasePrincipal::CreateContentPrincipal(aOriginNoSuffix);
}
RemoveFromPrincipal(principal, mTypeArray[aType]);
} else if (permEntry.mPermission == nsIPermissionManager::UNKNOWN_ACTION) {
@ -2534,15 +2496,9 @@ nsPermissionManager::GetPermissionHashKey(nsIURI* aURI,
nsCOMPtr<nsIURI> uri;
if (aURI) {
uri = GetNextSubDomainURI(aURI);
} else {
nsresult rv = NS_NewURI(getter_AddRefs(uri), aOriginNoSuffix);
if (NS_WARN_IF(NS_FAILED(rv))) {
return nullptr;
}
uri = GetNextSubDomainURI(uri);
}
if (uri) {
return GetPermissionHashKey(uri, EmptyCString(), aType, aExactHostMatch);
return GetPermissionHashKey(uri, aType, aExactHostMatch);
}
}

View File

@ -77,8 +77,6 @@ class nsPermissionManager final : public nsIPermissionManager,
static PermissionKey* CreateFromPrincipal(nsIPrincipal* aPrincipal,
nsresult& aResult);
static PermissionKey* CreateFromURI(nsIURI* aURI, nsresult& aResult);
static PermissionKey* CreateFromOriginNoSuffix(
const nsACString& aOriginNoSuffix);
explicit PermissionKey(const nsACString& aOrigin)
: mOrigin(aOrigin), mHashCode(mozilla::HashString(aOrigin)) {}
@ -327,9 +325,8 @@ class nsPermissionManager final : public nsIPermissionManager,
PermissionHashKey* GetPermissionHashKey(nsIPrincipal* aPrincipal,
uint32_t aType, bool aExactHostMatch);
PermissionHashKey* GetPermissionHashKey(nsIURI* aURI,
const nsACString& aOriginNoSuffix,
uint32_t aType, bool aExactHostMatch);
PermissionHashKey* GetPermissionHashKey(nsIURI* aURI, uint32_t aType,
bool aExactHostMatch);
// The int32_t is the type index, the nsresult is an early bail-out return
// code.
@ -436,8 +433,8 @@ class nsPermissionManager final : public nsIPermissionManager,
}
return CommonTestPermissionInternal(
aPrincipal, nullptr, EmptyCString(), preparationResult.as<int32_t>(),
aType, aPermission, aExactHostMatch, aIncludingSession);
aPrincipal, nullptr, preparationResult.as<int32_t>(), aType,
aPermission, aExactHostMatch, aIncludingSession);
}
// If aTypeIndex is passed -1, we try to inder the type index from aType.
nsresult CommonTestPermission(nsIURI* aURI, int32_t aTypeIndex,
@ -453,14 +450,16 @@ class nsPermissionManager final : public nsIPermissionManager,
}
return CommonTestPermissionInternal(
nullptr, aURI, EmptyCString(), preparationResult.as<int32_t>(), aType,
aPermission, aExactHostMatch, aIncludingSession);
nullptr, aURI, preparationResult.as<int32_t>(), aType, aPermission,
aExactHostMatch, aIncludingSession);
}
// Only one of aPrincipal or aURI is allowed to be passed in.
nsresult CommonTestPermissionInternal(
nsIPrincipal* aPrincipal, nsIURI* aURI, const nsACString& aOriginNoSuffix,
int32_t aTypeIndex, const nsACString& aType, uint32_t* aPermission,
bool aExactHostMatch, bool aIncludingSession);
nsresult CommonTestPermissionInternal(nsIPrincipal* aPrincipal, nsIURI* aURI,
int32_t aTypeIndex,
const nsACString& aType,
uint32_t* aPermission,
bool aExactHostMatch,
bool aIncludingSession);
nsresult OpenDatabase(nsIFile* permissionsFile);
nsresult InitDB(bool aRemoveFile);

View File

@ -21,13 +21,6 @@ function run_test() {
pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"),
pm.ALLOW_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
sub1Principal.originNoSuffix,
"test/subdomains"
),
pm.ALLOW_ACTION
);
// A sub-sub-domain should get the permission.
let subsubPrincipal = getPrincipalFromURI("http://sub.sub1.example.com");
@ -35,13 +28,6 @@ function run_test() {
pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"),
pm.ALLOW_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
subsubPrincipal.originNoSuffix,
"test/subdomains"
),
pm.ALLOW_ACTION
);
// Another sub-domain shouldn't get the permission.
let sub2Principal = getPrincipalFromURI("http://sub2.example.com");
@ -49,13 +35,6 @@ function run_test() {
pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
sub2Principal.originNoSuffix,
"test/subdomains"
),
pm.UNKNOWN_ACTION
);
// Remove current permissions.
pm.removeFromPrincipal(sub1Principal, "test/subdomains");
@ -63,13 +42,6 @@ function run_test() {
pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
sub1Principal.originNoSuffix,
"test/subdomains"
),
pm.UNKNOWN_ACTION
);
// Adding the permission to the main domain. Checks if it is working.
let mainPrincipal = getPrincipalFromURI("http://example.com");
@ -78,48 +50,20 @@ function run_test() {
pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"),
pm.ALLOW_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
mainPrincipal.originNoSuffix,
"test/subdomains"
),
pm.ALLOW_ACTION
);
// All sub-domains should have the permission now.
Assert.equal(
pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"),
pm.ALLOW_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
sub1Principal.originNoSuffix,
"test/subdomains"
),
pm.ALLOW_ACTION
);
Assert.equal(
pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"),
pm.ALLOW_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
sub2Principal.originNoSuffix,
"test/subdomains"
),
pm.ALLOW_ACTION
);
Assert.equal(
pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"),
pm.ALLOW_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
subsubPrincipal.originNoSuffix,
"test/subdomains"
),
pm.ALLOW_ACTION
);
// Remove current permissions.
pm.removeFromPrincipal(mainPrincipal, "test/subdomains");
@ -127,46 +71,18 @@ function run_test() {
pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
mainPrincipal.originNoSuffix,
"test/subdomains"
),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
sub1Principal.originNoSuffix,
"test/subdomains"
),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
sub2Principal.originNoSuffix,
"test/subdomains"
),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
subsubPrincipal.originNoSuffix,
"test/subdomains"
),
pm.UNKNOWN_ACTION
);
// A sanity check that the previous implementation wasn't passing...
let crazyPrincipal = getPrincipalFromURI("http://com");
@ -175,55 +91,20 @@ function run_test() {
pm.testPermissionFromPrincipal(crazyPrincipal, "test/subdomains"),
pm.ALLOW_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
crazyPrincipal.originNoSuffix,
"test/subdomains"
),
pm.ALLOW_ACTION
);
Assert.equal(
pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
mainPrincipal.originNoSuffix,
"test/subdomains"
),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
sub1Principal.originNoSuffix,
"test/subdomains"
),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
sub2Principal.originNoSuffix,
"test/subdomains"
),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"),
pm.UNKNOWN_ACTION
);
Assert.equal(
pm.testPermissionOriginNoSuffix(
subsubPrincipal.originNoSuffix,
"test/subdomains"
),
pm.UNKNOWN_ACTION
);
}

View File

@ -219,21 +219,6 @@ interface nsIPermissionManager : nsISupports
uint32_t testPermissionFromPrincipal(in nsIPrincipal principal,
in ACString type);
/**
* Test whether a website specified by a given origin string has permission
* to perform the given action. This function is similar to testPermission()
* and is intended to be used where the cost of parsing a URI in the common
* case is to be avoided.
* @param originNoSuffix the origin string to be tested.
* @param type a case-sensitive ASCII string, identifying the
* permission.
* @param return see add(), param permission. returns UNKNOWN_ACTION
* when there is no stored permission for this uri and/
* or type.
*/
uint32_t testPermissionOriginNoSuffix(in ACString originNoSuffix,
in ACString type);
/**
* Test whether the principal associated with the window's document has the
* permission to perform a given action. System principals will always