Bug 1337794 - remove obsolete pagehide handling hacks from browser.js draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 25 Jan 2017 11:16:26 +0000
changeset 789962 d4035c46db9dc15e29009df2d4d2f21bcee7246b
parent 789560 c552490c8659bf2e7d279ed28d46fb5d5a245a96
push id108374
push userbmo:gijskruitbosch+bugs@gmail.com
push dateMon, 30 Apr 2018 20:47:15 +0000
bugs1337794
milestone61.0a1
Bug 1337794 - remove obsolete pagehide handling hacks from browser.js MozReview-Commit-ID: 1BeeU0zJnXF
browser/base/content/browser.js
browser/base/content/test/popups/browser_popup_frames.js
layout/base/tests/test_reftests_with_caret.html
toolkit/components/aboutmemory/tests/test_aboutmemory4.xul
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -5309,42 +5309,16 @@ var TabsProgressListener = {
           TelemetryStopwatch.finish("FX_PAGE_LOAD_MS", aBrowser);
         }
       } else if (aStateFlags & Ci.nsIWebProgressListener.STATE_STOP &&
                  aStatus == Cr.NS_BINDING_ABORTED &&
                  stopwatchRunning /* we won't see STATE_START events for pre-rendered tabs */) {
         TelemetryStopwatch.cancel("FX_PAGE_LOAD_MS", aBrowser);
       }
     }
-
-    // We used to listen for clicks in the browser here, but when that
-    // became unnecessary, removing the code below caused focus issues.
-    // This code should be removed. Tracked in bug 1337794.
-    let isRemoteBrowser = aBrowser.isRemoteBrowser;
-    // We check isRemoteBrowser here to avoid requesting the doc CPOW
-    let doc = isRemoteBrowser ? null : aWebProgress.DOMWindow.document;
-
-    if (!isRemoteBrowser &&
-        aStateFlags & Ci.nsIWebProgressListener.STATE_STOP &&
-        Components.isSuccessCode(aStatus) &&
-        doc.documentURI.startsWith("about:") &&
-        !doc.documentURI.toLowerCase().startsWith("about:blank") &&
-        !doc.documentURI.toLowerCase().startsWith("about:home") &&
-        !doc.documentElement.hasAttribute("hasBrowserHandlers")) {
-      // STATE_STOP may be received twice for documents, thus store an
-      // attribute to ensure handling it just once.
-      doc.documentElement.setAttribute("hasBrowserHandlers", "true");
-      aBrowser.addEventListener("pagehide", function onPageHide(event) {
-        if (event.target.defaultView.frameElement)
-          return;
-        aBrowser.removeEventListener("pagehide", onPageHide, true);
-        if (event.target.documentElement)
-          event.target.documentElement.removeAttribute("hasBrowserHandlers");
-      }, true);
-    }
   },
 
   onLocationChange(aBrowser, aWebProgress, aRequest, aLocationURI, aFlags) {
     // Filter out location changes caused by anchor navigation
     // or history.push/pop/replaceState.
     if (aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT) {
       // Reader mode cares about history.pushState and friends.
       // FIXME: The content process should manage this directly (bug 1445351).
--- a/browser/base/content/test/popups/browser_popup_frames.js
+++ b/browser/base/content/test/popups/browser_popup_frames.js
@@ -22,19 +22,19 @@ add_task(async function test_opening_blo
   let notification;
   await BrowserTestUtils.waitForCondition(() =>
     notification = gBrowser.getNotificationBox().getNotificationWithValue("popup-blocked"));
 
   ok(notification, "Should have notification.");
 
   await ContentTask.spawn(tab.linkedBrowser, baseURL, async function(uri) {
     let iframe = content.document.createElement("iframe");
-    iframe.src = uri;
     let pageHideHappened = ContentTaskUtils.waitForEvent(this, "pagehide", true);
     content.document.body.appendChild(iframe);
+    iframe.src = uri;
     await pageHideHappened;
   });
 
   notification = gBrowser.getNotificationBox().getNotificationWithValue("popup-blocked");
   ok(notification, "Should still have notification");
 
   // Now navigate the subframe.
   await ContentTask.spawn(tab.linkedBrowser, null, async function() {
--- a/layout/base/tests/test_reftests_with_caret.html
+++ b/layout/base/tests/test_reftests_with_caret.html
@@ -61,16 +61,20 @@ const MAX_ITERATIONS = 1000;
 
 function createIframe(url,next) {
   var iframe = document.createElement("iframe");
   iframe.src = url;
   iframe.remotePageLoaded = remotePageLoaded;
   var me = this;
   var currentIteration = 0;
   function iframeLoadCompleted() {
+    let loc = iframe.contentWindow.location;
+    if (loc && loc.href == "about:blank") {
+      return;
+    }
     var docEl = iframe.contentDocument.documentElement;
     if (docEl.className.includes("reftest-wait")) {
       if (currentIteration++ > MAX_ITERATIONS) {
         ok(false, "iframe load for " + url + " timed out");
         endTest();
       } else {
         setTimeout(iframeLoadCompleted, 0);
       }
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory4.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory4.xul
@@ -32,52 +32,55 @@
 
   // Load the given file into the frame, then copy+paste the entire frame and
   // check that the cut text matches what we expect.
   function test(aFilename, aExpected, aNext) {
     let frame = document.createElementNS("http://www.w3.org/1999/xhtml", "iframe")
     frame.height = 300;
     frame.src = "about:memory?file=" + makePathname(aFilename);
     document.documentElement.appendChild(frame);
-    frame.focus();
+    frame.addEventListener("load", function onFrameLoad(e) {
+      frame.removeEventListener("load", onFrameLoad);
+      frame.focus();
 
-    // Initialize the clipboard contents.
-    SpecialPowers.clipboardCopyString("initial clipboard value");
+      // Initialize the clipboard contents.
+      SpecialPowers.clipboardCopyString("initial clipboard value");
 
-    let numFailures = 0, maxFailures = 30;
+      let numFailures = 0, maxFailures = 30;
 
-    // Because the file load is async, we don't know when it will finish and
-    // the output will show up.  So we poll.
-    function copyPasteAndCheck() {
-      // Copy and paste frame contents, and filter out non-deterministic
-      // differences.
-      synthesizeKey("A", {accelKey: true});
-      synthesizeKey("C", {accelKey: true});
-      let actual = SpecialPowers.getClipboardData("text/unicode");
-      actual = actual.replace(/\(pid \d+\)/, "(pid NNN)");
+      // Because the file load is async, we don't know when it will finish and
+      // the output will show up.  So we poll.
+      function copyPasteAndCheck() {
+        // Copy and paste frame contents, and filter out non-deterministic
+        // differences.
+        synthesizeKey("A", {accelKey: true});
+        synthesizeKey("C", {accelKey: true});
+        let actual = SpecialPowers.getClipboardData("text/unicode");
+        actual = actual.replace(/\(pid \d+\)/, "(pid NNN)");
 
-      if (actual.trim() === aExpected.trim()) {
-        SimpleTest.ok(true, "Clipboard has the expected contents");
-        aNext();
-      } else {
-        numFailures++;
-        if (numFailures === maxFailures) {
-          ok(false, "pasted text doesn't match");
-          dump("******EXPECTED******\n");
-          dump(aExpected);
-          dump("*******ACTUAL*******\n");
-          dump(actual);
-          dump("********************\n");
-          SimpleTest.finish();
+        if (actual.trim() === aExpected.trim()) {
+          SimpleTest.ok(true, "Clipboard has the expected contents");
+          aNext();
         } else {
-          setTimeout(copyPasteAndCheck, 100);
+          numFailures++;
+          if (numFailures === maxFailures) {
+            ok(false, "pasted text doesn't match");
+            dump("******EXPECTED******\n");
+            dump(aExpected);
+            dump("*******ACTUAL*******\n");
+            dump(actual);
+            dump("********************\n");
+            SimpleTest.finish();
+          } else {
+            setTimeout(copyPasteAndCheck, 100);
+          }
         }
       }
-    }
-    copyPasteAndCheck();
+      copyPasteAndCheck();
+    });
   }
 
   // Returns a function that chains together multiple test() calls.
   function chain(aFrameIds) {
     let x = aFrameIds.shift();
     if (x) {
       return function() { test(x.filename, x.expected, chain(aFrameIds)); }
     } else {