Bug 1466678 - Store multiple selected tabs in a WeakSet instead of a WeakMap r=jaws
authorOriol Brufau <oriol-bugzilla@hotmail.com>
Mon, 04 Jun 2018 23:20:46 +0200
changeset 421333 5915bc04887591653c2953c3affe33d7c0c9da20
parent 421332 9acec00d049242f4f9b17de389782c2ae29cbf90
child 421334 721c4f5f0fe78f00c41addab8006a8df84763566
push id34091
push userbtara@mozilla.com
push dateTue, 05 Jun 2018 13:52:34 +0000
treeherdermozilla-central@752465b44c79 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1466678
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1466678 - Store multiple selected tabs in a WeakSet instead of a WeakMap r=jaws MozReview-Commit-ID: 3sfRtQruInF
browser/base/content/tabbrowser.js
browser/base/content/test/tabs/browser_multiselect_tabs_using_Ctrl.js
browser/base/content/test/tabs/browser_multiselect_tabs_using_Shift.js
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -131,17 +131,17 @@ window._gBrowser = {
     "resumeMedia", "mute", "unmute", "blockedPopups", "lastURI",
     "purgeSessionHistory", "stopScroll", "startScroll",
     "userTypedValue", "userTypedClear",
     "didStartLoadSinceLastUserTyping", "audioMuted"
   ],
 
   _removingTabs: [],
 
-  _multiSelectedTabsMap: new WeakMap(),
+  _multiSelectedTabsSet: new WeakSet(),
 
   _lastMultiSelectedTabRef: null,
 
   /**
    * Tab close requests are ignored if the window is closing anyway,
    * e.g. when holding Ctrl+W.
    */
   _windowIsClosing: false,
@@ -2619,18 +2619,18 @@ window._gBrowser = {
     this.removeCollectionOfTabs(tabs);
   },
 
   removeMultiSelectedTabs() {
     if (!this.warnAboutClosingTabs(this.closingTabsEnum.MULTI_SELECTED)) {
       return;
     }
 
-    let selectedTabs = ChromeUtils.nondeterministicGetWeakMapKeys(this._multiSelectedTabsMap)
-                                    .filter(tab => tab.isConnected);
+    let selectedTabs = ChromeUtils.nondeterministicGetWeakSetKeys(this._multiSelectedTabsSet)
+                                  .filter(tab => tab.isConnected);
     this.removeCollectionOfTabs(selectedTabs);
   },
 
   removeCollectionOfTabs(tabs) {
     let tabsWithBeforeUnload = [];
     let lastToClose;
     let aParams = {animation: true};
     for (let tab of tabs) {
@@ -3625,17 +3625,17 @@ window._gBrowser = {
   },
 
   addToMultiSelectedTabs(aTab) {
     if (aTab.multiselected) {
       return;
     }
 
     aTab.setAttribute("multiselected", "true");
-    this._multiSelectedTabsMap.set(aTab, null);
+    this._multiSelectedTabsSet.add(aTab);
   },
 
   /**
    * Adds two given tabs and all tabs between them into the (multi) selected tabs collection
    */
   addRangeToMultiSelectedTabs(aTab1, aTab2) {
     // Let's avoid going through all the heavy process below when the same
     // tab is given as params.
@@ -3656,38 +3656,38 @@ window._gBrowser = {
     }
   },
 
   removeFromMultiSelectedTabs(aTab) {
     if (!aTab.multiselected) {
       return;
     }
     aTab.removeAttribute("multiselected");
-    this._multiSelectedTabsMap.delete(aTab);
+    this._multiSelectedTabsSet.delete(aTab);
   },
 
   clearMultiSelectedTabs() {
-    const selectedTabs = ChromeUtils.nondeterministicGetWeakMapKeys(this._multiSelectedTabsMap);
+    const selectedTabs = ChromeUtils.nondeterministicGetWeakSetKeys(this._multiSelectedTabsSet);
     for (let tab of selectedTabs) {
       if (tab.isConnected && tab.multiselected) {
         tab.removeAttribute("multiselected");
       }
     }
-    this._multiSelectedTabsMap = new WeakMap();
+    this._multiSelectedTabsSet = new WeakSet();
   },
 
   get multiSelectedTabsCount() {
-    return ChromeUtils.nondeterministicGetWeakMapKeys(this._multiSelectedTabsMap)
+    return ChromeUtils.nondeterministicGetWeakSetKeys(this._multiSelectedTabsSet)
       .filter(tab => tab.isConnected && !tab.closing)
       .length;
   },
 
   get lastMultiSelectedTab() {
     let tab = this._lastMultiSelectedTabRef ? this._lastMultiSelectedTabRef.get() : null;
-    if (tab && tab.isConnected && this._multiSelectedTabsMap.has(tab)) {
+    if (tab && tab.isConnected && this._multiSelectedTabsSet.has(tab)) {
       return tab;
     }
     return gBrowser.selectedTab;
   },
 
   set lastMultiSelectedTab(aTab) {
     this._lastMultiSelectedTabRef = Cu.getWeakReference(aTab);
   },
--- a/browser/base/content/test/tabs/browser_multiselect_tabs_using_Ctrl.js
+++ b/browser/base/content/test/tabs/browser_multiselect_tabs_using_Ctrl.js
@@ -1,13 +1,13 @@
 const PREF_MULTISELECT_TABS = "browser.tabs.multiselect";
 
 add_task(async function clickWithoutPrefSet() {
   let tab = await addTab();
-  let mSelectedTabs = gBrowser._multiSelectedTabsMap;
+  let mSelectedTabs = gBrowser._multiSelectedTabsSet;
 
   isnot(gBrowser.selectedTab, tab, "Tab doesn't have focus");
 
   // We make sure that the tab-switch is completely done before executing checks
   await BrowserTestUtils.switchTab(gBrowser, () => {
     triggerClickOn(tab, { ctrlKey: true });
   });
 
@@ -24,17 +24,17 @@ add_task(async function clickWithoutPref
 
 add_task(async function clickWithPrefSet() {
   await SpecialPowers.pushPrefEnv({
     set: [
       [PREF_MULTISELECT_TABS, true]
     ]
   });
 
-  let mSelectedTabs = gBrowser._multiSelectedTabsMap;
+  let mSelectedTabs = gBrowser._multiSelectedTabsSet;
   const initialFocusedTab = gBrowser.selectedTab;
   const tab = await addTab();
 
   await triggerClickOn(tab, { ctrlKey: true });
   ok(tab.multiselected && mSelectedTabs.has(tab), "Tab should be (multi) selected after click");
   isnot(gBrowser.selectedTab, tab, "Multi-selected tab is not focused");
   is(gBrowser.selectedTab, initialFocusedTab, "Focused tab doesn't change");
 
@@ -55,18 +55,18 @@ add_task(async function clearSelection()
   const tab1 = await addTab();
   const tab2 = await addTab();
   const tab3 = await addTab();
 
   info("We multi-select tab1 and tab2 with ctrl key down");
   await triggerClickOn(tab1, { ctrlKey: true });
   await triggerClickOn(tab2, { ctrlKey: true });
 
-  ok(tab1.multiselected && gBrowser._multiSelectedTabsMap.has(tab1), "Tab1 is (multi) selected");
-  ok(tab2.multiselected && gBrowser._multiSelectedTabsMap.has(tab2), "Tab2 is (multi) selected");
+  ok(tab1.multiselected && gBrowser._multiSelectedTabsSet.has(tab1), "Tab1 is (multi) selected");
+  ok(tab2.multiselected && gBrowser._multiSelectedTabsSet.has(tab2), "Tab2 is (multi) selected");
   is(gBrowser.multiSelectedTabsCount, 2, "Two tabs (multi) selected");
   isnot(tab3, gBrowser.selectedTab, "Tab3 doesn't have focus");
 
   info("We select tab3 with Ctrl key up");
   await triggerClickOn(tab3, { ctrlKey: false });
 
   ok(!tab1.multiselected, "Tab1 is not (multi) selected");
   ok(!tab2.multiselected, "Tab2 is not (multi) selected");
--- a/browser/base/content/test/tabs/browser_multiselect_tabs_using_Shift.js
+++ b/browser/base/content/test/tabs/browser_multiselect_tabs_using_Shift.js
@@ -1,16 +1,16 @@
 const PREF_MULTISELECT_TABS = "browser.tabs.multiselect";
 
 add_task(async function prefNotSet() {
     let tab1 = await addTab();
     let tab2 = await addTab();
     let tab3 = await addTab();
 
-    let mSelectedTabs = gBrowser._multiSelectedTabsMap;
+    let mSelectedTabs = gBrowser._multiSelectedTabsSet;
 
     await BrowserTestUtils.switchTab(gBrowser, tab1);
 
     is(gBrowser.selectedTab, tab1, "Tab1 has focus now");
     is(gBrowser.multiSelectedTabsCount, 0, "No tab is mutli-selected");
 
     info("Click on tab3 while holding shift key");
     await BrowserTestUtils.switchTab(gBrowser, () => {
@@ -37,17 +37,17 @@ add_task(async function setPref() {
 });
 
 add_task(async function noItemsInTheCollectionBeforeShiftClicking() {
     let tab1 = await addTab();
     let tab2 = await addTab();
     let tab3 = await addTab();
     let tab4 = await addTab();
     let tab5 = await addTab();
-    let mSelectedTabs = gBrowser._multiSelectedTabsMap;
+    let mSelectedTabs = gBrowser._multiSelectedTabsSet;
 
     await BrowserTestUtils.switchTab(gBrowser, tab1);
 
     is(gBrowser.selectedTab, tab1, "Tab1 has focus now");
     is(gBrowser.multiSelectedTabsCount, 0, "No tab is multi-selected");
 
     gBrowser.hideTab(tab3);
     ok(tab3.hidden, "Tab3 is hidden");
@@ -72,17 +72,17 @@ add_task(async function noItemsInTheColl
 
 add_task(async function itemsInTheCollectionBeforeShiftClicking() {
     let tab1 = await addTab();
     let tab2 = await addTab();
     let tab3 = await addTab();
     let tab4 = await addTab();
     let tab5 = await addTab();
 
-    let mSelectedTabs = gBrowser._multiSelectedTabsMap;
+    let mSelectedTabs = gBrowser._multiSelectedTabsSet;
 
     await BrowserTestUtils.switchTab(gBrowser, () => triggerClickOn(tab1, {}));
 
     is(gBrowser.selectedTab, tab1, "Tab1 has focus now");
     is(gBrowser.multiSelectedTabsCount, 0, "No tab is multi-selected");
 
     await triggerClickOn(tab3, { ctrlKey: true });
     is(gBrowser.selectedTab, tab1, "Tab1 still has focus");