Bug 1246291 - Only send RefreshBlocked message to the parent once onLocationChange and onRefreshAttempted have both fired. r?Mossop draft
authorMike Conley <mconley@mozilla.com>
Wed, 17 Feb 2016 15:35:33 -0500
changeset 331872 acc64ced47f8a18f3f545ea8284c9cefb7c194fb
parent 331871 0c6861b50d13f449a95f03b77ff4f889cacfc6c7
child 331873 967e2a34c8a4232b38bb26e4e3223b212214ef76
child 331929 37a844148b29426f43a54eb17db9724fa02baa41
child 331930 e5f8fc0c8364a0696bc3ae415df9c0eb9ac6cd9b
child 331931 1835e951d9a4490aba51ad0b11b376208cf87e2b
child 332049 b3cea37851ecacce462901a2ea3f6f90b3cb43ba
child 332050 0cc14cdbc441715354e6ed6ebc3b4b6f2f6e925a
push id11106
push usermconley@mozilla.com
push dateThu, 18 Feb 2016 15:13:45 +0000
reviewersMossop
bugs1246291
milestone47.0a1
Bug 1246291 - Only send RefreshBlocked message to the parent once onLocationChange and onRefreshAttempted have both fired. r?Mossop MozReview-Commit-ID: BxJA0L0j78E
browser/base/content/tab-content.js
--- a/browser/base/content/tab-content.js
+++ b/browser/base/content/tab-content.js
@@ -686,16 +686,48 @@ var DOMFullscreenHandler = {
     }
   }
 };
 DOMFullscreenHandler.init();
 
 var RefreshBlocker = {
   PREF: "accessibility.blockautorefresh",
 
+  // Bug 1247100 - When a refresh is caused by an HTTP header,
+  // onRefreshAttempted will be fired before onLocationChange.
+  // When a refresh is caused by a <meta> tag in the document,
+  // onRefreshAttempted will be fired after onLocationChange.
+  //
+  // We only ever want to send a message to the parent after
+  // onLocationChange has fired, since the parent uses the
+  // onLocationChange update to clear transient notifications.
+  // Sending the message before onLocationChange will result in
+  // us creating the notification, and then clearing it very
+  // soon after.
+  //
+  // To account for both cases (onRefreshAttempted before
+  // onLocationChange, and onRefreshAttempted after onLocationChange),
+  // we'll hold a mapping of DOM Windows that we see get
+  // sent through both onLocationChange and onRefreshAttempted.
+  // When either run, they'll check the WeakMap for the existence
+  // of the DOM Window. If it doesn't exist, it'll add it. If
+  // it finds it, it'll know that it's safe to send the message
+  // to the parent, since we know that both have fired.
+  //
+  // The DOM Window is removed from blockedWindows when we notice
+  // the nsIWebProgress change state to STATE_STOP for the
+  // STATE_IS_WINDOW case.
+  //
+  // DOM Windows are mapped to a JS object that contains the data
+  // to be sent to the parent to show the notification. Since that
+  // data is only known when onRefreshAttempted is fired, it's only
+  // ever stashed in the map if onRefreshAttempted fires first -
+  // otherwise, null is set as the value of the mapping.
+  blockedWindows: new WeakMap(),
+
   init() {
     if (Services.prefs.getBoolPref(this.PREF)) {
       this.enable();
     }
 
     Services.prefs.addObserver(this.PREF, this, false);
   },
 
@@ -715,49 +747,99 @@ var RefreshBlocker = {
         this.disable();
       }
     }
   },
 
   enable() {
     this._filter = Cc["@mozilla.org/appshell/component/browser-status-filter;1"]
                      .createInstance(Ci.nsIWebProgress);
-    this._filter.addProgressListener(this, Ci.nsIWebProgress.NOTIFY_REFRESH);
+    this._filter.addProgressListener(this, Ci.nsIWebProgress.NOTIFY_ALL);
 
     let webProgress = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
                               .getInterface(Ci.nsIWebProgress);
-    webProgress.addProgressListener(this._filter, Ci.nsIWebProgress.NOTIFY_REFRESH);
+    webProgress.addProgressListener(this._filter, Ci.nsIWebProgress.NOTIFY_ALL);
 
     addMessageListener("RefreshBlocker:Refresh", this);
   },
 
   disable() {
     let webProgress = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
                               .getInterface(Ci.nsIWebProgress);
     webProgress.removeProgressListener(this._filter);
 
     this._filter.removeProgressListener(this);
     this._filter = null;
 
     removeMessageListener("RefreshBlocker:Refresh", this);
   },
 
+  send(data) {
+    sendAsyncMessage("RefreshBlocker:Blocked", data);
+  },
+
+  /**
+   * Notices when the nsIWebProgress transitions to STATE_STOP for
+   * the STATE_IS_WINDOW case, which will clear any mappings from
+   * blockedWindows.
+   */
+  onStateChange(aWebProgress, aRequest, aStateFlags, aStatus) {
+    if (aStateFlags & Ci.nsIWebProgressListener.STATE_IS_WINDOW &&
+        aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) {
+      this.blockedWindows.delete(aWebProgress.DOMWindow);
+    }
+  },
+
+  /**
+   * Notices when the location has changed. If, when running,
+   * onRefreshAttempted has already fired for this DOM Window, will
+   * send the appropriate refresh blocked data to the parent.
+   */
+  onLocationChange(aWebProgress, aRequest, aLocation, aFlags) {
+    let win = aWebProgress.DOMWindow;
+    if (this.blockedWindows.has(win)) {
+      let data = this.blockedWindows.get(win);
+      if (data) {
+        // We saw onRefreshAttempted before onLocationChange, so
+        // send the message to the parent to show the notification.
+        this.send(data);
+      }
+    } else {
+      this.blockedWindows.set(win, null);
+    }
+  },
+
+  /**
+   * Notices when a refresh / reload was attempted. If, when running,
+   * onLocationChange has not yet run, will stash the appropriate data
+   * into the blockedWindows map to be sent when onLocationChange fires.
+   */
   onRefreshAttempted(aWebProgress, aURI, aDelay, aSameURI) {
     let win = aWebProgress.DOMWindow;
     let outerWindowID = win.QueryInterface(Ci.nsIInterfaceRequestor)
                            .getInterface(Ci.nsIDOMWindowUtils)
                            .outerWindowID;
 
-    sendAsyncMessage("RefreshBlocker:Blocked", {
+    let data = {
       URI: aURI.spec,
       originCharset: aURI.originCharset,
       delay: aDelay,
       sameURI: aSameURI,
       outerWindowID,
-    });
+    };
+
+    if (this.blockedWindows.has(win)) {
+      // onLocationChange must have fired before, so we can tell the
+      // parent to show the notification.
+      this.send(data);
+    } else {
+      // onLocationChange hasn't fired yet, so stash the data in the
+      // map so that onLocationChange can send it when it fires.
+      this.blockedWindows.set(win, data);
+    }
 
     return false;
   },
 
   receiveMessage(message) {
     let data = message.data;
 
     if (message.name == "RefreshBlocker:Refresh") {