From 5cb79c7f0f568fb258ad39219cbedb9da947767b Mon Sep 17 00:00:00 2001 From: Raphael Kubo da Costa Date: Mon, 9 Dec 2019 10:40:49 +0100 Subject: [PATCH] testdriver.js: Make set_permission() take a PermissionDescriptor, not a name Making the public API take a permission name only works for permissions that take a `PermissionDescriptor`, and makes it impossible to set permissions that require extra information (i.e. they take a `MidiPermissionDescriptor` or a `PushPermissionDescriptor` instead). Instead of taking a string corresponding to a permission name, take a `PermissionDescriptor` directly so that all required information can be specified. While here, make `set_permission.html` an HTTPS test, as some permissions are only available in a secure context (such as "push", which is being used to test setting a permission that requires extra parameters in the descriptor). Fixes #20672. --- ...ission.html.ini => set_permission.https.html.ini} | 2 +- ...set_permission.html => set_permission.https.html} | 4 ++-- resources/testdriver.js | 12 ++++++------ tools/wptrunner/wptrunner/executors/base.py | 2 +- .../wptrunner/executors/executorwebdriver.py | 6 ++---- tools/wptrunner/wptrunner/executors/protocol.py | 4 ++-- 6 files changed, 14 insertions(+), 16 deletions(-) rename infrastructure/metadata/infrastructure/testdriver/{set_permission.html.ini => set_permission.https.html.ini} (62%) rename infrastructure/testdriver/{set_permission.html => set_permission.https.html} (71%) diff --git a/infrastructure/metadata/infrastructure/testdriver/set_permission.html.ini b/infrastructure/metadata/infrastructure/testdriver/set_permission.https.html.ini similarity index 62% rename from infrastructure/metadata/infrastructure/testdriver/set_permission.html.ini rename to infrastructure/metadata/infrastructure/testdriver/set_permission.https.html.ini index 9a250edf1a918b..2101196fa2fe43 100644 --- a/infrastructure/metadata/infrastructure/testdriver/set_permission.html.ini +++ b/infrastructure/metadata/infrastructure/testdriver/set_permission.https.html.ini @@ -1,3 +1,3 @@ -[set_permission.html] +[set_permission.https.html] expected: if product != "chrome": ERROR diff --git a/infrastructure/testdriver/set_permission.html b/infrastructure/testdriver/set_permission.https.html similarity index 71% rename from infrastructure/testdriver/set_permission.html rename to infrastructure/testdriver/set_permission.https.html index 1e92a26398d321..af743f63828960 100644 --- a/infrastructure/testdriver/set_permission.html +++ b/infrastructure/testdriver/set_permission.https.html @@ -8,10 +8,10 @@ diff --git a/resources/testdriver.js b/resources/testdriver.js index 22d5ead72267a3..f102c8774f7efc 100644 --- a/resources/testdriver.js +++ b/resources/testdriver.js @@ -216,7 +216,9 @@ * This function simulates a user setting a permission into a particular state as described * in {@link https://w3c.github.io/permissions/#set-permission-command} * - * @param {String} name - the name of the permission + * @param {Object} descriptor - a [PermissionDescriptor]{@link + * https://w3c.github.io/permissions/#dictdef-permissiondescriptor} + * object * @param {String} state - the state of the permission * @param {boolean} one_realm - Optional. Whether the permission applies to only one realm * @@ -226,12 +228,10 @@ * @returns {Promise} fulfilled after the permission is set, or rejected if setting the * permission fails */ - set_permission: function(name, state, one_realm) { + set_permission: function(descriptor, state, one_realm) { let permission_params = { - descriptor: { - name: name - }, - state: state, + descriptor, + state, oneRealm: one_realm, }; return window.test_driver_internal.set_permission(permission_params); diff --git a/tools/wptrunner/wptrunner/executors/base.py b/tools/wptrunner/wptrunner/executors/base.py index 5e55f33ee1f5f4..a1ce92a0025235 100644 --- a/tools/wptrunner/wptrunner/executors/base.py +++ b/tools/wptrunner/wptrunner/executors/base.py @@ -782,7 +782,7 @@ def __call__(self, payload): state = permission_params["state"] one_realm = permission_params.get("oneRealm", False) self.logger.debug("Setting permission %s to %s, oneRealm=%s" % (name, state, one_realm)) - self.protocol.set_permission.set_permission(name, state, one_realm) + self.protocol.set_permission.set_permission(descriptor, state, one_realm) class AddVirtualAuthenticatorAction(object): def __init__(self, logger, protocol): diff --git a/tools/wptrunner/wptrunner/executors/executorwebdriver.py b/tools/wptrunner/wptrunner/executors/executorwebdriver.py index 8ea59b2577d082..221f48a3ea9cb8 100644 --- a/tools/wptrunner/wptrunner/executors/executorwebdriver.py +++ b/tools/wptrunner/wptrunner/executors/executorwebdriver.py @@ -208,11 +208,9 @@ class WebDriverSetPermissionProtocolPart(SetPermissionProtocolPart): def setup(self): self.webdriver = self.parent.webdriver - def set_permission(self, name, state, one_realm): + def set_permission(self, descriptor, state, one_realm): permission_params_dict = { - "descriptor": { - "name": name - }, + "descriptor": descriptor, "state": state, } if one_realm is not None: diff --git a/tools/wptrunner/wptrunner/executors/protocol.py b/tools/wptrunner/wptrunner/executors/protocol.py index 1a47fa9f1d8cc4..a1e8dacecb695a 100644 --- a/tools/wptrunner/wptrunner/executors/protocol.py +++ b/tools/wptrunner/wptrunner/executors/protocol.py @@ -312,10 +312,10 @@ class SetPermissionProtocolPart(ProtocolPart): name = "set_permission" @abstractmethod - def set_permission(self, name, state, one_realm=False): + def set_permission(self, descriptor, state, one_realm=False): """Set permission state. - :param name: The name of the permission to set. + :param descriptor: A PermissionDescriptor object. :param state: The state to set the permission to. :param one_realm: Whether to set the permission for only one realm.""" pass