1524074 - Implement Permissions API extension for Marionette

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, enhancement, P2)

Description

[Andreas Tolfsen ❲:ato❳]

We are currently failing all Permissions API WebDriver extension tests:
https://wpt.fyi/results/webdriver/tests/permissions/set.py?label=master&label=experimental&product=chrome&product=firefox&product=safari

The WebDriver extensions are defined here:
https://w3c.github.io/permissions/#automation

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

James, what's the importance of that given that we seem to fail a lot of wpt tests. See also the comment from Johann over on bug 1671407.

Comment 3

[James Graham [:jgraham]]

Ah, apologies, I didn't realise there was already a bug for this.

The question on priorities is really for the people implementing the permissions features. I'd assume that the ability to test those features, and share the tests with Chrome, WebKit and others would be high priority.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Which other tests beside the following wdspec tests do we have?

Comment 5

[James Graham [:jgraham]]

Based on rg -l test_driver\..*permiss in testing/web-platform/tests:

screen_enumeration/screen_enumeration_permission.window.js
screen_enumeration/getScreens.values.https.html
screen_enumeration/isMultiScreen.tentative.https.window.js
screen_enumeration/getScreens.tentative.https.window.js
resources/chromium/generic_sensor_mocks.js
resources/chromium/nfc-mock.js
screen-wake-lock/wakelock-enabled-by-feature-policy.https.sub.html
screen-wake-lock/wakelockpermissiondescriptor.https.html
screen-wake-lock/idlharness.https.window.js
screen-wake-lock/wakelock-enabled-on-self-origin-by-feature-policy.https.sub.html
screen-wake-lock/wakelock-onrelease.https.html
screen-wake-lock/wakelock-request-denied.https.html
screen-wake-lock/wakelock-released.https.html
storage-access-api/requestStorageAccess.sub.window.js
idle-detection/idle-detection-allowed-by-feature-policy.https.sub.html
idle-detection/interceptor.https.html
idle-detection/basics.tentative.https.window.js
idle-detection/idle-detection-default-feature-policy.https.sub.html
idle-detection/idle-permission.tentative.https.window.js
idle-detection/idlharness-worker.https.window.js
idle-detection/idle-detection-allowed-by-feature-policy-attribute-redirect-on-load.https.sub.html
idle-detection/idlharness.https.window.js
idle-detection/idle-detection-allowed-by-feature-policy-attribute.https.sub.html
clipboard-apis/async-write-svg-read-svg.https.html
clipboard-apis/async-write-blobs-read-blobs.https.html
clipboard-apis/async-write-html-read-html.https.html
clipboard-apis/async-html-script-removal.https.html
clipboard-apis/async-write-image-read-image.https.html
clipboard-apis/async-svg-script-removal.https.html
web-nfc/nfc_permission.https.html
web-nfc/NDEFWriter_write.https.html
web-nfc/NDEFReader_scan.https.html
mediacapture-image/MediaStreamTrack-getCapabilities.html
mediacapture-image/MediaStreamTrack-applyConstraints.html
mediacapture-image/MediaStreamTrack-getSettings.html
mediacapture-image/MediaStreamTrack-applyConstraints-reject.html
mediacapture-image/MediaStreamTrack-applyConstraints-getSettings.html
generic-sensor/generic-sensor-permission.https.html
feature-policy/resources/feature-policy-screen-wakelock.html
feature-policy/resources/feature-policy-clipboard-write.html
feature-policy/resources/feature-policy-clipboard-read.html
docs/writing-tests/testdriver.md
font-access/resources/test-expectations.js
native-file-system/resources/native-fs-test-helpers.js
clipboard-apis/detached-iframe/writeText-readText-on-detached-iframe.https.html
clipboard-apis/detached-iframe/write-on-detaching-iframe.https.html
clipboard-apis/detached-iframe/write-read-on-detached-iframe.https.html
clipboard-apis/text-write-read/async-writeText-readText.https.html
clipboard-apis/text-write-read/async-writeText-read.https.html
clipboard-apis/detached-iframe/read-on-detaching-iframe.https.html
clipboard-apis/text-write-read/async-write-read.https.html
clipboard-apis/permissions/readText-granted.https.html
clipboard-apis/permissions/writeText-denied.https.html
clipboard-apis/text-write-read/async-write-readText.https.html
clipboard-apis/permissions/writeText-granted.https.html
clipboard-apis/permissions/readText-denied.https.html
fullscreen/api/element-request-fullscreen-options.tentative.html
infrastructure/testdriver/set_permission.https.html
clipboard-apis/feature-policy/clipboard-write/clipboard-write-enabled-on-self-origin-by-feature-policy.tentative.https.sub.html
clipboard-apis/feature-policy/clipboard-write/clipboard-write-disabled-by-feature-policy.tentative.https.sub.html
clipboard-apis/feature-policy/clipboard-write/clipboard-write-enabled-by-feature-policy.tentative.https.sub.html
clipboard-apis/feature-policy/clipboard-read/clipboard-read-enabled-on-self-origin-by-feature-policy.tentative.https.sub.html
clipboard-apis/feature-policy/clipboard-read/clipboard-read-disabled-by-feature-policy.tentative.https.sub.html
clipboard-apis/feature-policy/clipboard-read/clipboard-read-enabled-by-feature-policy.tentative.https.sub.html

Comment 6

[James Graham [:jgraham]]

Is this on the DOM radar? I'm trying to figure out what we need to do to make sure we get wpt coverage for these APIs that have additional WebDriver endpoints. Is this work blocked on something specific? Is there help required from the automation people?

Comment 7

[Anne (:annevk)]

With Andrea working on other things I don't think this has an owner in DOM currently. Maybe Johann can help out? (I initially wrote something about the tests listed above, but I did not see that was a scrollable box. It seems our coverage for quite a few APIs could be improved by this.)

Comment 8

[Johann Hofmann [:johannh]]

I don't think anyone on our team can work on this right now, though I will put it on our backlog. :|

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Right now permissions on Android are handled by the individual app, but not by Gecko itself. It means that we won't actually get full support across all platforms.

But that will change with bug 1654832. Once it's done Gecko itself will handle the permission flags.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Now with bug 1654832 fixed there should be a way for us to handle permissions in GeckoView. I had a look but didn't found the entry point. Agi, maybe you can give a quick hint in how our code in Marionette could utilize the GV permissions? Thanks!

Comment 11

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

What permissions are you looking to handle? for most of them now GeckoView uses Gecko's permission store so there shouldn't be anything special that you need to do for mobile specifically (just store the permission using nsIPermissionManager). Or maybe you need a higher level API than nsIPermissionManager?

Comment 13

[Vincent Hilla [:vhilla]]

:whimboo this bug blocks testing Screen Wake Lock, Bug 1589554. I can add a similar workaround using a pref to permissions.sys.mjs but what is preventing us from implementing this? I poked around a bit but my knowledge of marionette seems to not be enough to implement this.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We currently do not have the man power to get this feature implemented. As such it's in the backlog. So yes, you could update the code and do it similar as for other permissions if that works. We are happy to accept patches. Otherwise please let us know.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We are going to implement this API for WebDriver BiDi first (bug 1875065). So it should then be easier to get it added for Marionette as well.

Comment 16

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1524074 - Generalize permission handling of storage-access to all permissions r=whimboo

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Kagami, thank you for the patch but could you please move it to a different bug, which will block this one? There is more work to do to have a full implementation for Permissions in Marionette and that we want to do on this bug. Thanks!

Comment 18

[Kagami Rosylight [:saschanaz] (they/them)]

I'll do, thanks!

Comment 19

[Phabricator Automation]

Comment on attachment 9373310 [details]
Bug 1524074 - Generalize permission handling of storage-access to all permissions r=whimboo

Revision D198836 was moved to bug 1875257. Setting attachment 9373310 [details] to obsolete.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

On bug 1875724 the marionette.setpermission.enabled pref will be set for all web-platform tests. We should wait for that until we do the full implementation for Marionette.

Comment 21

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1524074 - [marionette] Add validation to "setPermission" API.

Comment 22

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1524074 - Enable "marionette.setpermission.enabled" preference for wpt tests.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/041acf255585
[marionette] Add validation to "setPermission" API. r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/7d9f74554923
Enable "marionette.setpermission.enabled" preference for wpt tests. r=webdriver-reviewers,whimboo

Comment 24

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46426 for changes under testing/web-platform/tests

Comment 25

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/041acf255585
https://hg.mozilla.org/mozilla-central/rev/7d9f74554923

Comment 26

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 28

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 29

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2fb2e755c8d
[wpt PR 46426] - [Gecko Bug 1524074] Enable "marionette.setpermission.enabled" preference for wpt tests., a=testonly

Comment 31

[Iulian Moraru]

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