1879363 - browsingContext module event subscription is inconsistent

Status: RESOLVED FIXED

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

Description

[Julian Descottes [:jdescottes] (PTO -> Jan 5th)]

After adding several events to the browsingContext module, the current code looks a bit inconsistent:

  #unsubscribeEvent(event) {
    switch (event) {
      case "browsingContext.contextCreated":
      case "browsingContext.contextDestroyed": {
        this.#contextListener.stopListening();
        this.#subscribedEvents.delete(event);
        break;
      }
      case "browsingContext.fragmentNavigated":
      case "browsingContext.navigationStarted": {
        this.#stopListeningToNavigationEvent();
        this.#subscribedEvents.delete(event);
        break;
      }
      case "browsingContext.userPromptClosed":
      case "browsingContext.userPromptOpened": {
        this.#stopListeningToPromptEvent(event);
        break;
      }
    }
  }

Each block uses a different pattern for no strong reason, also stopListeningToNavigationEvent is supposed to take an "event" parameter, but here we call it without any parameter.

The contextCreated/contextDestroyed block is a bit worrying because it makes you think we will stop the listener if we unsubscribe from just one of the 2 events. It actually works because applySessionData ALWAYS calls #subscribeEvent for all events. But that means if you do something like:

• subscribe([contextCreated, contextDestroyed])
• unsubscribe([contextDestroyed])
  The unsubscribe will stop the listener and start it immediately afterwards. We should rather reapply the same pattern as for the other events to avoid adding/removing observers unnecessarily.

Comment 1

[Julian Descottes [:jdescottes] (PTO -> Jan 5th)]

Attachment: Bug 1879363 - [wdspec] Use consistent pattern for all event subsriptions in browsingContext module

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e74d1619128b
[wdspec] Use consistent pattern for all event subsriptions in browsingContext module r=webdriver-reviewers,Sasha

Comment 3

[Noemi Erli[:noemi_erli]]

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