Bug 1693857 - [marionette] Implicitly accept "beforeunload" prompts in Marionette. r=webdriver-reviewers,jdescottes

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

Author: whimboo

remote/components/Marionette.sys.mjs

@@ -40,13 +40,6 @@ const ENV_ENABLED = "MOZ_MARIONETTE";
 // pref being set to 4444.
 const ENV_PRESERVE_PREFS = "MOZ_MARIONETTE_PREF_STATE_ACROSS_RESTARTS";
 
-// Map of Marionette-specific preferences that should be set via
-// RecommendedPreferences.
-const RECOMMENDED_PREFS = new Map([
-  // Automatically unload beforeunload alerts
-  ["dom.disable_beforeunload", true],
-]);
-
 const isRemote =
   Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT;
 
@@ -147,7 +140,7 @@ class MarionetteParentProcess {
             Services.obs.addObserver(this, "domwindowopened");
           }
 
-          lazy.RecommendedPreferences.applyPreferences(RECOMMENDED_PREFS);
+          lazy.RecommendedPreferences.applyPreferences();
 
           // Only set preferences to preserve in a new profile
           // when Marionette is enabled.

remote/marionette/driver.sys.mjs

@@ -114,6 +114,9 @@ export function GeckoDriver(server) {
   // WebDriver Session
   this._currentSession = null;
 
+  // Flag to indicate that the application is shutting down
+  this._isShuttingDown = false;
+
   this.browsers = {};
 
   // points to current browser
@@ -211,7 +214,16 @@ GeckoDriver.prototype.handleClosedModalDialog = function () {
  */
 GeckoDriver.prototype.handleOpenModalDialog = function (eventName, data) {
   this.dialog = data.prompt;
-  this.getActor().notifyDialogOpened();
+
+  if (this.dialog.promptType === "beforeunload") {
+    lazy.logger.trace(`Implicitly accepted "beforeunload" prompt`);
+    this.dialog.accept();
+    return;
+  }
+
+  if (!this._isShuttingDown) {
+    this.getActor().notifyDialogOpened(this.dialog);
+  }
 };
 
 /**
@@ -2375,7 +2387,9 @@ GeckoDriver.prototype.deleteSession = function () {
   // reset to the top-most frame
   this.mainFrame = null;
 
-  if (this.promptListener) {
+  if (!this._isShuttingDown && this.promptListener) {
+    // Do not stop the prompt listener when quitting the browser to
+    // allow us to also accept beforeunload prompts during shutdown.
     this.promptListener.stopListening();
     this.promptListener = null;
   }
@@ -2792,6 +2806,12 @@ GeckoDriver.prototype._handleUserPrompts = async function () {
     return;
   }
 
+  if (this.dialog.promptType == "beforeunload") {
+    // Wait until the "beforeunload" prompt has been accepted.
+    await this.promptListener.dialogClosed();
+    return;
+  }
+
   const textContent = await this.dialog.getText();
 
   const behavior = this.currentSession.unhandledPromptBehavior;
@@ -2899,16 +2919,17 @@ GeckoDriver.prototype.quit = async function (cmd) {
 
   let quitApplicationResponse;
   try {
+    this._isShuttingDown = true;
     quitApplicationResponse = await lazy.quit(
       flags,
       safeMode,
       this.currentSession.capabilities.get("moz:windowless")
     );
   } catch (e) {
+    this._isShuttingDown = false;
     if (e instanceof TypeError) {
       throw new lazy.error.InvalidArgumentError(e.message);
     }
-
     throw new lazy.error.UnsupportedOperationError(e.message);
   } finally {
     Services.obs.removeObserver(this, TOPIC_QUIT_APPLICATION_REQUESTED);

remote/marionette/navigate.sys.mjs

@@ -255,20 +255,26 @@ navigate.waitForNavigationCompleted = async function waitForNavigationCompleted(
     }
   };
 
-  const onPromptOpened = action => {
+  const onPromptOpened = (_, data) => {
+    if (data.prompt.promptType === "beforeunload") {
+      // Ignore beforeunload prompts which are handled by the driver class.
+      return;
+    }
+
     lazy.logger.trace("Canceled page load listener because a dialog opened");
     checkDone({ finished: true });
   };
 
   const onTimer = timer => {
-    // In the case when a document has a beforeunload handler
-    // registered, the currently active command will return immediately
-    // due to the modal dialog observer.
+    // For the command "Element Click" we want to detect a potential navigation
+    // as early as possible. The `beforeunload` event is an indication for that
+    // but could still cause the navigation to get aborted by the user. As such
+    // wait a bit longer for the `unload` event to happen, which usually will
+    // occur pretty soon after `beforeunload`.
     //
-    // Otherwise the timeout waiting for the document to start
-    // navigating is increased by 5000 ms to ensure a possible load
-    // event is not missed. In the common case such an event should
-    // occur pretty soon after beforeunload, and we optimise for this.
+    // Note that with WebDriver BiDi enabled the `beforeunload` prompts might
+    // not get implicitly accepted, so lets keep the timer around until we know
+    // that it is really not required.
     if (seenBeforeUnload) {
       seenBeforeUnload = false;
       unloadTimer.initWithCallback(

testing/marionette/client/marionette_driver/geckoinstance.py

@@ -60,8 +60,6 @@ class GeckoInstance(object):
         # Do not show datareporting policy notifications which can interfere with tests
         "datareporting.policy.dataSubmissionEnabled": False,
         "datareporting.policy.dataSubmissionPolicyBypassNotification": True,
-        # Automatically unload beforeunload alerts
-        "dom.disable_beforeunload": True,
         # Enabling the support for File object creation in the content process.
         "dom.file.createInChild": True,
         # Disable delayed user input event handling

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

@@ -505,29 +505,6 @@ def test_click_link_page_load(self):
         self.assertNotEqual(self.marionette.get_url(), self.test_page)
         self.assertEqual(self.marionette.title, "Marionette Test")
 
-    def test_click_link_page_load_dismissed_beforeunload_prompt(self):
-        self.marionette.navigate(
-            inline(
-                """
-          <input type="text"></input>
-          <a href="{}">Click</a>
-          <script>
-            window.addEventListener("beforeunload", function (event) {{
-              event.preventDefault();
-            }});
-          </script>
-        """.format(
-                    self.marionette.absolute_url("clicks.html")
-                )
-            )
-        )
-        self.marionette.find_element(By.TAG_NAME, "input").send_keys("foo")
-        self.marionette.find_element(By.TAG_NAME, "a").click()
-
-        # navigation auto-dismisses beforeunload prompt
-        with self.assertRaises(errors.NoAlertPresentException):
-            Alert(self.marionette).text
-
     def test_click_link_anchor(self):
         self.marionette.find_element(By.ID, "anchor").click()
         self.assertEqual(self.marionette.get_url(), "{}#".format(self.test_page))

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

@@ -385,12 +385,7 @@ def run_bfcache_test(self, test_pages):
 
         def check_page_status(page, expected_history_length):
             if "alert_text" in page:
-                if page["alert_text"] is None:
-                    # navigation auto-dismisses beforeunload prompt
-                    with self.assertRaises(errors.NoAlertPresentException):
-                        Alert(self.marionette).text
-                else:
-                    self.assertEqual(Alert(self.marionette).text, page["alert_text"])
+                self.assertEqual(Alert(self.marionette).text, page["alert_text"])
 
             self.assertEqual(self.marionette.get_url(), page["url"])
             self.assertEqual(self.history_length, expected_history_length)
@@ -442,29 +437,6 @@ def test_no_history_items(self):
         self.marionette.go_back()
         self.marionette.go_forward()
 
-    def test_dismissed_beforeunload_prompt(self):
-        url_beforeunload = inline(
-            """
-          <input type="text">
-          <script>
-            window.addEventListener("beforeunload", function (event) {
-              event.preventDefault();
-            });
-          </script>
-        """
-        )
-
-        def modify_page():
-            self.marionette.find_element(By.TAG_NAME, "input").send_keys("foo")
-
-        test_pages = [
-            {"url": inline("<p>foobar</p>"), "alert_text": None},
-            {"url": url_beforeunload, "callback": modify_page},
-            {"url": inline("<p>foobar</p>"), "alert_text": None},
-        ]
-
-        self.run_bfcache_test(test_pages)
-
     def test_data_urls(self):
         test_pages = [
             {"url": inline("<p>foobar</p>")},
@@ -686,26 +658,6 @@ def test_file_url(self):
         self.marionette.refresh()
         self.assertEqual(self.test_page_file_url, self.marionette.get_url())
 
-    def test_dismissed_beforeunload_prompt(self):
-        self.marionette.navigate(
-            inline(
-                """
-          <input type="text">
-          <script>
-            window.addEventListener("beforeunload", function (event) {
-              event.preventDefault();
-            });
-          </script>
-        """
-            )
-        )
-        self.marionette.find_element(By.TAG_NAME, "input").send_keys("foo")
-        self.marionette.refresh()
-
-        # navigation auto-dismisses beforeunload prompt
-        with self.assertRaises(errors.NoAlertPresentException):
-            Alert(self.marionette).text
-
     def test_image(self):
         image = self.marionette.absolute_url("black.png")
 

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

@@ -59,26 +59,6 @@ def test_close_window_for_browser_tab(self):
         self.assertNotIn(new_tab, window_handles)
         self.assertListEqual(self.start_tabs, window_handles)
 
-    def test_close_window_with_dismissed_beforeunload_prompt(self):
-        new_tab = self.open_tab()
-        self.marionette.switch_to_window(new_tab)
-
-        self.marionette.navigate(
-            inline(
-                """
-          <input type="text">
-          <script>
-            window.addEventListener("beforeunload", function (event) {
-              event.preventDefault();
-            });
-          </script>
-        """
-            )
-        )
-
-        self.marionette.find_element(By.TAG_NAME, "input").send_keys("foo")
-        self.marionette.close()
-
     def test_close_window_for_browser_window_with_single_tab(self):
         new_tab = self.open_window()
         self.marionette.switch_to_window(new_tab)

testing/profiles/web-platform/user.js

@@ -47,8 +47,6 @@ user_pref("browser.safebrowsing.downloads.enabled", false);
 user_pref("browser.safebrowsing.malware.enabled", false);
 user_pref("browser.safebrowsing.phishing.enabled", false);
 user_pref("browser.safebrowsing.update.enabled", false);
-// Automatically unload beforeunload alerts
-user_pref("dom.disable_beforeunload", true);
 // Disable high DPI
 user_pref("layout.css.devPixelsPerPx", "1.0");
 // Enable the parallel styling code.

testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/firefox_android.py

@@ -161,6 +161,10 @@ def _set_required_prefs(self, profile):
             "dom.send_after_paint_to_content": True,
         })
 
+        if self.package_name == "org.mozilla.geckoview.test_runner":
+            # Bug 1879324: The TestRunner doesn't support "beforeunload" prompts yet
+            profile.set_preferences({"dom.disable_beforeunload": True})
+
         if self.test_type == "reftest":
             self.logger.info("Setting android reftest preferences")
             profile.set_preferences({