1743083 - MessageHandler Modules can be initialized before the corresponding MessageHandler is ready

Status: RESOLVED FIXED

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

Description

[Julian Descottes [:jdescottes]]

When creating modules to handle initial session data, the MessageHandler instance owning the module will not be fully initialized.

See the constructor for MessageHandler and WindowGlobalMessageHandler

class MessageHandler extends EventEmitter {
  constructor(sessionId, context, sessionDataItems) {
    super();
    // ...

    if (Array.isArray(sessionDataItems)) {
      this._applyInitialSessionDataItems(sessionDataItems);
    }
  }

class WindowGlobalMessageHandler extends MessageHandler {
  constructor() {
    super(...arguments);

    this._innerWindowId = this._context.window.windowGlobalChild.innerWindowId;
  }

If there is relevant session data, modules will be initialized at the end of the MessageHandler constructor, before the WindowGlobalMessageHandler could create its _innerWindowId. Meaning that if a module tries to access the information in their own constructor, they will get undefined.

The call to _applyInitialSessionDataItems should only be done after the constructor has returned, so that module are guaranteed to have a fully functional message handler instance.

Comment 1

[Julian Descottes [:jdescottes]]

Considering that MessageHandler are only instantiated from the MessageHandlerRegistry, I would simply suggest updating the following method:

_createMessageHandler(sessionId, sessionDataItems) {
    const messageHandler = new this._messageHandlerClass(
      sessionId,
      this._context,
      sessionDataItems
    );

to do

_createMessageHandler(sessionId, sessionDataItems) {
    const messageHandler = new this._messageHandlerClass(
      sessionId,
      this._context,
    );
    messageHandler.applyInitialSessionDataItems(sessionDataItems);

Comment 2

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

Yes, that sounds like the right approach to me too.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1743083 - [remote] Instantiate MessageHandler modules after constructor has returned

Depends on D132064

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0d8dbce59b1f
[remote] Instantiate MessageHandler modules after constructor has returned r=webdriver-reviewers,whimboo

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/0d8dbce59b1f