Bug 1445316 - Optimize tabs.query with 'active', 'currentWindow', 'highlighted', 'index', 'lastFocusedWindow' or 'windowId' draft
authorOriol Brufau <oriol-bugzilla@hotmail.com>
Tue, 13 Mar 2018 20:52:58 +0100
changeset 775220 7f4b8170dc21ec5e4423fc7881acb6928a99518f
parent 775219 0405f6006f3a3f653dd42d587c3eefe08cffa37d
push id104660
push userbmo:oriol-bugzilla@hotmail.com
push dateFri, 30 Mar 2018 17:03:03 +0000
bugs1445316
milestone61.0a1
Bug 1445316 - Optimize tabs.query with 'active', 'currentWindow', 'highlighted', 'index', 'lastFocusedWindow' or 'windowId' MozReview-Commit-ID: L5i129iC44W
browser/components/extensions/parent/ext-browser.js
browser/components/extensions/test/browser/browser_ext_tabs_query.js
mobile/android/components/extensions/ext-utils.js
mobile/android/components/extensions/test/mochitest/test_ext_tabs_query.html
toolkit/components/extensions/parent/ext-tabs-base.js
--- a/browser/components/extensions/parent/ext-browser.js
+++ b/browser/components/extensions/parent/ext-browser.js
@@ -646,20 +646,16 @@ class Tab extends TabBase {
   get pinned() {
     return this.nativeTab.pinned;
   }
 
   get active() {
     return this.nativeTab.selected;
   }
 
-  get highlighted() {
-    return this.nativeTab.selected;
-  }
-
   get selected() {
     return this.nativeTab.selected;
   }
 
   get status() {
     if (this.nativeTab.getAttribute("busy") === "true") {
       return "loading";
     }
@@ -871,16 +867,23 @@ class Window extends WindowBase {
   }
 
   get activeTab() {
     let {tabManager} = this.extension;
 
     return tabManager.getWrapper(this.window.gBrowser.selectedTab);
   }
 
+  getTabAtIndex(index) {
+    let nativeTab = this.window.gBrowser.tabs[index];
+    if (nativeTab) {
+      return this.extension.tabManager.getWrapper(nativeTab);
+    }
+  }
+
   /**
    * Converts session store data to an object compatible with the return value
    * of the convert() method, representing that data.
    *
    * @param {Extension} extension
    *        The extension for which to convert the data.
    * @param {Object} windowData
    *        Session store data for a closed window, as returned by
--- a/browser/components/extensions/test/browser/browser_ext_tabs_query.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_query.js
@@ -35,21 +35,16 @@ add_task(async function() {
         browser.test.assertEq(tabs[0].url, "about:robots", "tab 0 url correct");
         browser.test.assertEq(tabs[1].url, "about:config", "tab 1 url correct");
 
         browser.test.assertEq(tabs[0].status, "complete", "tab 0 status correct");
         browser.test.assertEq(tabs[1].status, "complete", "tab 1 status correct");
 
         browser.test.assertEq(tabs[0].title, "Gort! Klaatu barada nikto!", "tab 0 title correct");
 
-        browser.test.assertThrows(
-          () => browser.tabs.query({index: -1}),
-          /-1 is too small \(must be at least 0\)/,
-          "tab indices must be non-negative");
-
         browser.test.notifyPass("tabs.query");
       });
     },
   });
 
   await extension.startup();
   await extension.awaitFinish("tabs.query");
   await extension.unload();
@@ -290,8 +285,72 @@ add_task(async function testQueryWithout
   });
 
   await extension.startup();
 
   await extension.awaitFinish("testQueryWithoutURLOrTitlePermissions");
 
   await extension.unload();
 });
+
+add_task(async function test_query_index() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "permissions": ["tabs"],
+    },
+
+    background: function() {
+      browser.tabs.onCreated.addListener(async function({index, windowId, id}) {
+        browser.test.assertThrows(
+          () => browser.tabs.query({index: -1}),
+          /-1 is too small \(must be at least 0\)/,
+          "tab indices must be non-negative");
+
+        let tabs = await browser.tabs.query({index, windowId});
+        browser.test.assertEq(tabs.length, 1, `Got one tab at index ${index}`);
+        browser.test.assertEq(tabs[0].id, id, "The tab is the right one");
+
+        tabs = await browser.tabs.query({index: 1e5, windowId});
+        browser.test.assertEq(tabs.length, 0, "There is no tab at this index");
+
+        browser.test.notifyPass("tabs.query");
+      });
+    },
+  });
+
+  await extension.startup();
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/");
+  await extension.awaitFinish("tabs.query");
+  BrowserTestUtils.removeTab(tab);
+  await extension.unload();
+});
+
+add_task(async function test_query_window() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "permissions": ["tabs"],
+    },
+
+    background: async function() {
+      let badWindowId = 0;
+      for (let {id} of await browser.windows.getAll()) {
+        badWindowId = Math.max(badWindowId, id + 1);
+      }
+
+      let tabs = await browser.tabs.query({windowId: badWindowId});
+      browser.test.assertEq(tabs.length, 0, "No tabs because there is no such window ID");
+
+      let {id: currentWindowId} = await browser.windows.getCurrent();
+      tabs = await browser.tabs.query({currentWindow: true});
+      browser.test.assertEq(tabs[0].windowId, currentWindowId, "Got tabs from the current window");
+
+      let {id: lastFocusedWindowId} = await browser.windows.getLastFocused();
+      tabs = await browser.tabs.query({lastFocusedWindow: true});
+      browser.test.assertEq(tabs[0].windowId, lastFocusedWindowId, "Got tabs from the last focused window");
+
+      browser.test.notifyPass("tabs.query");
+    },
+  });
+
+  await extension.startup();
+  await extension.awaitFinish("tabs.query");
+  await extension.unload();
+});
--- a/mobile/android/components/extensions/ext-utils.js
+++ b/mobile/android/components/extensions/ext-utils.js
@@ -539,20 +539,16 @@ class Tab extends TabBase {
       // all the active tabs.
       if (tabTracker.extensionPopupTab === this.nativeTab) {
         return false;
       }
     }
     return this.nativeTab.getActive();
   }
 
-  get highlighted() {
-    return this.nativeTab.getActive();
-  }
-
   get selected() {
     return this.nativeTab.getActive();
   }
 
   get status() {
     if (this.browser.webProgress.isLoadingDocument) {
       return "loading";
     }
@@ -687,19 +683,34 @@ class Window extends WindowBase {
     let {tabManager} = this.extension;
 
     for (let nativeTab of this.window.BrowserApp.tabs) {
       yield tabManager.getWrapper(nativeTab);
     }
   }
 
   get activeTab() {
-    let {tabManager} = this.extension;
+    let {BrowserApp} = this.window;
+    let {selectedTab} = BrowserApp;
+
+    // If the current tab is an extension popup tab, we use the parentId to retrieve
+    // and return the tab that was selected when the popup tab has been opened.
+    if (selectedTab === tabTracker.extensionPopupTab) {
+      selectedTab = BrowserApp.getTabForId(selectedTab.parentId);
+    }
 
-    return tabManager.getWrapper(this.window.BrowserApp.selectedTab);
+    let {tabManager} = this.extension;
+    return tabManager.getWrapper(selectedTab);
+  }
+
+  getTabAtIndex(index) {
+    let nativeTab = this.window.BrowserApp.tabs[index];
+    if (nativeTab) {
+      return this.extension.tabManager.getWrapper(nativeTab);
+    }
   }
 }
 
 Object.assign(global, {Tab, TabContext, Window});
 
 class TabManager extends TabManagerBase {
   get(tabId, default_ = undefined) {
     let nativeTab = tabTracker.getTab(tabId, default_);
--- a/mobile/android/components/extensions/test/mochitest/test_ext_tabs_query.html
+++ b/mobile/android/components/extensions/test/mochitest/test_ext_tabs_query.html
@@ -8,49 +8,105 @@
   <script type="text/javascript" src="head.js"></script>
   <link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
 </head>
 <body>
 
 <script type="text/javascript">
 "use strict";
 
+var {BrowserActions} = SpecialPowers.Cu.import("resource://gre/modules/BrowserActions.jsm", {});
 var {Services} = SpecialPowers.Cu.import("resource://gre/modules/Services.jsm", {});
 
 add_task(async function test_query_highlighted() {
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "permissions": ["tabs"],
+      "browser_action": {
+        "default_popup": "popup.html",
+      },
     },
 
     background: async function() {
       let tabs1 = await browser.tabs.query({highlighted: false});
       browser.test.assertEq(3, tabs1.length, "should have three non-highlighted tabs");
 
       let tabs2 = await browser.tabs.query({highlighted: true});
       browser.test.assertEq(1, tabs2.length, "should have one highlighted tab");
 
       for (let tab of [...tabs1, ...tabs2]) {
         browser.test.assertEq(tab.active, tab.highlighted, "highlighted and active are equal in tab " + tab.index);
       }
 
       browser.test.notifyPass("tabs.query");
     },
+
+    files: {
+      "popup.html": `<script src="popup.js"><\/script>`,
+      "popup.js": async function popupScript() {
+        let active = await browser.tabs.query({active: true});
+        let highlighted = await browser.tabs.query({highlighted: true});
+
+        browser.test.assertEq(1, active.length, "should have one active tab");
+        browser.test.assertEq(1, highlighted.length, "should have one highlighted tab");
+        browser.test.assertEq(active[0].id, highlighted[0].id, "the active and highlighted tabs are the same one");
+
+        browser.test.sendMessage("tabs.query.popup");
+      },
+    },
   });
 
   const {BrowserApp} = Services.wm.getMostRecentWindow("navigator:browser");
   let tabs = [];
   for (let url of ["http://example.com/", "http://example.net/", "http://test1.example.org/MochiKit/"]) {
     tabs.push(BrowserApp.addTab(url));
   }
 
   await extension.startup();
   await extension.awaitFinish("tabs.query");
+
+  // Open popup
+  BrowserActions.synthesizeClick(`{${extension.uuid}}`);
+  await extension.awaitMessage("tabs.query.popup");
+
   await extension.unload();
 
   for (let tab of tabs) {
     BrowserApp.closeTab(tab);
   }
 });
+
+add_task(async function test_query_index() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "permissions": ["tabs"],
+    },
+
+    background: function() {
+      browser.tabs.onCreated.addListener(async function({index, windowId, id}) {
+        browser.test.assertThrows(
+          () => browser.tabs.query({index: -1}),
+          /-1 is too small \(must be at least 0\)/,
+          "tab indices must be non-negative");
+
+        let tabs = await browser.tabs.query({index, windowId});
+        browser.test.assertEq(tabs.length, 1, `Got one tab at index ${index}`);
+        browser.test.assertEq(tabs[0].id, id, "The tab is the right one");
+
+        tabs = await browser.tabs.query({index: 1e5, windowId});
+        browser.test.assertEq(tabs.length, 0, "There is no tab at this index");
+
+        browser.test.notifyPass("tabs.query");
+      });
+    },
+  });
+
+  const {BrowserApp} = Services.wm.getMostRecentWindow("navigator:browser");
+  await extension.startup();
+  let tab = BrowserApp.addTab("http://example.com/");
+  await extension.awaitFinish("tabs.query");
+  BrowserApp.closeTab(tab);
+  await extension.unload();
+});
 </script>
 
 </body>
 </html>
--- a/toolkit/components/extensions/parent/ext-tabs-base.js
+++ b/toolkit/components/extensions/parent/ext-tabs-base.js
@@ -394,16 +394,26 @@ class TabBase {
    *        @readonly
    *        @abstract
    */
   get active() {
     throw new Error("Not implemented");
   }
 
   /**
+   * @property {boolean} highlighted
+   *        Alias for `active`.
+   *        @readonly
+   *        @abstract
+   */
+  get highlighted() {
+    return this.active;
+  }
+
+  /**
    * @property {boolean} selected
    *        An alias for `active`.
    *        @readonly
    *        @abstract
    */
   get selected() {
     throw new Error("Not implemented");
   }
@@ -1037,16 +1047,28 @@ class WindowBase {
   }
 
   /**
    * @property {TabBase} The window's currently active tab.
    */
   get activeTab() {
     throw new Error("Not implemented");
   }
+
+  /**
+   * Returns the window's tab at the specified index.
+   *
+   * @param {integer} index
+   *        The index of the desired tab.
+   *
+   * @returns {TabBase|undefined}
+   */
+  getTabAtIndex(index) {
+    throw new Error("Not implemented");
+  }
   /* eslint-enable valid-jsdoc */
 }
 
 Object.assign(WindowBase, {WINDOW_ID_NONE, WINDOW_ID_CURRENT});
 
 /**
  * The parameter type of "tab-attached" events, which are emitted when a
  * pre-existing tab is attached to a new window.
@@ -1363,43 +1385,49 @@ class WindowTrackerBase extends EventEmi
    * browser window if the context does not belong to a browser window.
    *
    * @param {BaseContext} context
    *        The extension context for which to return the current window.
    *
    * @returns {DOMWindow|null}
    */
   getCurrentWindow(context) {
-    return context.currentWindow || this.topWindow;
+    return (context && context.currentWindow) || this.topWindow;
   }
 
   /**
    * Returns the browser window with the given ID.
    *
    * @param {integer} id
    *        The ID of the window to return.
    * @param {BaseContext} context
    *        The extension context for which the matching is being performed.
    *        Used to determine the current window for relevant properties.
+   * @param {boolean} [strict = true]
+   *        If false, undefined will be returned instead of throwing an error
+   *        in case no window exists with the given ID.
    *
-   * @returns {DOMWindow}
+   * @returns {DOMWindow|undefined}
    * @throws {ExtensionError}
-   *        If no window exists with the given ID.
+   *        If no window exists with the given ID and `strict` is true.
    */
-  getWindow(id, context) {
+  getWindow(id, context, strict = true) {
     if (id === WINDOW_ID_CURRENT) {
       return this.getCurrentWindow(context);
     }
 
     for (let window of this.browserWindows(true)) {
       if (this.getId(window) === id) {
         return window;
       }
     }
-    throw new ExtensionError(`Invalid window ID: ${id}`);
+
+    if (strict) {
+      throw new ExtensionError(`Invalid window ID: ${id}`);
+    }
   }
 
   /**
    * @property {boolean} _haveListeners
    *        Returns true if any window open or close listeners are currently
    *        registered.
    * @private
    */
@@ -1795,20 +1823,38 @@ class TabManagerBase {
    *        {@link WindowBase#matches}. Unknown properties will be ignored.
    * @param {BaseContext|null} [context = null]
    *        The extension context for which the matching is being performed.
    *        Used to determine the current window for relevant properties.
    *
    * @returns {Iterator<TabBase>}
    */
   * query(queryInfo = null, context = null) {
-    for (let window of this.extension.windowManager.query(queryInfo, context)) {
-      for (let tab of window.getTabs()) {
-        if (!queryInfo || tab.matches(queryInfo)) {
-          yield tab;
+    function* candidates(windowWrapper) {
+      if (queryInfo) {
+        let {active, highlighted, index} = queryInfo;
+        if (active === true || highlighted === true) {
+          yield windowWrapper.activeTab;
+          return;
+        }
+        if (index != null) {
+          let tabWrapper = windowWrapper.getTabAtIndex(index);
+          if (tabWrapper) {
+            yield tabWrapper;
+          }
+          return;
+        }
+      }
+      yield* windowWrapper.getTabs();
+    }
+    let windowWrappers = this.extension.windowManager.query(queryInfo, context);
+    for (let windowWrapper of windowWrappers) {
+      for (let tabWrapper of candidates(windowWrapper)) {
+        if (!queryInfo || tabWrapper.matches(queryInfo)) {
+          yield tabWrapper;
         }
       }
     }
   }
 
   /**
    * Returns a TabBase wrapper for the tab with the given ID.
    *
@@ -1896,19 +1942,39 @@ class WindowManagerBase {
    *        Unknown properties will be ignored.
    * @param {BaseContext|null} [context = null]
    *        The extension context for which the matching is being performed.
    *        Used to determine the current window for relevant properties.
    *
    * @returns {Iterator<WindowBase>}
    */
   * query(queryInfo = null, context = null) {
-    for (let window of this.getAll()) {
-      if (!queryInfo || window.matches(queryInfo, context)) {
-        yield window;
+    function* candidates(windowManager) {
+      if (queryInfo) {
+        let {currentWindow, windowId, lastFocusedWindow} = queryInfo;
+        if (currentWindow === true && windowId == null) {
+          windowId = WINDOW_ID_CURRENT;
+        }
+        if (windowId != null) {
+          let window = global.windowTracker.getWindow(windowId, context, false);
+          if (window) {
+            yield windowManager.getWrapper(window);
+          }
+          return;
+        }
+        if (lastFocusedWindow === true) {
+          yield windowManager.getWrapper(global.windowTracker.topWindow);
+          return;
+        }
+      }
+      yield* windowManager.getAll();
+    }
+    for (let windowWrapper of candidates(this)) {
+      if (!queryInfo || windowWrapper.matches(queryInfo, context)) {
+        yield windowWrapper;
       }
     }
   }
 
   /**
    * Returns a WindowBase wrapper for the browser window with the given ID.
    *
    * @param {integer} id