Bug 841096 - Part 2: speed up retrieval of open tabs. r=rnewman

services/sync/modules/engines/tabs.js

@@ -96,39 +96,60 @@ TabStore.prototype = {
     return id == this.engine.service.clientsEngine.localID;
   },
 
-  getAllTabs: function getAllTabs(filter) {
+  getWindowEnumerator: function () {
+    return Services.wm.getEnumerator("navigator:browser");
+  },
+
+  shouldSkipWindow: function (win) {
+    return win.closed ||
+           PrivateBrowsingUtils.isWindowPrivate(win);
+  },
+
+  getTabState: function (tab) {
+    return JSON.parse(Svc.Session.getTabState(tab));
+  },
+
+  getAllTabs: function (filter) {
     let filteredUrls = new RegExp(Svc.Prefs.get("engine.tabs.filteredUrls"), "i");
 
     let allTabs = [];
 
-    let currentState = JSON.parse(Svc.Session.getBrowserState());
-    currentState.windows.forEach(function (window) {
-      if (window.isPrivate) {
-        return;
+    let winEnum = this.getWindowEnumerator();
+    while (winEnum.hasMoreElements()) {
+      let win = winEnum.getNext();
+      if (this.shouldSkipWindow(win)) {
+        continue;
       }
-      window.tabs.forEach(function (tab) {
+
+      dump("WIN IS " + JSON.stringify(win)  + "\n");
+      for (let tab of win.gBrowser.tabs) {
+        tabState = this.getTabState(tab);
+
         // Make sure there are history entries to look at.
-        if (!tab.entries.length)
-          return;
+        if (!tabState || !tabState.entries.length) {
+          continue;
+        }
+
         // Until we store full or partial history, just grab the current entry.
         // index is 1 based, so make sure we adjust.
-        let entry = tab.entries[tab.index - 1];
+        let entry = tabState.entries[tabState.index - 1];
 
         // Filter out some urls if necessary. SessionStore can return empty
         // tabs in some cases - easiest thing is to just ignore them for now.
-        if (!entry.url || filter && filteredUrls.test(entry.url))
-          return;
+        if (!entry.url || filter && filteredUrls.test(entry.url)) {
+          continue;
+        }
 
         // I think it's also possible that attributes[.image] might not be set
         // so handle that as well.
         allTabs.push({
           title: entry.title || "",
           urlHistory: [entry.url],
-          icon: tab.attributes && tab.attributes.image || "",
-          lastUsed: Math.floor((tab.lastAccessed || 0) / 1000)
+          icon: tabState.attributes && tabState.attributes.image || "",
+          lastUsed: Math.floor((tabState.lastAccessed || 0) / 1000)
         });
-      });
-    });
+      }
+    }
 
     return allTabs;
   },

services/sync/tests/unit/head_helpers.js

@@ -2,6 +2,7 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 Cu.import("resource://services-common/async.js");
+Cu.import("resource://testing-common/services-common/utils.js");
 
 let provider = {
   getFile: function(prop, persistent) {
@@ -124,3 +125,70 @@ function generateNewKeys(collectionKeys, collections=null) {
   collectionKeys.setContents(wbo.cleartext, modified);
 }
 
+// Helpers for testing open tabs.
+// These reflect part of the internal structure of TabEngine,
+// and stub part of Service.wm.
+
+function mockShouldSkipWindow (win) {
+  return win.closed ||
+         win.mockIsPrivate;
+}
+
+function mockGetTabState (tab) {
+  return tab;
+}
+
+function mockGetWindowEnumerator(url, numWindows, numTabs) {
+  let elements = [];
+  for (let w = 0; w < numWindows; ++w) {
+    let tabs = [];
+    let win = {
+      closed: false,
+      mockIsPrivate: false,
+      gBrowser: {
+        tabs: tabs,
+      },
+    };
+    elements.push(win);
+
+    for (let t = 0; t < numTabs; ++t) {
+      tabs.push(TestingUtils.deepCopy({
+        index: 1,
+        entries: [{
+          url: ((typeof url == "string") ? url : url()),
+          title: "title"
+        }],
+        attributes: {
+          image: "image"
+        },
+        lastAccessed: 1499
+      }));
+    }
+  }
+
+  // Always include a closed window and a private window.
+  elements.push({
+    closed: true,
+    mockIsPrivate: false,
+    gBrowser: {
+      tabs: [],
+    },
+  });
+ 
+  elements.push({
+    closed: false,
+    mockIsPrivate: true,
+    gBrowser: {
+      tabs: [],
+    },
+  });
+
+  return {
+    hasMoreElements: function () {
+      return elements.length;
+    },
+    getNext: function () {
+      return elements.shift();
+    },
+  };
+}

services/sync/tests/unit/test_tab_engine.js

@@ -5,36 +5,23 @@ Cu.import("resource://services-sync/engines/tabs.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 
-function fakeSessionSvc() {
-  let tabs = [];
-  for(let i = 0; i < arguments.length; i++) {
-    tabs.push({
-      index: 1,
-      entries: [{
-        url: arguments[i],
-        title: "title"
-      }],
-      attributes: {
-        image: "image"
-      }
-    });
-  }
-  let obj = {windows: [{tabs: tabs}]};
-
-  // delete the getter, or the previously created fake Session
-  delete Svc.Session;
-  Svc.Session = {
-    getBrowserState: function() JSON.stringify(obj)
-  };
+function getMocks() {
+  let engine = new TabEngine(Service);
+  let store = engine._store;
+  store.getTabState = mockGetTabState;
+  store.shouldSkipWindow = mockShouldSkipWindow;
+  return [engine, store];
 }
 
 function run_test() {
+  _("Test getOpenURLs.");
+  let [engine, store] = getMocks();
 
-  _("test getOpenURLs");
-  let engine = new TabEngine(Service);
-
-  // 3 tabs
-  fakeSessionSvc("http://bar.com", "http://foo.com", "http://foobar.com");
+  let urls = ["http://bar.com", "http://foo.com", "http://foobar.com"];
+  function threeURLs() {
+    return urls.pop();
+  }
+  store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, threeURLs, 1, 3);
 
   let matches;
 

services/sync/tests/unit/test_tab_store.js

@@ -6,6 +6,14 @@ Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://testing-common/services-common/utils.js");
 
+function getMockStore() {
+  let engine = new TabEngine(Service);
+  let store = engine._store;
+  store.getTabState = mockGetTabState;
+  store.shouldSkipWindow = mockShouldSkipWindow;
+  return store;
+}
+
 function test_create() {
   let store = new TabEngine(Service)._store;
 
@@ -40,43 +48,15 @@ function test_create() {
   Svc.Prefs.reset("notifyTabState");
 }
 
-function fakeSessionSvc(url, numtabs) {
-  // first delete the getter, or the previously
-  // created fake Session
-  delete Svc.Session;
-  Svc.Session = {
-    getBrowserState: function() {
-      let obj = {
-        windows: [{
-          tabs: [{
-            index: 1,
-            entries: [{
-              url: url,
-              title: "title"
-            }],
-            attributes: {
-              image: "image"
-            },
-            lastAccessed: 1499
-          }]
-        }]
-      };
-      if (numtabs) {
-        let tabs = obj.windows[0].tabs;
-        for (let i = 0; i < numtabs-1; i++)
-          tabs.push(TestingUtils.deepCopy(tabs[0]));
-      }
-      return JSON.stringify(obj);
-    }
-  };
-};
-
 function test_getAllTabs() {
-  let store = new TabEngine(Service)._store, tabs;
+  let store = getMockStore();
+  let tabs;
 
-  _("get all tabs");
-  fakeSessionSvc("http://foo.com");
+  store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://foo.com", 1, 1);
+
+  _("Get all tabs.");
   tabs = store.getAllTabs();
+  _("Tabs: " + JSON.stringify(tabs));
   do_check_eq(tabs.length, 1);
   do_check_eq(tabs[0].title, "title");
   do_check_eq(tabs[0].urlHistory.length, 1);
@@ -84,31 +64,32 @@ function test_getAllTabs() {
   do_check_eq(tabs[0].icon, "image");
   do_check_eq(tabs[0].lastUsed, 1);
 
-  _("get all tabs, and check that filtering works");
-  // we don't bother testing every URL type here, the
-  // filteredUrls regex really should have it own tests
-  fakeSessionSvc("about:foo");
+  _("Get all tabs, and check that filtering works.");
+  store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "about:foo", 1, 1);
   tabs = store.getAllTabs(true);
+  _("Filtered: " + JSON.stringify(tabs));
   do_check_eq(tabs.length, 0);
 }
 
 function test_createRecord() {
-  let store = new TabEngine(Service)._store, record;
+  let store = getMockStore();
+  let record;
+
+  store.getTabState = mockGetTabState;
+  store.shouldSkipWindow = mockShouldSkipWindow;
+  store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://foo.com", 1, 1);
 
-  // get some values before testing
-  fakeSessionSvc("http://foo.com");
   let tabs = store.getAllTabs();
   let tabsize = JSON.stringify(tabs[0]).length;
   let numtabs = Math.ceil(20000./77.);
 
-  _("create a record");
-  fakeSessionSvc("http://foo.com");
+  store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://foo.com", 1, 1);
   record = store.createRecord("fake-guid");
   do_check_true(record instanceof TabSetRecord);
   do_check_eq(record.tabs.length, 1);
 
   _("create a big record");
-  fakeSessionSvc("http://foo.com", numtabs);
+  store.getWindowEnumerator = mockGetWindowEnumerator.bind(this, "http://foo.com", 1, numtabs);
   record = store.createRecord("fake-guid");
   do_check_true(record instanceof TabSetRecord);
   do_check_eq(record.tabs.length, 256);