1723592 - MessageHandler: Modules should explicitly declare their public commands and events

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Description

[Julian Descottes [:jdescottes]]

In Bug 1713439, we consider that all the methods implemented by a module class can be called as commands.

See the following method: https://searchfox.org/mozilla-central/rev/7b1de5e29d878cc163dec7beaf9b57a2f0f41aaa/remote/shared/messagehandler/MessageHandler.jsm#135-141

  _isCommandSupportedByModule(commandName, mod) {
    // TODO: With the current implementation, all functions of a given module
    // are considered as valid commands.
    // This should probably be replaced by a more explicit declaration, via a
    // manifest for instance.
    return mod && typeof mod[commandName] === "function";
  }

However we should prevent external consumers from calling methods which are not intended as public commands.

The goal of this bug is to add a proper implementation for _isCommandSupportedByModule which instead of checking for the existence of a function will check against a list exposed by the module. Alternatively, if we have a base Module class, we could add a generic supportsCommand method on this base class.

Note that internally, Modules also rely on commands to call one another. For those internal calls, we should not be limited to only public methods. So we need this API to make the distinction between an external call and an internal one. For internal calls, we might just reuse the current logic (ie allow to call any implemented method).

In a first step we don't necessarily have to support this granularity and we can just add all necessary commands as public ones.

Comment 1

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

To be able to determine if a module supports a given command we definitely need this feature in M2.

Comment 2

[Julian Descottes [:jdescottes]]

Updating the title to also mention "events", which would be useful in order to move the subscribe logic fully to the root session module.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1723592 - [remote] Throw InvalidArgumentException when passing invalid events to session subscribe/unsubscribe

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1723592 - [wdspec] Update wdspec tests to test unsupported events in session module

Depends on D132972

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0a8287451b3
[remote] Throw InvalidArgumentException when passing invalid events to session subscribe/unsubscribe r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/aaf6e76bcdb7
[wdspec] Update wdspec tests to test unsupported events in session module r=webdriver-reviewers,whimboo

Comment 6

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

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

Comment 7

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/d0a8287451b3
https://hg.mozilla.org/mozilla-central/rev/aaf6e76bcdb7

Comment 8

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

Upstream PR merged by moz-wptsync-bot