Bug 1070053 - Avoid spurious hidden-plugin notifications when changing locations by doing a Principal comparison. r=gfritzsche

When the PluginRemoved event is fired when changing locations, it's fired
asynchronously such that the document that the plugin belongs to has already
been unloaded. This was causing the hidden plugin notification to appear in
some cases when users browsed away from documents that had hidden plugins
in them. Now we pass the Principal for the unloading document back to the
parent and do a comparison with the current browser Principal to ensure
that they match before showing the hidden plugin notification.

--HG--
rename : browser/base/content/test/plugins/plugin_small.html => browser/base/content/test/plugins/plugin_small_2.html
extra : rebase_source : e748e3b09de77cc7796b1a78f8e39a23af64049a

browser/base/content/browser-plugins.js

@@ -306,6 +306,15 @@ var gPluginHandler = {
   },
 
   updateHiddenPluginUI: function (browser, haveInsecure, actions, principal, host) {
+    // It is possible that we've received a message from the frame script to show
+    // the hidden plugin notification for a principal that no longer matches the one
+    // that the browser's content now has assigned (ie, the browser has browsed away
+    // after the message was sent, but before the message was received). In that case,
+    // we should just ignore the message.
+    if (!principal.equals(browser.contentPrincipal)) {
+      return;
+    }
+
     // Set up the icon
     document.getElementById("plugins-notification-icon").classList.
       toggle("plugin-blocked", haveInsecure);

browser/base/content/test/plugins/browser.ini

@@ -33,6 +33,7 @@ support-files =
   plugin_overlayed.html
   plugin_positioned.html
   plugin_small.html
+  plugin_small_2.html
   plugin_syncRemoved.html
   plugin_test.html
   plugin_test2.html
@@ -62,6 +63,7 @@ run-if = crashreporter
 [browser_CTP_nonplugins.js]
 [browser_CTP_notificationBar.js]
 [browser_CTP_outsideScrollArea.js]
+[browser_CTP_remove_navigate.js]
 [browser_CTP_resize.js]
 [browser_CTP_zoom.js]
 [browser_globalplugin_crashinfobar.js]

browser/base/content/test/plugins/browser_CTP_remove_navigate.js

@@ -0,0 +1,82 @@
+/* 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/. */
+
+const gTestRoot = getRootDirectory(gTestPath);
+const gHttpTestRoot = gTestRoot.replace("chrome://mochitests/content/",
+                                        "http://127.0.0.1:8888/");
+
+add_task(function* () {
+  Services.prefs.setBoolPref("plugins.click_to_play", true);
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref("plugins.click_to_play");
+  });
+})
+
+/**
+ * Tests that if a plugin is removed just as we transition to
+ * a different page, that we don't show the hidden plugin
+ * notification bar on the new page.
+ */
+add_task(function* () {
+  let newTab = gBrowser.addTab();
+  gBrowser.selectedTab = newTab;
+  let browser = gBrowser.selectedBrowser;
+
+  setTestPluginEnabledState(Ci.nsIPluginTag.STATE_CLICKTOPLAY);
+
+  // Load up a page with a plugin...
+  let notificationPromise =
+    waitForNotificationBar("plugin-hidden", gBrowser.selectedBrowser);
+  yield loadPage(browser, gHttpTestRoot + "plugin_small.html")
+  yield forcePluginBindingAttached(browser);
+  yield notificationPromise;
+
+  // Trigger the PluginRemoved event to be fired, and then immediately
+  // browse to a new page.
+  let plugin = browser.contentDocument.getElementById("test");
+  plugin.remove();
+  yield loadPage(browser, "about:mozilla");
+
+  // There should be no hidden plugin notification bar at about:mozilla.
+  let notificationBox = gBrowser.getNotificationBox(browser);
+  is(notificationBox.getNotificationWithValue("plugin-hidden"), null,
+     "Expected no notification box");
+  gBrowser.removeTab(newTab);
+});
+
+/**
+ * Tests that if a plugin is removed just as we transition to
+ * a different page with a plugin, that we show the right notification
+ * for the new page.
+ */
+add_task(function* () {
+  let newTab = gBrowser.addTab();
+  gBrowser.selectedTab = newTab;
+  let browser = gBrowser.selectedBrowser;
+
+  setTestPluginEnabledState(Ci.nsIPluginTag.STATE_CLICKTOPLAY,
+                            "Second Test Plug-in");
+
+  // Load up a page with a plugin...
+  let notificationPromise =
+    waitForNotificationBar("plugin-hidden", browser);
+  yield loadPage(browser, gHttpTestRoot + "plugin_small.html")
+  yield forcePluginBindingAttached(browser);
+  yield notificationPromise;
+
+  // Trigger the PluginRemoved event to be fired, and then immediately
+  // browse to a new page.
+  let plugin = browser.contentDocument.getElementById("test");
+  plugin.remove();
+  yield loadPage(browser, gTestRoot + "plugin_small_2.html");
+  let notification = yield waitForNotificationBar("plugin-hidden", browser);
+  ok(notification, "There should be a notification shown for the new page.");
+
+  // Ensure that the notification is showing information about
+  // the x-second-test plugin.
+  ok(notification.label.contains("Second Test"), "Should mention the second plugin");
+  ok(!notification.label.contains("127.0.0.1"), "Should not refer to old principal");
+  ok(notification.label.contains("null"), "Should refer to the new principal");
+  gBrowser.removeTab(newTab);
+});

browser/base/content/test/plugins/head.js

@@ -121,15 +121,78 @@ function waitForNotificationPopup(notificationID, browser, callback) {
   );
 }
 
+/**
+ * Returns a Promise that resolves when a notification bar
+ * for a browser is shown. Alternatively, for old-style callers,
+ * can automatically call a callback before it resolves.
+ *
+ * @param notificationID
+ *        The ID of the notification to look for.
+ * @param browser
+ *        The browser to check for the notification bar.
+ * @param callback (optional)
+ *        A function to be called just before the Promise resolves.
+ *
+ * @return Promise
+ */
 function waitForNotificationBar(notificationID, browser, callback) {
+  return new Promise((resolve, reject) => {
     let notification;
     let notificationBox = gBrowser.getNotificationBox(browser);
     waitForCondition(
       () => (notification = notificationBox.getNotificationWithValue(notificationID)),
       () => {
         ok(notification, `Successfully got the ${notificationID} notification bar`);
+        if (callback) {
           callback(notification);
+        }
+        resolve(notification);
       },
       `Waited too long for the ${notificationID} notification bar`
     );
+  });
+}
+
+/**
+ * Due to layout being async, "PluginBindAttached" may trigger later.
+ * This returns a Promise that resolves once we've forced a layout
+ * flush, which triggers the PluginBindAttached event to fire.
+ *
+ * @param browser
+ *        The browser to force plugin bindings in.
+ *
+ * @return Promise
+ */
+function forcePluginBindingAttached(browser) {
+  return new Promise((resolve, reject) => {
+    let doc = browser.contentDocument;
+    let elems = doc.getElementsByTagName('embed');
+    if (elems.length < 1) {
+      elems = doc.getElementsByTagName('object');
+    }
+    elems[0].clientTop;
+    executeSoon(resolve);
+  });
+}
+
+/**
+ * Loads a page in a browser, and returns a Promise that
+ * resolves once the "load" event has been fired for that
+ * browser.
+ *
+ * @param browser
+ *        The browser to load the page in.
+ * @param uri
+ *        The URI to load.
+ *
+ * @return Promise
+ */
+function loadPage(browser, uri) {
+  return new Promise((resolve, reject) => {
+    browser.addEventListener("load", function onLoad(event) {
+      browser.removeEventListener("load", onLoad, true);
+      resolve();
+    }, true);
+    browser.loadURI(uri);
+  });
 }

browser/base/content/test/plugins/plugin_small_2.html

@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+</head>
+<body>
+<embed id="test" style="width: 10px; height: 10px" type="application/x-second-test">
+</body>
+</html>

browser/modules/PluginContent.jsm

@@ -285,7 +285,7 @@ PluginContent.prototype = {
     }
 
     if (eventType == "PluginRemoved") {
-      this.updateNotificationUI();
+      this.updateNotificationUI(event.target);
       return;
     }
 
@@ -698,7 +698,29 @@ PluginContent.prototype = {
     }, null, principal);
   },
 
-  updateNotificationUI: function () {
+  /**
+   * Updates the "hidden plugin" notification bar UI.
+   *
+   * @param document (optional)
+   *        Specify the document that is causing the update.
+   *        This is useful when the document is possibly no longer
+   *        the current loaded document (for example, if we're
+   *        responding to a PluginRemoved event for an unloading
+   *        document). If this parameter is omitted, it defaults
+   *        to the current top-level document.
+   */
+  updateNotificationUI: function (document) {
+    let principal;
+
+    if (document) {
+      // We're only interested in the top-level document, since that's
+      // the one that provides the Principal that we send back to the
+      // parent.
+      principal = document.defaultView.top.document.nodePrincipal;
+    } else {
+      principal = this.content.document.nodePrincipal;
+    }
+
     // Make a copy of the actions from the last popup notification.
     let haveInsecure = false;
     let actions = new Map();
@@ -718,8 +740,7 @@ PluginContent.prototype = {
     }
 
     // Remove plugins that are already active, or large enough to show an overlay.
-    let contentWindow = this.global.content;
-    let cwu = contentWindow.QueryInterface(Ci.nsIInterfaceRequestor)
+    let cwu = this.content.QueryInterface(Ci.nsIInterfaceRequestor)
                           .getInterface(Ci.nsIDOMWindowUtils);
     for (let plugin of cwu.plugins) {
       let info = this._getPluginInfo(plugin);
@@ -755,7 +776,6 @@ PluginContent.prototype = {
 
     // If there are any items remaining in `actions` now, they are hidden
     // plugins that need a notification bar.
-    let principal = contentWindow.document.nodePrincipal;
     this.global.sendAsyncMessage("PluginContent:UpdateHiddenPluginUI", {
       haveInsecure: haveInsecure,
       actions: [... actions.values()],