Bug 1477977 - [marionette] Support dynamic handling of modal dialogs and tab modal alerts. r=maja_zf

With this patch Marionette registers globally for the dialog notifications
and events while a session is active. Also it provides an interface for
custom dialog handlers to hook in.

Instead of the callbacks custom events could have been fired, but that would
be some more work, and should preferable be done in a follow-up bug.

Differential Revision: https://phabricator.services.mozilla.com/D34139

--HG--
extra : moz-landing-system : lando

testing/marionette/driver.js

@@ -149,9 +149,9 @@ this.GeckoDriver = function(server) {
   this.listener = proxy.toListener(
       this.sendAsync.bind(this), () => this.curBrowser);
 
-  // points to an alert instance if a modal dialog is present
+  // used for modal dialogs or tab modal alerts
   this.dialog = null;
-  this.dialogHandler = this.modalDialogHandler.bind(this);
+  this.dialogObserver = null;
 };
 
 Object.defineProperty(GeckoDriver.prototype, "a11yChecks", {
@@ -304,16 +304,16 @@ GeckoDriver.prototype.uninit = function() {
  * Callback used to observe the creation of new modal or tab modal dialogs
  * during the session's lifetime.
  */
-GeckoDriver.prototype.modalDialogHandler = function(subject, topic) {
-  logger.trace(`Received observer notification ${topic}`);
+GeckoDriver.prototype.handleModalDialog = function(action, dialog, win) {
+  // Only care about modals of the currently selected window.
+  if (win !== this.curBrowser.window) {
+    return;
+  }
 
-  switch (topic) {
-    case modal.COMMON_DIALOG_LOADED:
-    case modal.TABMODAL_DIALOG_LOADED:
-      // Always keep a weak reference to the current dialog
-      let winRef = Cu.getWeakReference(subject);
-      this.dialog = new modal.Dialog(() => this.curBrowser, winRef);
-      break;
+  if (action === modal.ACTION_OPENED) {
+    this.dialog = new modal.Dialog(() => this.curBrowser, dialog);
+  } else if (action === modal.ACTION_CLOSED) {
+    this.dialog = null;
   }
 };
 
@@ -797,9 +797,11 @@ GeckoDriver.prototype.newSession = async function(cmd) {
     this.curBrowser.contentBrowser.focus();
   }
 
-  // Setup global listener for modal dialogs, and check if there is already
-  // one open for the currently selected browser window.
-  modal.addHandler(this.dialogHandler);
+  // Setup observer for modal dialogs
+  this.dialogObserver = new modal.DialogObserver(this);
+  this.dialogObserver.add(this.handleModalDialog.bind(this));
+
+  // Check if there is already an open dialog for the selected browser window.
   this.dialog = modal.findModalDialogs(this.curBrowser);
 
   return {
@@ -2886,7 +2888,10 @@ GeckoDriver.prototype.deleteSession = function() {
     this.observing = null;
   }
 
-  modal.removeHandler(this.dialogHandler);
+  if (this.dialogObserver) {
+    this.dialogObserver.cleanup();
+    this.dialogObserver = null;
+  }
 
   this.sandboxes.clear();
   CertificateOverrideManager.uninstall();
@@ -3178,8 +3183,7 @@ GeckoDriver.prototype.dismissDialog = async function() {
   (button1 ? button1 : button0).click();
 
   await dialogClosed;
-
-  this.dialog = null;
+  await new IdlePromise(win);
 };
 
 /**
@@ -3196,8 +3200,7 @@ GeckoDriver.prototype.acceptDialog = async function() {
   button0.click();
 
   await dialogClosed;
-
-  this.dialog = null;
+  await new IdlePromise(win);
 };
 
 /**

testing/marionette/harness/marionette_harness/tests/unit/test_modal_dialogs.py

@@ -192,20 +192,24 @@ class TestModalAlerts(BaseAlertTestCase):
 
     def setUp(self):
         super(TestModalAlerts, self).setUp()
-        self.marionette.set_pref("network.auth.non-web-content-triggered-resources-http-auth-allow",
-                                 True)
+        self.marionette.set_pref(
+            "network.auth.non-web-content-triggered-resources-http-auth-allow",
+            True,
+        )
+
+        self.new_tab = self.open_tab()
+        self.marionette.switch_to_window(self.new_tab)
 
     def tearDown(self):
-        # Ensure to close a possible remaining modal dialog
-        self.close_all_windows()
-        self.marionette.clear_pref("network.auth.non-web-content-triggered-resources-http-auth-allow")
+        self.close_all_tabs()
+        self.marionette.clear_pref(
+            "network.auth.non-web-content-triggered-resources-http-auth-allow")
 
         super(TestModalAlerts, self).tearDown()
 
     def test_http_auth_dismiss(self):
         self.marionette.navigate(self.marionette.absolute_url("http_auth"))
         self.wait_for_alert(timeout=self.marionette.timeout.page_load)
-
         alert = self.marionette.switch_to_alert()
         alert.dismiss()
 

testing/marionette/modal.js

@@ -5,6 +5,11 @@
 "use strict";
 
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
+
+const {Log} = ChromeUtils.import("chrome://marionette/content/log.js");
+
+XPCOMUtils.defineLazyGetter(this, "logger", Log.get);
 
 this.EXPORTED_SYMBOLS = ["modal"];
 
@@ -15,38 +20,8 @@ const isFirefox = () =>
 
 /** @namespace */
 this.modal = {
-  COMMON_DIALOG_LOADED: "common-dialog-loaded",
-  TABMODAL_DIALOG_LOADED: "tabmodal-dialog-loaded",
-  handlers: {
-    "common-dialog-loaded": new Set(),
-    "tabmodal-dialog-loaded": new Set(),
-  },
-};
-
-/**
- * Add handler that will be called when a global- or tab modal dialogue
- * appears.
- *
- * This is achieved by installing observers for common-
- * and tab modal loaded events.
- *
- * This function is a no-op if called on any other product than Firefox.
- *
- * @param {function(Object, string)} handler
- *     The handler to be called, which is passed the
- *     subject (e.g. ChromeWindow) and the topic (one of
- *     {@code modal.COMMON_DIALOG_LOADED} or
- *     {@code modal.TABMODAL_DIALOG_LOADED}.
- */
-modal.addHandler = function(handler) {
-  if (!isFirefox()) {
-    return;
-  }
-
-  Object.keys(this.handlers).map(topic => {
-    this.handlers[topic].add(handler);
-    Services.obs.addObserver(handler, topic);
-  });
+  ACTION_CLOSED: "closed",
+  ACTION_OPENED: "opened",
 };
 
 /**
@@ -89,32 +64,109 @@ modal.findModalDialogs = function(context) {
 };
 
 /**
- * Remove modal dialogue handler by function reference.
+ * Observer for modal and tab modal dialogs.
  *
- * This function is a no-op if called on any other product than Firefox.
- *
- * @param {function} toRemove
- *     The handler previously passed to modal.addHandler which will now
- *     be removed.
+ * @return {modal.DialogObserver}
+ *     Returns instance of the DialogObserver class.
  */
-modal.removeHandler = function(toRemove) {
-  if (!isFirefox()) {
-    return;
+modal.DialogObserver = class {
+  constructor() {
+    this.callbacks = new Set();
+    this.register();
   }
 
-  for (let topic of Object.keys(this.handlers)) {
-    let handlers = this.handlers[topic];
-    for (let handler of handlers) {
-      if (handler == toRemove) {
-        Services.obs.removeObserver(handler, topic);
-        handlers.delete(handler);
-      }
+  register() {
+    Services.obs.addObserver(this, "common-dialog-loaded");
+    Services.obs.addObserver(this, "tabmodal-dialog-loaded");
+    Services.obs.addObserver(this, "toplevel-window-ready");
+
+    // Register event listener for all already open windows
+    for (let win of Services.wm.getEnumerator(null)) {
+      win.addEventListener("DOMModalDialogClosed", this);
     }
   }
+
+  unregister() {
+    Services.obs.removeObserver(this, "common-dialog-loaded");
+    Services.obs.removeObserver(this, "tabmodal-dialog-loaded");
+    Services.obs.removeObserver(this, "toplevel-window-ready");
+
+    // Unregister event listener for all open windows
+    for (let win of Services.wm.getEnumerator(null)) {
+      win.removeEventListener("DOMModalDialogClosed", this);
+    }
+  }
+
+  cleanup() {
+    this.callbacks.clear();
+    this.unregister();
+  }
+
+  handleEvent(event) {
+    logger.trace(`Received event ${event.type}`);
+
+    let chromeWin = event.target.opener ? event.target.opener.ownerGlobal :
+        event.target.ownerGlobal;
+
+    let targetRef = Cu.getWeakReference(event.target);
+
+    this.callbacks.forEach(callback => {
+      callback(modal.ACTION_CLOSED, targetRef, chromeWin);
+    });
+  }
+
+  observe(subject, topic) {
+    logger.trace(`Received observer notification ${topic}`);
+
+    switch (topic) {
+      case "common-dialog-loaded":
+      case "tabmodal-dialog-loaded":
+        let chromeWin = subject.opener ? subject.opener.ownerGlobal :
+            subject.ownerGlobal;
+
+        // Always keep a weak reference to the current dialog
+        let targetRef = Cu.getWeakReference(subject);
+
+        this.callbacks.forEach(callback => {
+          callback(modal.ACTION_OPENED, targetRef, chromeWin);
+        });
+        break;
+
+      case "toplevel-window-ready":
+        subject.addEventListener("DOMModalDialogClosed", this);
+        break;
+    }
+  }
+
+  /**
+   * Add dialog handler by function reference.
+   *
+   * @param {function} callback
+   *     The handler to be added.
+   */
+  add(callback) {
+    if (this.callbacks.has(callback)) {
+      return;
+    }
+    this.callbacks.add(callback);
+  }
+
+  /**
+   * Remove dialog handler by function reference.
+   *
+   * @param {function} callback
+   *     The handler to be removed.
+   */
+  remove(callback) {
+    if (!this.callbacks.has(callback)) {
+      return;
+    }
+    this.callbacks.delete(callback);
+  }
 };
 
 /**
- * Represents the current modal dialogue.
+ * Represents a modal dialog.
  *
  * @param {function(): browser.Context} curBrowserFn
  *     Function that returns the current |browser.Context|.

testing/marionette/proxy.js

@@ -78,7 +78,7 @@ proxy.AsyncMessageChannel = class {
     this.activeMessageId = null;
 
     this.listeners_ = new Map();
-    this.dialogueObserver_ = null;
+    this.dialogHandler = null;
     this.closeHandler = null;
   }
 
@@ -164,7 +164,12 @@ proxy.AsyncMessageChannel = class {
       // A modal or tab modal dialog has been opened. To be able to handle it,
       // the active command has to be aborted. Therefore remove all handlers,
       // and cancel any ongoing requests in the listener.
-      this.dialogueObserver_ = (subject, topic) => {
+      this.dialogHandler = (action, dialogRef, win) => {
+        // Only care about modals of the currently selected window.
+        if (win !== this.browser.window) {
+          return;
+        }
+
         this.removeAllListeners_();
         // TODO(ato): It's not ideal to have listener specific behaviour here:
         this.sendAsync("cancelRequest");
@@ -187,7 +192,7 @@ proxy.AsyncMessageChannel = class {
    * Add all necessary handlers for events and observer notifications.
    */
   addHandlers() {
-    modal.addHandler(this.dialogueObserver_);
+    this.browser.driver.dialogObserver.add(this.dialogHandler.bind(this));
 
     // Register event handlers in case the command closes the current
     // tab or window, and the promise has to be escaped.
@@ -206,7 +211,7 @@ proxy.AsyncMessageChannel = class {
    * Remove all registered handlers for events and observer notifications.
    */
   removeHandlers() {
-    modal.removeHandler(this.dialogueObserver_);
+    this.browser.driver.dialogObserver.remove(this.dialogHandler.bind(this));
 
     if (this.browser) {
       this.browser.window.removeEventListener("unload", this.closeHandler);

testing/marionette/test/unit/test_modal.js

@@ -0,0 +1,115 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+const {InvalidArgumentError} = ChromeUtils.import("chrome://marionette/content/error.js");
+const {modal} = ChromeUtils.import("chrome://marionette/content/modal.js");
+
+const mockModalDialog = {
+  opener: {
+    ownerGlobal: "foo",
+  },
+};
+
+const mockTabModalDialog = {
+  ownerGlobal: "foo",
+};
+
+add_test(function test_addCallback() {
+  let observer = new modal.DialogObserver();
+  let cb1 = () => true;
+  let cb2 = () => false;
+
+  equal(observer.callbacks.size, 0);
+  observer.add(cb1);
+  equal(observer.callbacks.size, 1);
+  observer.add(cb1);
+  equal(observer.callbacks.size, 1);
+  observer.add(cb2);
+  equal(observer.callbacks.size, 2);
+
+  run_next_test();
+});
+
+add_test(function test_removeCallback() {
+  let observer = new modal.DialogObserver();
+  let cb1 = () => true;
+  let cb2 = () => false;
+
+  equal(observer.callbacks.size, 0);
+  observer.add(cb1);
+  observer.add(cb2);
+
+  equal(observer.callbacks.size, 2);
+  observer.remove(cb1);
+  equal(observer.callbacks.size, 1);
+  observer.remove(cb1);
+  equal(observer.callbacks.size, 1);
+  observer.remove(cb2);
+  equal(observer.callbacks.size, 0);
+
+  run_next_test();
+});
+
+add_test(function test_registerDialogClosedEventHandler() {
+  let observer = new modal.DialogObserver();
+  let mockChromeWindow = {
+    addEventListener(event, cb) {
+      equal(event, "DOMModalDialogClosed", "registered event for closing modal");
+      equal(cb, observer, "set itself as handler");
+      run_next_test();
+    },
+  };
+
+  observer.observe(mockChromeWindow, "toplevel-window-ready");
+});
+
+add_test(function test_handleCallbackOpenModalDialog() {
+  let observer = new modal.DialogObserver();
+
+  observer.add((action, target, win) => {
+    equal(action, modal.ACTION_OPENED, "'opened' action has been passed");
+    equal(target.get(), mockModalDialog, "weak reference has been created for target");
+    equal(win, mockModalDialog.opener.ownerGlobal, "chrome window has been passed");
+    run_next_test();
+  });
+  observer.observe(mockModalDialog, "common-dialog-loaded");
+});
+
+add_test(function test_handleCallbackCloseModalDialog() {
+  let observer = new modal.DialogObserver();
+
+  observer.add((action, target, win) => {
+    equal(action, modal.ACTION_CLOSED, "'closed' action has been passed");
+    equal(target.get(), mockModalDialog, "weak reference has been created for target");
+    equal(win, mockModalDialog.opener.ownerGlobal, "chrome window has been passed");
+    run_next_test();
+  });
+  observer.handleEvent({type: "DOMModalDialogClosed", target: mockModalDialog});
+});
+
+add_test(function test_handleCallbackOpenTabModalDialog() {
+  let observer = new modal.DialogObserver();
+
+  observer.add((action, target, win) => {
+    equal(action, modal.ACTION_OPENED, "'opened' action has been passed");
+    equal(target.get(), mockTabModalDialog, "weak reference has been created for target");
+    equal(win, mockTabModalDialog.ownerGlobal, "chrome window has been passed");
+    run_next_test();
+  });
+  observer.observe(mockTabModalDialog, "tabmodal-dialog-loaded");
+});
+
+add_test(function test_handleCallbackCloseTabModalDialog() {
+  let observer = new modal.DialogObserver();
+
+  observer.add((action, target, win) => {
+    equal(action, modal.ACTION_CLOSED, "'closed' action has been passed");
+    equal(target.get(), mockTabModalDialog, "weak reference has been created for target");
+    equal(win, mockTabModalDialog.ownerGlobal, "chrome window has been passed");
+    run_next_test();
+  });
+  observer.handleEvent({type: "DOMModalDialogClosed", target: mockTabModalDialog});
+});

testing/marionette/test/unit/xpcshell.ini

@@ -16,6 +16,7 @@ skip-if = appname == "thunderbird"
 [test_evaluate.js]
 [test_format.js]
 [test_message.js]
+[test_modal.js]
 [test_navigate.js]
 [test_prefs.js]
 [test_sync.js]