Backed out 2 changesets (bug 1190688) for browser_ext_tabs_executeScript.js permatimeouts
authorWes Kocher <wkocher@mozilla.com>
Wed, 02 Dec 2015 11:22:31 -0800
changeset 275210 689eebc89b6d3b2a1bc0fdaa68eba8dc718d805b
parent 275209 92c54aa86e1c8c07f649d1b3c45c40ad0d3db592
child 275211 de8bc7c996fe0dea3af17163d25cc77617759f57
push id16517
push userkwierso@gmail.com
push dateWed, 02 Dec 2015 19:22:38 +0000
treeherderfx-team@689eebc89b6d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1190688
milestone45.0a1
backs out1d5e9f3d094d075cfda767dd2108b07c7f8557de
4a10c564dfca3189ab521c3ccfa92330163189b1
Backed out 2 changesets (bug 1190688) for browser_ext_tabs_executeScript.js permatimeouts Backed out changeset 1d5e9f3d094d (bug 1190688) Backed out changeset 4a10c564dfca (bug 1190688)
browser/components/extensions/ext-browserAction.js
browser/components/extensions/ext-contextMenus.js
browser/components/extensions/ext-pageAction.js
browser/components/extensions/ext-tabs.js
browser/components/extensions/ext-utils.js
browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
browser/components/extensions/test/browser/head.js
--- a/browser/components/extensions/ext-browserAction.js
+++ b/browser/components/extensions/ext-browserAction.js
@@ -27,18 +27,16 @@ var nextActionId = 0;
 // Responsible for the browser_action section of the manifest as well
 // as the associated popup.
 function BrowserAction(options, extension)
 {
   this.extension = extension;
   this.id = makeWidgetId(extension.id) + "-browser-action";
   this.widget = null;
 
-  this.tabManager = TabManager.for(extension);
-
   let title = extension.localize(options.default_title || "");
   let popup = extension.localize(options.default_popup || "");
   if (popup) {
     popup = extension.baseURI.resolve(popup);
   }
 
   this.defaults = {
     enabled: true,
@@ -71,17 +69,16 @@ BrowserAction.prototype = {
 
         this.updateButton(node, this.defaults);
 
         let tabbrowser = document.defaultView.gBrowser;
 
         node.addEventListener("command", event => {
           let tab = tabbrowser.selectedTab;
           let popup = this.getProperty(tab, "popup");
-          this.tabManager.addActiveTabPermission(tab);
           if (popup) {
             this.togglePopup(node, popup);
           } else {
             this.emit("click");
           }
         });
 
         return node;
--- a/browser/components/extensions/ext-contextMenus.js
+++ b/browser/components/extensions/ext-contextMenus.js
@@ -38,21 +38,18 @@ var menuBuilder = {
   build: function(contextData) {
     // TODO: icons should be set for items
     let xulMenu = contextData.menu;
     xulMenu.addEventListener("popuphidden", this);
     let doc = xulMenu.ownerDocument;
     for (let [ext, menuItemMap] of contextMenuMap) {
       let parentMap = new Map();
       let topLevelItems = new Set();
-      for (let entry of menuItemMap) {
-        // We need a closure over |item|, and we don't currently get a
-        // fresh binding per loop if we declare it in the loop head.
-        let [id, item] = entry;
-
+      for (let [id, item] of menuItemMap) {
+        dump(id + " : " + item + "\n");
         if (item.enabledForContext(contextData)) {
           let element;
           if (item.isMenu) {
             element = doc.createElement("menu");
             // Menu elements need to have a menupopup child for
             // its menu items.
             let menupopup = doc.createElement("menupopup");
             element.appendChild(menupopup);
@@ -77,23 +74,25 @@ var menuBuilder = {
             // If parentId is set we have to look up its parent
             // and appened to its menupopup.
             let parentElement = parentMap.get(parentId);
             parentElement.appendChild(element);
           } else {
             topLevelItems.add(element);
           }
 
-          element.addEventListener("command", event => {
-            item.tabManager.addActiveTabPermission();
-            if (item.onclick) {
-              let clickData = item.getClickData(contextData, event);
-              runSafe(item.extContext, item.onclick, clickData);
+          if (item.onclick) {
+            function clickHandlerForItem(item) {
+              return event => {
+                let clickData = item.getClickData(contextData, event);
+                runSafe(item.extContext, item.onclick, clickData);
+              }
             }
-          });
+            element.addEventListener("command", clickHandlerForItem(item));
+          }
         }
       }
       if (topLevelItems.size > 1) {
         // If more than one top level items are visible, callopse them.
         let top = doc.createElement("menu");
         top.setAttribute("label", ext.name);
         top.setAttribute("ext-type", "top-level-menu");
         let menupopup = doc.createElement("menupopup");
@@ -162,18 +161,16 @@ function getContexts(contextData) {
   return contexts;
 }
 
 function MenuItem(extension, extContext, createProperties)
 {
   this.extension = extension;
   this.extContext = extContext;
 
-  this.tabManager = TabManager.for(extension);
-
   this.init(createProperties);
 }
 
 MenuItem.prototype = {
   // init is called from the MenuItem ctor and from update. The |update|
   // flag is for the later case.
   init(createProperties, update=false) {
     let item = this;
--- a/browser/components/extensions/ext-pageAction.js
+++ b/browser/components/extensions/ext-pageAction.js
@@ -15,18 +15,16 @@ var pageActionMap = new WeakMap();
 
 // Handles URL bar icons, including the |page_action| manifest entry
 // and associated API.
 function PageAction(options, extension)
 {
   this.extension = extension;
   this.id = makeWidgetId(extension.id) + "-page-action";
 
-  this.tabManager = TabManager.for(extension);
-
   let title = extension.localize(options.default_title || "");
   let popup = extension.localize(options.default_popup || "");
   if (popup) {
     popup = extension.baseURI.resolve(popup);
   }
 
   this.defaults = {
     show: false,
@@ -136,18 +134,16 @@ PageAction.prototype = {
   // window.
   // If the page action has a |popup| property, a panel is opened to
   // that URL. Otherwise, a "click" event is emitted, and dispatched to
   // the any click listeners in the add-on.
   handleClick(window) {
     let tab = window.gBrowser.selectedTab;
     let popup = this.tabContext.get(tab).popup;
 
-    this.tabManager.addActiveTabPermission(tab);
-
     if (popup) {
       openPanel(this.getButton(window), popup, this.extension);
     } else {
       this.emit("click", tab);
     }
   },
 
   handleLocationChange(eventType, tab, fromBrowse) {
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -452,18 +452,23 @@ extensions.registerAPI((extension, conte
           if (pattern && !pattern.matches(Services.io.newURI(tab.url, null, null))) {
             return false;
           }
 
           return true;
         }
 
         let result = [];
-        for (let window of WindowListManager.browserWindows()) {
-          let tabs = TabManager.for(extension).getTabs(window);
+        let e = Services.wm.getEnumerator("navigator:browser");
+        while (e.hasMoreElements()) {
+          let window = e.getNext();
+          if (window.document.readyState != "complete") {
+            continue;
+          }
+          let tabs = TabManager.getTabs(extension, window);
           for (let tab of tabs) {
             if (matches(window, tab)) {
               result.push(tab);
             }
           }
         }
         runSafe(context, callback, result);
       },
@@ -479,26 +484,20 @@ extensions.registerAPI((extension, conte
           // We need to send the inner window ID to make sure we only
           // execute the script if the window is currently navigated to
           // the document that we expect.
           //
           // TODO: When we add support for callbacks, non-matching
           // window IDs and insufficient permissions need to result in a
           // callback with |lastError| set.
           innerWindowID: tab.linkedBrowser.innerWindowID,
+
+          matchesHost: extension.whiteListedHosts.serialize(),
         };
 
-        if (TabManager.for(extension).hasActiveTabPermission(tab)) {
-          // If we have the "activeTab" permission for this tab, ignore
-          // the host whitelist.
-          options.matchesHost = ["<all_urls>"];
-        } else {
-          options.matchesHost = extension.whiteListedHosts.serialize();
-        }
-
         if (details.code) {
           options[kind + 'Code'] = details.code;
         }
         if (details.file) {
           let url = context.uri.resolve(details.file);
           if (extension.isExtensionURL(url)) {
             // We should really set |lastError| here, and go straight to
             // the callback, but we don't have |lastError| yet.
--- a/browser/components/extensions/ext-utils.js
+++ b/browser/components/extensions/ext-utils.js
@@ -262,97 +262,17 @@ TabContext.prototype = {
   },
 
   shutdown() {
     AllWindowEvents.removeListener("progress", this);
     AllWindowEvents.removeListener("TabSelect", this);
   },
 };
 
-// Manages tab mappings and permissions for a specific extension.
-function ExtensionTabManager(extension) {
-  this.extension = extension;
-
-  // A mapping of tab objects to the inner window ID the extension currently has
-  // the active tab permission for. The active permission for a given tab is
-  // valid only for the inner window that was active when the permission was
-  // granted. If the tab navigates, the inner window ID changes, and the
-  // permission automatically becomes stale.
-  //
-  // WeakMap[tab => inner-window-id<int>]
-  this.hasTabPermissionFor = new WeakMap();
-}
-
-ExtensionTabManager.prototype = {
-  addActiveTabPermission(tab = TabManager.activeTab) {
-    if (this.extension.hasPermission("activeTab")) {
-      // Note that, unlike Chrome, we don't currently clear this permission with
-      // the tab navigates. If the inner window is revived from BFCache before
-      // we've granted this permission to a new inner window, the extension
-      // maintains its permissions for it.
-      this.hasTabPermissionFor.set(tab, tab.linkedBrowser.innerWindowID);
-    }
-  },
-
-  // Returns true if the extension has the "activeTab" permission for this tab.
-  // This is somewhat more permissive than the generic "tabs" permission, as
-  // checked by |hasTabPermission|, in that it also allows programmatic script
-  // injection without an explicit host permission.
-  hasActiveTabPermission(tab) {
-    // This check is redundant with addTabPermission, but cheap.
-    if (this.extension.hasPermission("activeTab")) {
-      return (this.hasTabPermissionFor.has(tab) &&
-              this.hasTabPermissionFor.get(tab) === tab.linkedBrowser.innerWindowID);
-    }
-    return false;
-  },
-
-  hasTabPermission(tab) {
-    return this.extension.hasPermission("tabs") || this.hasActiveTabPermission(tab);
-  },
-
-  convert(tab) {
-    let window = tab.ownerDocument.defaultView;
-    let windowActive = window == WindowManager.topWindow;
-
-    let result = {
-      id: TabManager.getId(tab),
-      index: tab._tPos,
-      windowId: WindowManager.getId(window),
-      selected: tab.selected,
-      highlighted: tab.selected,
-      active: tab.selected,
-      pinned: tab.pinned,
-      status: TabManager.getStatus(tab),
-      incognito: PrivateBrowsingUtils.isBrowserPrivate(tab.linkedBrowser),
-      width: tab.linkedBrowser.clientWidth,
-      height: tab.linkedBrowser.clientHeight,
-    };
-
-    if (this.hasTabPermission(tab)) {
-      result.url = tab.linkedBrowser.currentURI.spec;
-      if (tab.linkedBrowser.contentTitle) {
-        result.title = tab.linkedBrowser.contentTitle;
-      }
-      let icon = window.gBrowser.getIcon(tab);
-      if (icon) {
-        result.favIconUrl = icon;
-      }
-    }
-
-    return result;
-  },
-
-  getTabs(window) {
-    return Array.from(window.gBrowser.tabs, tab => this.convert(tab));
-  },
-};
-
-
-// Manages global mappings between XUL tabs and extension tab IDs.
+// Manages mapping between XUL tabs and extension tab IDs.
 global.TabManager = {
   _tabs: new WeakMap(),
   _nextId: 1,
 
   getId(tab) {
     if (this._tabs.has(tab)) {
       return this._tabs.get(tab);
     }
@@ -397,36 +317,54 @@ global.TabManager = {
     return null;
   },
 
   getStatus(tab) {
     return tab.getAttribute("busy") == "true" ? "loading" : "complete";
   },
 
   convert(extension, tab) {
-    return TabManager.for(extension).convert(tab);
+    let window = tab.ownerDocument.defaultView;
+    let windowActive = window == WindowManager.topWindow;
+    let result = {
+      id: this.getId(tab),
+      index: tab._tPos,
+      windowId: WindowManager.getId(window),
+      selected: tab.selected,
+      highlighted: tab.selected,
+      active: tab.selected,
+      pinned: tab.pinned,
+      status: this.getStatus(tab),
+      incognito: PrivateBrowsingUtils.isBrowserPrivate(tab.linkedBrowser),
+      width: tab.linkedBrowser.clientWidth,
+      height: tab.linkedBrowser.clientHeight,
+    };
+
+    if (extension.hasPermission("tabs")) {
+      result.url = tab.linkedBrowser.currentURI.spec;
+      if (tab.linkedBrowser.contentTitle) {
+        result.title = tab.linkedBrowser.contentTitle;
+      }
+      let icon = window.gBrowser.getIcon(tab);
+      if (icon) {
+        result.favIconUrl = icon;
+      }
+    }
+
+    return result;
+  },
+
+  getTabs(extension, window) {
+    if (!window.gBrowser) {
+      return [];
+    }
+    return Array.map(window.gBrowser.tabs, tab => this.convert(extension, tab));
   },
 };
 
-// WeakMap[Extension -> ExtensionTabManager]
-let tabManagers = new WeakMap();
-
-// Returns the extension-specific tab manager for the given extension, or
-// creates one if it doesn't already exist.
-TabManager.for = function (extension) {
-  if (!tabManagers.has(extension)) {
-    tabManagers.set(extension, new ExtensionTabManager(extension));
-  }
-  return tabManagers.get(extension);
-};
-
-extensions.on("shutdown", (type, extension) => {
-  tabManagers.delete(extension);
-});
-
 // Manages mapping between XUL windows and extension window IDs.
 global.WindowManager = {
   _windows: new WeakMap(),
   _nextId: 0,
 
   WINDOW_ID_NONE: -1,
   WINDOW_ID_CURRENT: -2,
 
@@ -468,17 +406,17 @@ global.WindowManager = {
       incognito: PrivateBrowsingUtils.isWindowPrivate(window),
 
       // We fudge on these next two.
       type: this.windowType(window),
       state: window.fullScreen ? "fullscreen" : "normal",
     };
 
     if (getInfo && getInfo.populate) {
-      results.tabs = TabManager.for(extension).getTabs(window);
+      results.tabs = TabManager.getTabs(extension, window);
     }
 
     return result;
   },
 };
 
 // Manages listeners for window opening and closing. A window is
 // considered open when the "load" event fires on it. A window is
--- a/browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
@@ -1,259 +1,32 @@
-"use strict";
-
-function* testHasPermission(params) {
-  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://mochi.test:8888/", true);
-
-  let contentSetup = params.contentSetup || (() => Promise.resolve());
+add_task(function* () {
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://mochi.test:8888/");
 
   let extension = ExtensionTestUtils.loadExtension({
-    manifest: params.manifest,
+    manifest: {
+      "permissions": ["tabs"]
+    },
 
-    background: `(${function(contentSetup) {
+    background: function() {
       browser.runtime.onMessage.addListener((msg, sender) => {
         browser.test.assertEq(msg, "script ran", "script ran");
         browser.test.notifyPass("executeScript");
       });
 
-      browser.test.onMessage.addListener(msg => {
-        browser.test.assertEq(msg, "execute-script");
-
-        browser.tabs.executeScript({
-          file: "script.js"
-        });
+      browser.tabs.executeScript({
+        file: "script.js"
       });
-
-      contentSetup().then(() => {
-        browser.test.sendMessage("ready");
-      });
-    }})(${contentSetup})`,
+    },
 
     files: {
       "script.js": function() {
         browser.runtime.sendMessage("script ran");
       }
     }
   });
 
   yield extension.startup();
-  yield extension.awaitMessage("ready");
-
-  if (params.setup) {
-    yield params.setup(extension);
-  }
-
-  extension.sendMessage("execute-script");
-
   yield extension.awaitFinish("executeScript");
   yield extension.unload();
 
   yield BrowserTestUtils.removeTab(tab);
-}
-
-// This is a pretty terrible hack, but it's the best we can do until we
-// support |executeScript| callbacks and |lastError|.
-function* testHasNoPermission(params) {
-  let tab1 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/");
-  let tab2 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, "http://mochi.test:8888/");
-
-  let contentSetup = params.contentSetup || (() => Promise.resolve());
-
-  let extension = ExtensionTestUtils.loadExtension({
-    manifest: params.manifest,
-
-    background: `(${function(contentSetup) {
-      browser.runtime.onMessage.addListener((msg, sender) => {
-        browser.test.assertEq(msg, "second script ran", "second script ran");
-        browser.test.notifyPass("executeScript");
-      });
-
-      browser.test.onMessage.addListener(msg => {
-        browser.test.assertEq(msg, "execute-script");
-
-        browser.tabs.query({ activeWindow: true }, tabs => {
-          browser.tabs.executeScript({
-            file: "script.js"
-          });
-
-          // Execute a script we know we have permissions for in the
-          // second tab, in the hopes that it will execute after the
-          // first one. This has intermittent failure written all over
-          // it, but it's just about the best we can do until we
-          // support callbacks for executeScript.
-          browser.tabs.executeScript(tabs[1].id, {
-            file: "second-script.js"
-          });
-        });
-      });
-
-      contentSetup().then(() => {
-        browser.test.sendMessage("ready");
-      });
-    }})(${contentSetup})`,
-
-    files: {
-      "script.js": function() {
-        browser.runtime.sendMessage("first script ran");
-      },
-
-      "second-script.js": function() {
-        browser.runtime.sendMessage("second script ran");
-      }
-    }
-  });
-
-  yield extension.startup();
-  yield extension.awaitMessage("ready");
-
-  if (params.setup) {
-    yield params.setup(extension);
-  }
-
-  extension.sendMessage("execute-script");
-
-  yield extension.awaitFinish("executeScript");
-  yield extension.unload();
-
-  yield BrowserTestUtils.removeTab(tab2);
-  yield BrowserTestUtils.removeTab(tab1);
-}
-
-add_task(function* testGoodPermissions() {
-  info("Test explicit host permission");
-  yield testHasPermission({
-    manifest: { "permissions": ["http://mochi.test/"] }
-  });
-
-  info("Test explicit host subdomain permission");
-  yield testHasPermission({
-    manifest: { "permissions": ["http://*.mochi.test/"] }
-  });
-
-  info("Test explicit <all_urls> permission");
-  yield testHasPermission({
-    manifest: { "permissions": ["<all_urls>"] }
-  });
-
-  info("Test activeTab permission with a browser action click");
-  yield testHasPermission({
-    manifest: {
-      "permissions": ["activeTab"],
-      "browser_action": {},
-    },
-    setup: clickBrowserAction,
-  });
-
-  info("Test activeTab permission with a page action click");
-  yield testHasPermission({
-    manifest: {
-      "permissions": ["activeTab"],
-      "page_action": {},
-    },
-    contentSetup() {
-      return new Promise(resolve => {
-        browser.tabs.query({ active: true, currentWindow: true }, tabs => {
-          browser.pageAction.show(tabs[0].id);
-          resolve();
-        });
-      });
-    },
-    setup: clickPageAction,
-  });
-
-  info("Test activeTab permission with a browser action w/popup click");
-  yield testHasPermission({
-    manifest: {
-      "permissions": ["activeTab"],
-      "browser_action": { "default_popup": "_blank.html" },
-    },
-    setup: clickBrowserAction,
-  });
-
-  info("Test activeTab permission with a page action w/popup click");
-  yield testHasPermission({
-    manifest: {
-      "permissions": ["activeTab"],
-      "page_action": { "default_popup": "_blank.html" },
-    },
-    contentSetup() {
-      return new Promise(resolve => {
-        browser.tabs.query({ active: true, currentWindow: true }, tabs => {
-          browser.pageAction.show(tabs[0].id);
-          resolve();
-        });
-      });
-    },
-    setup: clickPageAction,
-  });
-
-  info("Test activeTab permission with a context menu click");
-  yield testHasPermission({
-    manifest: {
-      "permissions": ["activeTab", "contextMenus"],
-    },
-    contentSetup() {
-      browser.contextMenus.create({ title: "activeTab", contexts: ["all"] });
-      return Promise.resolve();
-    },
-    setup: function* (extension) {
-      info("setup");
-      let contextMenu = document.getElementById("contentAreaContextMenu");
-      let awaitPopupShown = BrowserTestUtils.waitForEvent(contextMenu, "popupshown");
-      let awaitPopupHidden = BrowserTestUtils.waitForEvent(contextMenu, "popuphidden");
-
-      yield BrowserTestUtils.synthesizeMouseAtCenter("a[href]", { type: "contextmenu", button: 2 },
-                                                     gBrowser.selectedBrowser);
-      info("clicked");
-      yield awaitPopupShown;
-      info("shown");
-
-      let item = contextMenu.querySelector("[label=activeTab]");
-
-      yield EventUtils.synthesizeMouseAtCenter(item, {}, window);
-      info("re-clicked");
-
-      yield awaitPopupHidden;
-      info("hidden");
-    },
-  });
 });
-
-add_task(function* testBadPermissions() {
-  info("Test no special permissions");
-  yield testHasNoPermission({
-    manifest: { "permissions": ["http://example.com/"] }
-  });
-
-  info("Test tabs permissions");
-  yield testHasNoPermission({
-    manifest: { "permissions": ["http://example.com/", "tabs"] }
-  });
-
-  info("Test active tab, browser action, no click");
-  yield testHasNoPermission({
-    manifest: {
-      "permissions": ["http://example.com/", "activeTab"],
-      "browser_action": {},
-    },
-  });
-
-  info("Test active tab, page action, no click");
-  yield testHasNoPermission({
-    manifest: {
-      "permissions": ["http://example.com/", "activeTab"],
-      "page_action": {},
-    },
-    contentSetup() {
-      return new Promise(resolve => {
-        browser.tabs.query({ active: true, currentWindow: true }, tabs => {
-          browser.pageAction.show(tabs[0].id);
-          resolve();
-        });
-      });
-    }
-  });
-});
-
-// TODO: Test that |executeScript| fails if the tab has navigated to a
-// new page, and no longer matches our expected state. This involves
-// intentionally trying to trigger a race condition, and is probably not
-// even worth attempting until we have proper |executeScript| callbacks.
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -39,11 +39,11 @@ function clickPageAction(extension, win 
   //
   // Unfortunately, that doesn't happen automatically in browser chrome
   // tests.
   SetPageProxyState("valid");
 
   let pageActionId = makeWidgetId(extension.id) + "-page-action";
   let elem = win.document.getElementById(pageActionId);
 
-  EventUtils.synthesizeMouseAtCenter(elem, {}, win);
+  EventUtils.synthesizeMouse(elem, 8, 8, {}, win);
   return new Promise(SimpleTest.executeSoon);
 }