1714945 - Persist tracking protection exceptions in Private Browsing (for Focus)

Status: RESOLVED FIXED

(GeckoView :: General, defect, P1)

Description

[Ionut Bedregeanu]

Focus issue: https://github.com/mozilla-mobile/focus-android/issues/4730

Right now to save a tracking exception we are calling runtime.contentBlockingController.addException(geckoEngineSession.geckoSession) which calls addException method and saveExceptionList.
To get the saved exceptions list we are using accept method wich returns always an empty list, so we can't restore the previous state.
Could you please take a look over these Gecko methods to confirm us that it behaves as expected?

Comment 1

[[ex-Mozilla] Agi Sferro | :agi]

johannh, pbz: in Focus (an android browser that is permanently in Private Browsing) we would like to store tracking protection exceptions permanently. Currently we don't allow that and we force it to be per session 1, do you think we could add some android-only code that allows to add permanent exceptions in private browsing? do you know who would be the best person to talk to about this if not you?

Comment 2

[Paul Zühlcke [:pbz]]

The tracking protection exceptions are stored as permissions. The permission manager automatically gives all permission set for private browsing SESSION expiry.
We change the permission expiry here: https://searchfox.org/mozilla-central/rev/42ae3bea104c37a9986c6f18d17bd9ddb387129c/extensions/permissions/PermissionManager.cpp#1586-1589
We could either extend the nsIPermissionManager interface to add a flag to override the behavior, or we add an ifdef and skip the check for this specific permission in Focus.

Comment 3

[Johann Hofmann [:johannh]]

Yup, what Paul said, I don't have a strong opinion on which of the options we should pick, both sound feasible. Note that permissions may be set in various ways and you'll want to avoid leaking permissions other than the trackingprotection one. So either you add an allow-list as part of the ifdef or only use the flag in question when setting trackingprotection permissions.

Comment 4

[Paul Zühlcke [:pbz]]

A static pref would probably also work.

Comment 6

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1714945 - Actually deprecate ContentBlockingController.

Comment 7

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1714945 - Allow setting permanent permissions for private browsing

By default nsIPermissionManager will not persist data in private browsing.

There are cases however, like private-only-browser Firefox Focus, where we
would want to be able to persist tracking protection exceptions permanently.

This patch adds a new argument to addFromPrincipal which allows specifying
the behavior for private browsing windows.

The possible values are:

• BEHAVIOR_ALWAYS_EXPIRE_SESSION, same as before this patch, permissions will
  always expire with the session for private browsing.

• BEHAVIOR_ALLOW_EXPIRE_NEVER, will allow permissions to EXPIRE_NEVER in
  private browsing.

Comment 8

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1714945 - Clean up PermissionDelegateTest.

Removes redundant namespaces, adds some static imports, cleans up some java-ism
(like .equals and semicolons).

Comment 9

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1714945 - Add StorageController::setPrivateBrowsingPermanentPermission.

Normally, a private browsing session doesn't store any data permamently. This
extends to permissions, which are cleared at the end of the browsing session
(i.e. when the GeckoRuntime is closed or killed).

There are cases, however, where we want to keep the data around. For example
Focus needs to store tracking protection permissions permanently.

For this reason, this patch introduces a new API that allows embedders to set
permanent permissions for private browsing GeckoSession.

This patch also always sends a tracking protection permission for every page
load, this allows embedders to set the tracking protection permission without
using the deprecated API setPermission, this was an oversight during the
implementation of the new StorageController API.

Comment 10

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1714945 - Re-enable permissions tests on Android.

Comment 11

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1714945 - Wait until app is ready for xpcshell on Android.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2f395ee6c64
Actually deprecate ContentBlockingController. r=owlish
https://hg.mozilla.org/integration/autoland/rev/92687fa491f6
Allow setting permanent permissions for private browsing r=timhuang,johannh
https://hg.mozilla.org/integration/autoland/rev/2486153cbbce
Clean up PermissionDelegateTest. r=owlish
https://hg.mozilla.org/integration/autoland/rev/d15cdf6837e0
Add StorageController::setPrivateBrowsingPermanentPermission. r=owlish
https://hg.mozilla.org/integration/autoland/rev/5015aaadb432
Re-enable permissions tests on Android. r=johannh
https://hg.mozilla.org/integration/autoland/rev/b64759becddf
Wait until app is ready for xpcshell on Android. r=jmaher,gbrown

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b2f395ee6c64
https://hg.mozilla.org/mozilla-central/rev/92687fa491f6
https://hg.mozilla.org/mozilla-central/rev/2486153cbbce
https://hg.mozilla.org/mozilla-central/rev/d15cdf6837e0
https://hg.mozilla.org/mozilla-central/rev/5015aaadb432
https://hg.mozilla.org/mozilla-central/rev/b64759becddf

Comment 14

[Chris Peterson [:cpeterson]]

Move GeckoView::Tracking Protection bugs to the GeckoView::General component.