Bug 841096 - Part 1: speed up about:sync-tabs. r=rnewman

browser/base/content/sync/aboutSyncTabs.js

@@ -140,16 +140,19 @@ let RemoteTabViewer = {
         list.removeItemAt(i);
     }
 
+    let seenURLs = new Set();
+    let localURLs = engine.getOpenURLs();
+
     for (let [guid, client] in Iterator(engine.getAllClients())) {
       // Create the client node, but don't add it in-case we don't show any tabs
       let appendClient = true;
-      let seenURLs = {};
+
       client.tabs.forEach(function({title, urlHistory, icon}) {
         let url = urlHistory[0];
-        if (engine.locallyOpenTabMatchesURL(url) || url in seenURLs)
+        if (!url || localURLs.has(url) || seenURLs.has(url)) {
           return;
-
-        seenURLs[url] = null;
+        }
+        seenURLs.add(url);
 
         if (appendClient) {
           let attrs = {

browser/metro/base/content/startui/RemoteTabsView.js

@@ -68,6 +68,7 @@ RemoteTabsView.prototype = Util.extend(Object.create(View.prototype), {
     let tabsEngine = Weave.Service.engineManager.get("tabs");
     let list = this._set;
     let seenURLs = new Set();
+    let localURLs = tabsEngine.getOpenURLs();
 
     // Clear grid, We don't know what has happened to tabs since last sync
     // Also can result in duplicate tabs(bug 864614)
@@ -76,7 +77,7 @@ RemoteTabsView.prototype = Util.extend(Object.create(View.prototype), {
     for (let [guid, client] in Iterator(tabsEngine.getAllClients())) {
       client.tabs.forEach(function({title, urlHistory, icon}) {
         let url = urlHistory[0];
-        if (tabsEngine.locallyOpenTabMatchesURL(url) || seenURLs.has(url)) {
+        if (!url || getOpenURLs.has(url) || seenURLs.has(url)) {
           return;
         }
         seenURLs.add(url);

services/sync/modules/engines/tabs.js

@@ -73,19 +73,15 @@ TabEngine.prototype = {
     this.service.resource(url).delete();
   },
 
-  /* The intent is not to show tabs in the menu if they're already
-   * open locally.  There are a couple ways to interpret this: for
-   * instance, we could do it by removing a tab from the list when
-   * you open it -- but then if you close it, you can't get back to
-   * it.  So the way I'm doing it here is to not show a tab in the menu
-   * if you have a tab open to the same URL, even though this means
-   * that as soon as you navigate anywhere, the original tab will
-   * reappear in the menu.
+  /**
+   * Return a Set of open URLs.
    */
-  locallyOpenTabMatchesURL: function TabEngine_localTabMatches(url) {
-    return this._store.getAllTabs().some(function (tab) {
-      return tab.urlHistory[0] == url;
-    });
+  getOpenURLs: function () {
+    let urls = new Set();
+    for (let entry of this._store.getAllTabs()) {
+      urls.add(entry.urlHistory[0]);
+    }
+    return urls;
   }
 };
 

services/sync/tests/unit/test_tab_engine.js

@@ -30,7 +30,7 @@ function fakeSessionSvc() {
 
 function run_test() {
 
-  _("test locallyOpenTabMatchesURL");
+  _("test getOpenURLs");
   let engine = new TabEngine(Service);
 
   // 3 tabs
@@ -39,10 +39,11 @@ function run_test() {
   let matches;
 
   _("  test matching works (true)");
-  matches = engine.locallyOpenTabMatchesURL("http://foo.com");
+  let openurlsset = engine.getOpenURLs();
+  matches = openurlsset.has("http://foo.com");
   do_check_true(matches);
 
   _("  test matching works (false)");
-  matches = engine.locallyOpenTabMatchesURL("http://barfoo.com");
+  matches = openurlsset.has("http://barfoo.com");
   do_check_false(matches);
 }