774819 - Handle system principals in nsPermissionManager when taking principals

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Mounir Lamouri (:mounir)]

Attachment: Patch

Comment 1

[Mounir Lamouri (:mounir)]

Attachment: Patch

Without compilation errors ;)

Comment 2

[Mounir Lamouri (:mounir)]

Attachment: Patch

... with `hg qref` :)

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 643091 [details] [diff] [review]
Patch

Review of attachment 643091 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm.. I feel like I commented on this one already but it might have gotten lost. I don't have connectivity right now so I can't check.

::: extensions/cookie/nsPermissionManager.cpp
@@ +514,5 @@
>  
> +  // We don't add the system principal because it actually has no URI and we
> +  // always allow action for them.
> +  if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
> +    return NS_OK;

Make this MOZ_ASSERT and return an error.

@@ +709,5 @@
>    NS_ENSURE_ARG_POINTER(aPrincipal);
>  
> +  // System principals are never added to the database, no need to remove them.
> +  if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
> +    return NS_OK;

Make this MOZ_ASSERT and return an error.

::: netwerk/base/public/nsIPermissionManager.idl
@@ +93,5 @@
>     * Add permission information for a given principal.
>     * It is internally calling the other add() method using the nsIURI from the
>     * principal.
> +   * Passing a system principal will be a no-op because they will always be
> +   * granted permissions.

Update this comment.

@@ +118,5 @@
>    /**
>     * Remove permission information for a given principal.
>     * This is internally calling remove() with the host from the principal's URI.
> +   * Passing system principal will be a no-op because we never add them to the
> +   * database.

Update this comment.

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d26ee2e87945