1780747 - [DNR] Add DNR API bindings and permissions

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, task, P1)

Description

[Rob Wu [:robwu]]

The registration of the declarativeNetRequest API and its permissions is not trivial, because it has three permissions, and one of them has a permission warning which requires coordination with an external repository (Android-Components, referenced in bug 1671453).

This bug is a placeholder to attach the patch that introduces the API bindings, permissions and unit tests for it (behind new default-off preferences).

I'm linking bug 1490797 to this one, because the introduction of a new permission-with-warning triggers the conditions described in that bug. I'm not marking it as a blocker, since the new API is disabled by default.

To improve the visibility of the work needed to introduce a new permission with a permission warning for future reference, I'm linking this to bug 1671453.

Comment 1

[Rob Wu [:robwu]]

Attachment: Bug 1780747 - Register DNR schema and permissions

This patch adds the minimum necessary to register the
declarativeNetRequest API and its permissions, behind prefs.

Tests have been added/updated to verify that the permissions and API
access are enforced correctly (effectiveness of preferences, API
visibility, permission warnings).

Before landing this, we need to register the permission warning in
Android-Components too, as mentioned in the bug (i.e. bug 1671453).

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Rob Wu [:robwu] from comment #1)

Before landing this, we need to register the permission warning in
Android-Components too, as mentioned in the bug (i.e. bug 1671453).

while I was reading this last part of the patch description, I was wondering:

• does the addition of the permission warning in the android components really need to block this while the about:config pref is still disabled by default?
• have we already considered to also keep the new API disabled (independently from the pref value) in non-nightly builds?
  (until we have finalized the permission strings to be shown and got enough of the API to make it useful and something that the extension developers may start to try it out)

Comment 3

[Rob Wu [:robwu]]

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #2)

(In reply to Rob Wu [:robwu] from comment #1)

Before landing this, we need to register the permission warning in
Android-Components too, as mentioned in the bug (i.e. bug 1671453).

while I was reading this last part of the patch description, I was wondering:

• does the addition of the permission warning in the android components really need to block this while the about:config pref is still disabled by default?

In this specific case, it isn't strictly necessary to be blocked on Android-Components, because the permission won't be seen on Android by default (with the implementation in the patch that filters out the permission). In general, it is recommended to not forget about the permission, or else the Android application would crash.

• have we already considered to also keep the new API disabled (independently from the pref value) in non-nightly builds?
  (until we have finalized the permission strings to be shown and got enough of the API to make it useful and something that the extension developers may start to try it out)

The pref being off-by-default should be sufficient. I am however open to disabling the feature by default on Nightly if we really want to prevent pre-release usage of this feature. In that case we would have to skip some tests too.

Comment 4

[Rob Wu [:robwu]]

Attachment: Bug 1780747 - Restrict DNR permissions to MV3 for now

Restrict the DNR permissions to MV3 to allow the add-ons linter to
easily flag the use of the not-yet-supported DNR permission, until
we do actually enable the feature.

Since the full DNR namespace is gated on this permission, this
effectively means that in order to use the API, not only the
extensions.dnr.enabled pref needs to be set, but also the
extensions.manifestV3.enabled pref + using manifest_version: 3..

Comment 5

[Rob Wu [:robwu]]

Created https://github.com/mozilla-mobile/android-components/issues/12616 as a follow-up for the A-C part.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b07c7315a02a
Register DNR schema and permissions r=rpl,geckoview-reviewers,owlish,flod
https://hg.mozilla.org/integration/autoland/rev/be950e847c33
Restrict DNR permissions to MV3 for now r=rpl

Comment 7

[Cristian Tuns]

Backed out for causing xpcshell failures on test_ext_permission_warnings.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_permission_warnings.js | declarativeNetRequest_permission_with_warning - [declarativeNetRequest_permission_with_warning : 498] Expected origins and permissions - {"permissions":[],"origins":[]} deepEqual {"origins":[],"permissions":["declarativeNetRequest"]}

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8149daf3084a
Register DNR schema and permissions r=rpl,geckoview-reviewers,owlish,flod
https://hg.mozilla.org/integration/autoland/rev/c9335a3ca699
Restrict DNR permissions to MV3 for now r=rpl

Comment 9

[Rob Wu [:robwu]]

(In reply to Cristian Tuns from comment #7)

Backed out for causing xpcshell failures on test_ext_permission_warnings.js

This was only in condprof tests, which is an issue on its own - handled in bug 1783828 (and follow-up in bug 1769184).

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/8149daf3084a
https://hg.mozilla.org/mozilla-central/rev/c9335a3ca699

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0ee0756abd0b
Port bug 1780747 - Register DNR schema and permissions. rs=bustage-fix