Bug 1876743 - Enable cross-container Tab Search in Nightly. r=mseibert

Fixes code to properly run tests with the feature enabled.
Fixes code not considering payload.userContextId is set to -1 for private
windows.
Fixes a bug in the _openTabs Map where multiple open tabs to the same url are
not properly counted.

Differential Revision: https://phabricator.services.mozilla.com/D200036

Author: mak77

browser/app/profile/firefox.js

@@ -556,7 +556,11 @@ pref("browser.urlbar.switchTabs.adoptIntoActiveWindow", false);
 
 // Controls whether searching for open tabs returns tabs from any container
 // or only from the current container.
+#ifdef NIGHTLY_BUILD
+pref("browser.urlbar.switchTabs.searchAllContainers", true);
+#else
 pref("browser.urlbar.switchTabs.searchAllContainers", false);
+#endif
 
 // Whether addresses and search results typed into the address bar
 // should be opened in new tabs by default.

browser/components/sessionstore/test/browser_restore_container_tabs_oa.js

@@ -4,6 +4,10 @@
 
 "use strict";
 
+const { UrlbarProviderOpenTabs } = ChromeUtils.importESModule(
+  "resource:///modules/UrlbarProviderOpenTabs.sys.mjs"
+);
+
 const PATH = "browser/browser/components/sessionstore/test/empty.html";
 
 /* import-globals-from ../../../base/content/test/tabs/helper_origin_attrs_testing.js */
@@ -118,6 +122,13 @@ add_task(async function testRestore() {
     "Should have restore data for the closed window"
   );
 
+  Assert.equal(
+    0,
+    (await UrlbarProviderOpenTabs.getDatabaseRegisteredOpenTabsForTests())
+      .length,
+    "No registered open pages should be left"
+  );
+
   // Now restore the window
   newWin = SessionStore.undoCloseWindow(0);
 
@@ -228,4 +239,11 @@ add_task(async function testRestore() {
   }
 
   await BrowserTestUtils.closeWindow(newWin);
+
+  Assert.equal(
+    0,
+    (await UrlbarProviderOpenTabs.getDatabaseRegisteredOpenTabsForTests())
+      .length,
+    "No registered open pages should be left"
+  );
 });

browser/components/urlbar/UrlbarInput.sys.mjs

@@ -1094,7 +1094,10 @@ export class UrlbarInput {
           Services.io.newURI(url),
           true,
           loadOpts,
-          lazy.UrlbarPrefs.get("switchTabs.searchAllContainers")
+          lazy.UrlbarPrefs.get("switchTabs.searchAllContainers") &&
+            lazy.UrlbarProviderOpenTabs.isNonPrivateUserContextId(
+              result.payload.userContextId
+            )
             ? result.payload.userContextId
             : null
         );

browser/components/urlbar/UrlbarMuxerUnifiedComplete.sys.mjs

@@ -16,6 +16,7 @@ const lazy = {};
 ChromeUtils.defineESModuleGetters(lazy, {
   QuickSuggest: "resource:///modules/QuickSuggest.sys.mjs",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.sys.mjs",
+  UrlbarProviderOpenTabs: "resource:///modules/UrlbarProviderOpenTabs.sys.mjs",
   UrlbarProviderQuickSuggest:
     "resource:///modules/UrlbarProviderQuickSuggest.sys.mjs",
   UrlbarProviderTabToSearch:
@@ -40,7 +41,10 @@ function makeMapKeyForTabResult(result) {
   return UrlbarUtils.tupleString(
     result.payload.url,
     lazy.UrlbarPrefs.get("switchTabs.searchAllContainers") &&
-      result.type == UrlbarUtils.RESULT_TYPE.TAB_SWITCH
+      result.type == UrlbarUtils.RESULT_TYPE.TAB_SWITCH &&
+      lazy.UrlbarProviderOpenTabs.isNonPrivateUserContextId(
+        result.payload.userContextId
+      )
       ? result.payload.userContextId
       : undefined
   );

browser/components/urlbar/UrlbarProviderInputHistory.sys.mjs

@@ -69,7 +69,7 @@ const SQL_ADAPTIVE_QUERY = `/* do not warn (bug 487789) */
    JOIN moz_places h ON h.id = i.place_id
    LEFT JOIN moz_openpages_temp t
           ON t.url = h.url
-         AND t.userContextId = :userContextId
+          AND (t.userContextId = :userContextId OR (t.userContextId <> -1 AND :userContextId IS NULL))
    WHERE AUTOCOMPLETE_MATCH(NULL, h.url,
                             IFNULL(btitle, h.title), tags,
                             h.visit_count, h.typed, bookmarked,
@@ -252,11 +252,12 @@ class ProviderInputHistory extends UrlbarProvider {
         search_string: queryContext.searchString.toLowerCase(),
         matchBehavior: Ci.mozIPlacesAutoComplete.MATCH_ANYWHERE,
         searchBehavior: lazy.UrlbarPrefs.get("defaultBehavior"),
-        userContextId:
-          lazy.UrlbarProviderOpenTabs.getUserContextIdForOpenPagesTable(
-            queryContext.userContextId,
-            queryContext.isPrivate
-          ),
+        userContextId: lazy.UrlbarPrefs.get("switchTabs.searchAllContainers")
+          ? lazy.UrlbarProviderOpenTabs.getUserContextIdForOpenPagesTable(
+              null,
+              queryContext.isPrivate
+            )
+          : queryContext.userContextId,
         maxResults: queryContext.maxResults,
       },
     ];

browser/components/urlbar/UrlbarProviderOpenTabs.sys.mjs

@@ -75,26 +75,48 @@ export class UrlbarProviderOpenTabs extends UrlbarProvider {
 
   /**
    * Maps the open tabs by userContextId.
+   * Each entry is a Map of url => count.
    */
   static _openTabs = new Map();
 
   /**
-   * Return urls that is opening on given user context id.
+   * Return unique urls that are open for given user context id.
    *
    * @param {integer} userContextId Containers user context id
-   * @param {boolean} isInPrivateWindow In private browsing window or not
+   * @param {boolean} [isInPrivateWindow] In private browsing window or not
    * @returns {Array} urls
    */
-  static getOpenTabs(userContextId, isInPrivateWindow) {
+  static getOpenTabs(userContextId, isInPrivateWindow = false) {
     userContextId = UrlbarProviderOpenTabs.getUserContextIdForOpenPagesTable(
       userContextId,
       isInPrivateWindow
     );
-    return UrlbarProviderOpenTabs._openTabs.get(userContextId);
+    return Array.from(
+      UrlbarProviderOpenTabs._openTabs.get(userContextId)?.keys() ?? []
+    );
   }
 
   /**
-   * Return userContextId that will be used in moz_openpages_temp table.
+   * Return urls registered in the memory table.
+   * This is mostly for testing purposes.
+   *
+   * @returns {Array} Array of {url, userContextId, count} objects.
+   */
+  static async getDatabaseRegisteredOpenTabsForTests() {
+    let conn = await lazy.PlacesUtils.promiseLargeCacheDBConnection();
+    let rows = await conn.execute(
+      "SELECT url, userContextId, open_count FROM moz_openpages_temp"
+    );
+    return rows.map(r => ({
+      url: r.getResultByIndex(0),
+      userContextId: r.getResultByIndex(1),
+      count: r.getResultByIndex(2),
+    }));
+  }
+
+  /**
+   * Return userContextId that is used in the moz_openpages_temp table and
+   * returned as part of the payload. It differs only for private windows.
    *
    * @param {integer} userContextId Containers user context id
    * @param {boolean} isInPrivateWindow In private browsing window or not
@@ -104,6 +126,26 @@ export class UrlbarProviderOpenTabs extends UrlbarProvider {
     return isInPrivateWindow ? PRIVATE_USER_CONTEXT_ID : userContextId;
   }
 
+  /**
+   * Return whether the provided userContextId is for a non-private tab.
+   *
+   * @param {integer} userContextId the userContextId to evaluate
+   * @returns {boolean}
+   */
+  static isNonPrivateUserContextId(userContextId) {
+    return userContextId != PRIVATE_USER_CONTEXT_ID;
+  }
+
+  /**
+   * Return whether the provided userContextId is for a container.
+   *
+   * @param {integer} userContextId the userContextId to evaluate
+   * @returns {boolean}
+   */
+  static isContainerUserContextId(userContextId) {
+    return userContextId > 0;
+  }
+
   /**
    * Copy over cached open tabs to the memory table once the Urlbar
    * connection has been initialized.
@@ -113,9 +155,11 @@ export class UrlbarProviderOpenTabs extends UrlbarProvider {
       // Must be set before populating.
       UrlbarProviderOpenTabs.memoryTableInitialized = true;
       // Populate the table with the current cached tabs.
-      for (let [userContextId, urls] of UrlbarProviderOpenTabs._openTabs) {
-        for (let url of urls) {
-          await addToMemoryTable(url, userContextId).catch(console.error);
+      for (let [userContextId, entries] of UrlbarProviderOpenTabs._openTabs) {
+        for (let [url, count] of entries) {
+          await addToMemoryTable(url, userContextId, count).catch(
+            console.error
+          );
         }
       }
     });
@@ -138,10 +182,12 @@ export class UrlbarProviderOpenTabs extends UrlbarProvider {
       isInPrivateWindow
     );
 
-    if (!UrlbarProviderOpenTabs._openTabs.has(userContextId)) {
-      UrlbarProviderOpenTabs._openTabs.set(userContextId, []);
+    let entries = UrlbarProviderOpenTabs._openTabs.get(userContextId);
+    if (!entries) {
+      entries = new Map();
+      UrlbarProviderOpenTabs._openTabs.set(userContextId, entries);
     }
-    UrlbarProviderOpenTabs._openTabs.get(userContextId).push(url);
+    entries.set(url, (entries.get(url) ?? 0) + 1);
     await addToMemoryTable(url, userContextId).catch(console.error);
   }
 
@@ -163,13 +209,19 @@ export class UrlbarProviderOpenTabs extends UrlbarProvider {
       isInPrivateWindow
     );
 
-    let openTabs = UrlbarProviderOpenTabs._openTabs.get(userContextId);
-    if (openTabs) {
-      let index = openTabs.indexOf(url);
-      if (index != -1) {
-        openTabs.splice(index, 1);
-        await removeFromMemoryTable(url, userContextId).catch(console.error);
+    let entries = UrlbarProviderOpenTabs._openTabs.get(userContextId);
+    if (entries) {
+      let oldCount = entries.get(url);
+      if (oldCount == 0) {
+        console.error("Tried to unregister a non registered open tab");
+        return;
       }
+      if (oldCount == 1) {
+        entries.delete(url);
+      } else {
+        entries.set(url, oldCount - 1);
+      }
+      await removeFromMemoryTable(url, userContextId).catch(console.error);
     }
   }
 
@@ -222,9 +274,10 @@ export class UrlbarProviderOpenTabs extends UrlbarProvider {
  *
  * @param {string} url Address of the page
  * @param {number} userContextId Containers user context id
+ * @param {number} [count] The number of times the page is open
  * @returns {Promise} resolved after the addition.
  */
-async function addToMemoryTable(url, userContextId) {
+async function addToMemoryTable(url, userContextId, count = 1) {
   if (!UrlbarProviderOpenTabs.memoryTableInitialized) {
     return;
   }
@@ -239,11 +292,11 @@ async function addToMemoryTable(url, userContextId) {
                           FROM moz_openpages_temp
                           WHERE url = :url
                           AND userContextId = :userContextId ),
-                        1
+                        :count
                       )
               )
     `,
-      { url, userContextId }
+      { url, userContextId, count }
     );
   });
 }

browser/components/urlbar/UrlbarProviderPlaces.sys.mjs

@@ -68,7 +68,7 @@ function defaultQuery(conditions = "") {
      FROM moz_places h
      LEFT JOIN moz_openpages_temp t
             ON t.url = h.url
-            AND (:userContextId IS NULL OR t.userContextId = :userContextId)
+            AND (t.userContextId = :userContextId OR (t.userContextId <> -1 AND :userContextId IS NULL))
      WHERE ${PAGES_FRECENCY_FIELD} <> 0
        AND CASE WHEN bookmarked
          THEN
@@ -95,7 +95,7 @@ const SQL_SWITCHTAB_QUERY = `SELECT :query_type, t.url, t.url, NULL, NULL, NULL,
    FROM moz_openpages_temp t
    LEFT JOIN moz_places h ON h.url_hash = hash(t.url) AND h.url = t.url
    WHERE h.id IS NULL
-     AND (:userContextId IS NULL OR t.userContextId = :userContextId)
+     AND (t.userContextId = :userContextId OR (t.userContextId <> -1 AND :userContextId IS NULL))
      AND AUTOCOMPLETE_MATCH(:searchString, t.url, t.url, NULL,
                             NULL, NULL, NULL, t.open_count,
                             :matchBehavior, :searchBehavior, NULL)
@@ -169,7 +169,8 @@ function makeMapKeyForResult(url, match) {
   return UrlbarUtils.tupleString(
     url,
     action?.type == "switchtab" &&
-      lazy.UrlbarPrefs.get("switchTabs.searchAllContainers")
+      lazy.UrlbarPrefs.get("switchTabs.searchAllContainers") &&
+      lazy.UrlbarProviderOpenTabs.isNonPrivateUserContextId(match.userContextId)
       ? match.userContextId
       : undefined
   );
@@ -469,12 +470,6 @@ function Search(queryContext, listener, provider) {
     this._filterOnHost = engine.searchUrlDomain;
   }
 
-  this._userContextId =
-    lazy.UrlbarProviderOpenTabs.getUserContextIdForOpenPagesTable(
-      this._userContextId,
-      this._inPrivateWindow
-    );
-
   // Use the original string here, not the stripped one, so the tokenizer can
   // properly recognize token types.
   let { tokens } = lazy.UrlbarTokenizer.tokenize({
@@ -1298,7 +1293,10 @@ Search.prototype = {
     params.userContextId = lazy.UrlbarPrefs.get(
       "switchTabs.searchAllContainers"
     )
-      ? null
+      ? lazy.UrlbarProviderOpenTabs.getUserContextIdForOpenPagesTable(
+          null,
+          this._inPrivateWindow
+        )
       : this._userContextId;
 
     if (this._filterOnHost) {
@@ -1325,7 +1323,10 @@ Search.prototype = {
         // original search string.
         searchString: this._keywordFilteredSearchString,
         userContextId: lazy.UrlbarPrefs.get("switchTabs.searchAllContainers")
-          ? null
+          ? lazy.UrlbarProviderOpenTabs.getUserContextIdForOpenPagesTable(
+              null,
+              this._inPrivateWindow
+            )
           : this._userContextId,
         maxResults: this._maxResults,
       },

browser/components/urlbar/UrlbarUtils.sys.mjs

@@ -21,6 +21,7 @@ ChromeUtils.defineESModuleGetters(lazy, {
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.sys.mjs",
   UrlbarProviderInterventions:
     "resource:///modules/UrlbarProviderInterventions.sys.mjs",
+  UrlbarProviderOpenTabs: "resource:///modules/UrlbarProviderOpenTabs.sys.mjs",
   UrlbarProvidersManager: "resource:///modules/UrlbarProvidersManager.sys.mjs",
   UrlbarProviderSearchTips:
     "resource:///modules/UrlbarProviderSearchTips.sys.mjs",
@@ -2129,8 +2130,10 @@ export class UrlbarQueryContext {
     this.deferUserSelectionProviders = new Set();
     this.trimmedSearchString = this.searchString.trim();
     this.userContextId =
-      options.userContextId ||
-      Ci.nsIScriptSecurityManager.DEFAULT_USER_CONTEXT_ID;
+      lazy.UrlbarProviderOpenTabs.getUserContextIdForOpenPagesTable(
+        options.userContextId,
+        this.isPrivate
+      ) || Ci.nsIScriptSecurityManager.DEFAULT_USER_CONTEXT_ID;
   }
 
   /**

browser/components/urlbar/UrlbarView.sys.mjs

@@ -13,6 +13,7 @@ ChromeUtils.defineESModuleGetters(lazy, {
     "resource://gre/modules/ContextualIdentityService.sys.mjs",
   L10nCache: "resource:///modules/UrlbarUtils.sys.mjs",
   ObjectUtils: "resource://gre/modules/ObjectUtils.sys.mjs",
+  UrlbarProviderOpenTabs: "resource:///modules/UrlbarProviderOpenTabs.sys.mjs",
   UrlbarPrefs: "resource:///modules/UrlbarPrefs.sys.mjs",
   UrlbarProviderQuickSuggest:
     "resource:///modules/UrlbarProviderQuickSuggest.sys.mjs",
@@ -2631,7 +2632,9 @@ export class UrlbarView {
     if (
       lazy.UrlbarPrefs.get("switchTabs.searchAllContainers") &&
       result.type == lazy.UrlbarUtils.RESULT_TYPE.TAB_SWITCH &&
-      result.payload.userContextId
+      lazy.UrlbarProviderOpenTabs.isContainerUserContextId(
+        result.payload.userContextId
+      )
     ) {
       let label = lazy.ContextualIdentityService.getUserContextLabel(
         result.payload.userContextId