Bug 1411393 - Marionette should not register listeners on the global window or document. draft
authorHenrik Skupin <mail@hskupin.info>
Wed, 25 Oct 2017 14:15:30 +0200
changeset 693542 e25c68aa8d12cc406a6daa928db204b9aa578a87
parent 693519 dc45ee24c55d1061951956321bd8481d517ce22a
child 739069 4cb94879f350ed449377f621d6a7168b29916fbe
push id87844
push userbmo:hskupin@gmail.com
push dateMon, 06 Nov 2017 13:14:36 +0000
bugs1411393
milestone58.0a1
Bug 1411393 - Marionette should not register listeners on the global window or document. Registering listeners for the "beforeunload" and "unload" events currently happens on the global window and document. This actually prevents Firefox from adding those pages to the bfcache. The correct is to add all the listeners to the tabchildglobal, which is the framescript itself. Also by using the capture and not the bubble phase the unload events are correctly propagaded to our registered listeners. MozReview-Commit-ID: 4hJjuqWsoBP
testing/marionette/listener.js
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -128,24 +128,21 @@ const loadListener = {
     timeout = startTime + timeout - new Date().getTime();
 
     if (timeout <= 0) {
       this.notify(this.timerPageLoad);
       return;
     }
 
     if (waitForUnloaded) {
-      addEventListener("hashchange", this, false);
-      addEventListener("pagehide", this, false);
-      addEventListener("popstate", this, false);
-
-      // The events can only be received when the event listeners are
-      // added to the currently selected frame.
-      curContainer.frame.addEventListener("beforeunload", this);
-      curContainer.frame.addEventListener("unload", this);
+      addEventListener("beforeunload", this, true);
+      addEventListener("hashchange", this, true);
+      addEventListener("pagehide", this, true);
+      addEventListener("popstate", this, true);
+      addEventListener("unload", this, true);
 
       Services.obs.addObserver(this, "outer-window-destroyed");
 
     } else {
       // The frame script got reloaded due to a new content process.
       // Due to the time it takes to re-register the browser in Marionette,
       // it can happen that page load events are missed before the listeners
       // are getting attached again. By checking the document readyState the
@@ -155,18 +152,18 @@ const loadListener = {
       logger.debug(`Check readyState "${readyState} for "${documentURI}"`);
 
       // If the page load has already finished, don't setup listeners and
       // timers but return immediatelly.
       if (this.handleReadyState(readyState, documentURI)) {
         return;
       }
 
-      addEventListener("DOMContentLoaded", loadListener);
-      addEventListener("pageshow", loadListener);
+      addEventListener("DOMContentLoaded", loadListener, true);
+      addEventListener("pageshow", loadListener, true);
     }
 
     this.timerPageLoad.initWithCallback(
         this, timeout, Ci.nsITimer.TYPE_ONE_SHOT);
   },
 
   /**
    * Stop listening for page unload/load events.
@@ -175,32 +172,23 @@ const loadListener = {
     if (this.timerPageLoad) {
       this.timerPageLoad.cancel();
     }
 
     if (this.timerPageUnload) {
       this.timerPageUnload.cancel();
     }
 
-    removeEventListener("hashchange", this);
-    removeEventListener("pagehide", this);
-    removeEventListener("popstate", this);
-    removeEventListener("DOMContentLoaded", this);
-    removeEventListener("pageshow", this);
-
-    // If the original content window, where the navigation was triggered,
-    // doesn't exist anymore, exceptions can be silently ignored.
-    try {
-      curContainer.frame.removeEventListener("beforeunload", this);
-      curContainer.frame.removeEventListener("unload", this);
-    } catch (e) {
-      if (e.name != "TypeError") {
-        throw e;
-      }
-    }
+    removeEventListener("beforeunload", this, true);
+    removeEventListener("hashchange", this, true);
+    removeEventListener("pagehide", this, true);
+    removeEventListener("popstate", this, true);
+    removeEventListener("DOMContentLoaded", this, true);
+    removeEventListener("pageshow", this, true);
+    removeEventListener("unload", this, true);
 
     // In case the observer was added before the frame script has been
     // reloaded, it will no longer be available. Exceptions can be ignored.
     try {
       Services.obs.removeObserver(this, "outer-window-destroyed");
     } catch (e) {}
   },
 
@@ -225,23 +213,23 @@ const loadListener = {
 
       case "unload":
         this.seenUnload = true;
         break;
 
       case "pagehide":
         this.seenUnload = true;
 
-        removeEventListener("hashchange", this);
-        removeEventListener("pagehide", this);
-        removeEventListener("popstate", this);
+        removeEventListener("hashchange", this, true);
+        removeEventListener("pagehide", this, true);
+        removeEventListener("popstate", this, true);
 
         // Now wait until the target page has been loaded
-        addEventListener("DOMContentLoaded", this, false);
-        addEventListener("pageshow", this, false);
+        addEventListener("DOMContentLoaded", this, true);
+        addEventListener("pageshow", this, true);
         break;
 
       case "hashchange":
       case "popstate":
         this.stop();
         sendOk(this.commandID);
         break;