Bug 1225690 - implement a SyncedTabs module to abstract away Sync's internals. r=rnewman
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 01 Dec 2015 13:07:21 +1100
changeset 308829 4cbc740f137d847a1bc73bf5a6bb476fc83c92ca
parent 308828 643e963294b0baafe86412ae723f06b164607cf9
child 308830 3307a801862270294770042f6e9b19ff92ed5b24
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs1225690
milestone45.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 1225690 - implement a SyncedTabs module to abstract away Sync's internals. r=rnewman
services/sync/modules/SyncedTabs.jsm
services/sync/modules/engines/tabs.js
services/sync/moz.build
services/sync/tests/unit/test_syncedtabs.js
services/sync/tests/unit/xpcshell.ini
new file mode 100644
--- /dev/null
+++ b/services/sync/modules/SyncedTabs.jsm
@@ -0,0 +1,267 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+this.EXPORTED_SYMBOLS = ["SyncedTabs"];
+
+
+const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
+
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+Cu.import("resource://gre/modules/Task.jsm");
+Cu.import("resource://gre/modules/Log.jsm");
+Cu.import("resource://gre/modules/PlacesUtils.jsm", this);
+Cu.import("resource://services-sync/main.js");
+Cu.import("resource://gre/modules/Preferences.jsm");
+
+// The Sync XPCOM service
+XPCOMUtils.defineLazyGetter(this, "weaveXPCService", function() {
+  return Cc["@mozilla.org/weave/service;1"]
+           .getService(Ci.nsISupports)
+           .wrappedJSObject;
+});
+
+// from MDN...
+function escapeRegExp(string) {
+  return string.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+}
+
+// A topic we fire whenever we have new tabs available. This might be due
+// to a request made by this module to refresh the tab list, or as the result
+// of a regularly scheduled sync. The intent is that consumers just listen
+// for this notification and update their UI in response.
+const TOPIC_TABS_CHANGED = "services.sync.tabs.changed";
+
+// The interval, in seconds, before which we consider the existing list
+// of tabs "fresh enough" and don't force a new sync.
+const TABS_FRESH_ENOUGH_INTERVAL = 30;
+
+let log = Log.repository.getLogger("Sync.RemoteTabs");
+// A new scope to do the logging thang...
+(function() {
+  let level = Preferences.get("services.sync.log.logger.tabs");
+  if (level) {
+    let appender = new Log.DumpAppender();
+    log.level = appender.level = Log.Level[level] || Log.Level.Debug;
+    log.addAppender(appender);
+  }
+})();
+
+
+// A private singleton that does the work.
+let SyncedTabsInternal = {
+  _getClientIcon(id) {
+    let isMobile = Weave.Service.clientsEngine.isMobile(id);
+    if (isMobile) {
+      return "chrome://browser/skin/sync-mobileIcon.png";
+    }
+    return "chrome://browser/skin/sync-desktopIcon.png";
+  },
+
+  /* Make a "tab" record. Returns a promise */
+  _makeTab: Task.async(function* (client, tab, url) {
+    let icon = tab.icon;
+    if (!icon) {
+      try {
+        icon = (yield PlacesUtils.promiseFaviconLinkUrl(url)).spec;
+      } catch (ex) { /* no favicon avaiable */ }
+    }
+    if (!icon) {
+      icon = PlacesUtils.favicons.defaultFavicon.spec;
+    }
+    return {
+      type:  "tab",
+      title: tab.title || url,
+      url,
+      icon,
+      client: client.id,
+      lastUsed: tab.lastUsed,
+    };
+  }),
+
+  /* Make a "client" record. Returns a promise for consistency with _makeTab */
+  _makeClient: Task.async(function* (client) {
+    return {
+      id: client.id,
+      type: "client",
+      name: client.clientName,
+      icon:  this._getClientIcon(client.id),
+      tabs: []
+    };
+  }),
+
+  _tabMatchesFilter(tab, filter) {
+    let reFilter = new RegExp(escapeRegExp(filter), "i");
+    return tab.url.match(reFilter) || tab.title.match(reFilter);
+  },
+
+  getTabClients: Task.async(function* (filter) {
+    log.info("Generating tab list with filter", filter);
+    let result = [];
+
+    // If Sync isn't ready, don't try and get anything.
+    if (!weaveXPCService.ready) {
+      log.debug("Sync isn't yet ready, so returning an empty tab list");
+      return result;
+    }
+
+    let engine = Weave.Service.engineManager.get("tabs");
+
+    let seenURLs = new Set();
+    let parentIndex = 0;
+    let ntabs = 0;
+
+    for (let [guid, client] in Iterator(engine.getAllClients())) {
+      let clientRepr = yield this._makeClient(client);
+      log.debug("Processing client", clientRepr);
+
+      for (let tab of client.tabs) {
+        let url = tab.urlHistory[0];
+        log.debug("remote tab", url);
+        // Note there are some issues with tracking "seen" tabs, including:
+        // * We really can't return the entire urlHistory record as we are
+        //   only checking the first entry - others might be different.
+        // * We don't update the |lastUsed| timestamp to reflect the
+        //   most-recently-seen time.
+        // In a followup we should consider simply dropping this |seenUrls|
+        // check and return duplicate records - it seems the user will be more
+        // confused by tabs not showing up on a device (because it was detected
+        // as a dupe so it only appears on a different device) than being
+        // confused by seeing the same tab on different clients.
+        if (!url || seenURLs.has(url)) {
+          continue;
+        }
+        let tabRepr = yield this._makeTab(client, tab, url);
+        if (filter && !this._tabMatchesFilter(tabRepr, filter)) {
+          continue;
+        }
+        seenURLs.add(url);
+        clientRepr.tabs.push(tabRepr);
+      }
+      // We return all clients, even those without tabs - the consumer should
+      // filter it if they care.
+      ntabs += clientRepr.tabs.length;
+      result.push(clientRepr);
+    }
+    log.info(`Final tab list has ${result.length} clients with ${ntabs} tabs.`);
+    return result;
+  }),
+
+  syncTabs(force) {
+    if (!force) {
+      // Don't bother refetching tabs if we already did so recently
+      let lastFetch = Preferences.get("services.sync.lastTabFetch", 0);
+      let now = Math.floor(Date.now() / 1000);
+      if (now - lastFetch < TABS_FRESH_ENOUGH_INTERVAL) {
+        log.info("_refetchTabs was done recently, do not doing it again");
+        return Promise.resolve(false);
+      }
+    }
+
+    // If Sync isn't configured don't try and sync, else we will get reports
+    // of a login failure.
+    if (Weave.Status.checkSetup() == Weave.CLIENT_NOT_CONFIGURED) {
+      log.info("Sync client is not configured, so not attempting a tab sync");
+      return Promise.resolve(false);
+    }
+    // Ask Sync to just do the tabs engine if it can.
+    // Sync is currently synchronous, so do it after an event-loop spin to help
+    // keep the UI responsive.
+    return new Promise((resolve, reject) => {
+      Services.tm.currentThread.dispatch(() => {
+        try {
+          log.info("Doing a tab sync.");
+          Weave.Service.sync(["tabs"]);
+          resolve(true);
+        } catch (ex) {
+          log.error("Sync failed", ex);
+          reject(ex);
+        };
+      }, Ci.nsIThread.DISPATCH_NORMAL);
+    });
+  },
+
+  observe(subject, topic, data) {
+    log.trace(`observed topic=${topic}, data=${data}, subject=${subject}`);
+    switch (topic) {
+      case "weave:engine:sync:finish":
+        if (data != "tabs") {
+          return;
+        }
+        // The tabs engine just finished syncing
+        // Set our lastTabFetch pref here so it tracks both explicit sync calls
+        // and normally scheduled ones.
+        Preferences.set("services.sync.lastTabFetch", Math.floor(Date.now() / 1000));
+        Services.obs.notifyObservers(null, TOPIC_TABS_CHANGED, null);
+        break;
+      case "weave:service:start-over":
+        // start-over needs to notify so consumers find no tabs.
+        Preferences.reset("services.sync.lastTabFetch");
+        Services.obs.notifyObservers(null, TOPIC_TABS_CHANGED, null);
+        break;
+      default:
+        break;
+    }
+  },
+
+  // Returns true if Sync is configured to Sync tabs, false otherwise
+  get isConfiguredToSyncTabs() {
+    if (!weaveXPCService.ready) {
+      log.debug("Sync isn't yet ready; assuming tab engine is enabled");
+      return true;
+    }
+
+    let engine = Weave.Service.engineManager.get("tabs");
+    return engine && engine.enabled;
+  },
+
+  get hasSyncedThisSession() {
+    let engine = Weave.Service.engineManager.get("tabs");
+    return engine && engine.hasSyncedThisSession;
+  },
+};
+
+Services.obs.addObserver(SyncedTabsInternal, "weave:engine:sync:finish", false);
+Services.obs.addObserver(SyncedTabsInternal, "weave:service:start-over", false);
+
+// The public interface.
+this.SyncedTabs = {
+  // A mock-point for tests.
+  _internal: SyncedTabsInternal,
+
+  // We make the topic for the observer notification public.
+  TOPIC_TABS_CHANGED,
+
+  // Returns true if Sync is configured to Sync tabs, false otherwise
+  get isConfiguredToSyncTabs() {
+    return this._internal.isConfiguredToSyncTabs;
+  },
+
+  // Returns true if a tab sync has completed once this session. If this
+  // returns false, then getting back no clients/tabs possibly just means we
+  // are waiting for that first sync to complete.
+  get hasSyncedThisSession() {
+    return this._internal.hasSyncedThisSession;
+  },
+
+  // Return a promise that resolves with an array of client records, each with
+  // a .tabs array. Note that part of the contract for this module is that the
+  // returned objects are not shared between invocations, so callers are free
+  // to mutate the returned objects (eg, sort, truncate) however they see fit.
+  getTabClients(query) {
+    return this._internal.getTabClients(query);
+  },
+
+  // Starts a background request to start syncing tabs. Returns a promise that
+  // resolves when the sync is complete, but there's no resolved value -
+  // callers should be listening for TOPIC_TABS_CHANGED.
+  // If |force| is true we always sync. If false, we only sync if the most
+  // recent sync wasn't "recently".
+  syncTabs(force) {
+    return this._internal.syncTabs(force);
+  },
+};
+
--- a/services/sync/modules/engines/tabs.js
+++ b/services/sync/modules/engines/tabs.js
@@ -38,16 +38,21 @@ this.TabEngine = function TabEngine(serv
   // Reset the client on every startup so that we fetch recent tabs.
   this._resetClient();
 }
 TabEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: TabStore,
   _trackerObj: TabTracker,
   _recordObj: TabSetRecord,
+  // A flag to indicate if we have synced in this session. This is to help
+  // consumers of remote tabs that may want to differentiate between "I've an
+  // empty tab list as I haven't yet synced" vs "I've an empty tab list
+  // as there really are no tabs"
+  hasSyncedThisSession: false,
 
   syncPriority: 3,
 
   getChangedIDs: function () {
     // No need for a proper timestamp (no conflict resolution needed).
     let changedIDs = {};
     if (this._tracker.modified)
       changedIDs[this.service.clientsEngine.localID] = 0;
@@ -62,16 +67,17 @@ TabEngine.prototype = {
   getClientById: function (id) {
     return this._store._remoteClients[id];
   },
 
   _resetClient: function () {
     SyncEngine.prototype._resetClient.call(this);
     this._store.wipe();
     this._tracker.modified = true;
+    this.hasSyncedThisSession = false;
   },
 
   removeClientData: function () {
     let url = this.engineURL + "/" + this.service.clientsEngine.localID;
     this.service.resource(url).delete();
   },
 
   /**
@@ -89,17 +95,22 @@ TabEngine.prototype = {
     // Skip our own record.
     // TabStore.itemExists tests only against our local client ID.
     if (this._store.itemExists(item.id)) {
       this._log.trace("Ignoring incoming tab item because of its id: " + item.id);
       return false;
     }
 
     return SyncEngine.prototype._reconcile.call(this, item);
-  }
+  },
+
+  _syncFinish() {
+    this.hasSyncedThisSession = true;
+    return SyncEngine.prototype._syncFinish.call(this);
+  },
 };
 
 
 function TabStore(name, engine) {
   Store.call(this, name, engine);
 }
 TabStore.prototype = {
   __proto__: Store.prototype,
--- a/services/sync/moz.build
+++ b/services/sync/moz.build
@@ -31,16 +31,17 @@ EXTRA_JS_MODULES['services-sync'] += [
     'modules/keys.js',
     'modules/main.js',
     'modules/policies.js',
     'modules/record.js',
     'modules/resource.js',
     'modules/rest.js',
     'modules/service.js',
     'modules/status.js',
+    'modules/SyncedTabs.jsm',
     'modules/userapi.js',
     'modules/util.js',
 ]
 
 EXTRA_PP_JS_MODULES['services-sync'] += [
     'modules/constants.js',
 ]
 
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_syncedtabs.js
@@ -0,0 +1,125 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
+ * vim:set ts=2 sw=2 sts=2 et:
+*/
+"use strict";
+
+Cu.import("resource://services-sync/main.js");
+Cu.import("resource://services-sync/SyncedTabs.jsm");
+Cu.import("resource://gre/modules/Log.jsm");
+
+Log.repository.getLogger("Sync.RemoteTabs").addAppender(new Log.DumpAppender());
+
+// A mock "Tabs" engine which the SyncedTabs module will use instead of the real
+// engine. We pass a constructor that Sync creates.
+function MockTabsEngine() {
+  this.clients = {}; // We'll set this dynamically
+}
+
+MockTabsEngine.prototype = {
+  name: "tabs",
+  enabled: true,
+
+  getAllClients() {
+    return this.clients;
+  },
+
+  getOpenURLs() {
+    return new Set();
+  },
+}
+
+// A clients engine that doesn't need to be a constructor.
+let MockClientsEngine = {
+  isMobile(guid) {
+    if (!guid.endsWith("desktop") && !guid.endsWith("mobile")) {
+      throw new Error("this module expected guids to end with 'desktop' or 'mobile'");
+    }
+    return guid.endsWith("mobile");
+  },
+}
+
+// Configure Sync with our mock tabs engine and force it to become initialized.
+Services.prefs.setCharPref("services.sync.username", "someone@somewhere.com");
+
+Weave.Service.engineManager.unregister("tabs");
+Weave.Service.engineManager.register(MockTabsEngine);
+Weave.Service.clientsEngine = MockClientsEngine;
+
+// Tell the Sync XPCOM service it is initialized.
+let weaveXPCService = Cc["@mozilla.org/weave/service;1"]
+                        .getService(Ci.nsISupports)
+                        .wrappedJSObject;
+weaveXPCService.ready = true;
+
+function configureClients(clients) {
+  // Configure the instance Sync created.
+  let engine = Weave.Service.engineManager.get("tabs");
+  // each client record is expected to have an id.
+  for (let [guid, client] in Iterator(clients)) {
+    client.id = guid;
+  }
+  engine.clients = clients;
+  // Send an observer that pretends the engine just finished a sync.
+  Services.obs.notifyObservers(null, "weave:engine:sync:finish", "tabs");
+}
+
+// The tests.
+add_task(function* test_noClients() {
+  // no clients, can't be tabs.
+  yield configureClients({});
+
+  let tabs = yield SyncedTabs.getTabClients();
+  equal(Object.keys(tabs).length, 0);
+});
+
+add_task(function* test_clientWithTabs() {
+  yield configureClients({
+    guid_desktop: {
+      clientName: "My Desktop",
+      tabs: [
+      {
+        urlHistory: ["http://foo.com/"],
+      }],
+    },
+    guid_mobile: {
+      clientName: "My Phone",
+      tabs: [],
+    }
+  });
+
+  let clients = yield SyncedTabs.getTabClients();
+  equal(clients.length, 2);
+  clients.sort((a, b) => { return a.name.localeCompare(b.name);});
+  equal(clients[0].tabs.length, 1);
+  equal(clients[0].tabs[0].url, "http://foo.com/");
+  // second client has no tabs.
+  equal(clients[1].tabs.length, 0);
+});
+
+add_task(function* test_filter() {
+  // Nothing matches.
+  yield configureClients({
+    guid_desktop: {
+      clientName: "My Desktop",
+      tabs: [
+      {
+        urlHistory: ["http://foo.com/"],
+        title: "A test page.",
+      },
+      {
+        urlHistory: ["http://bar.com/"],
+        title: "Another page.",
+      }],
+    },
+  });
+
+  let clients = yield SyncedTabs.getTabClients("foo");
+  equal(clients.length, 1);
+  equal(clients[0].tabs.length, 1);
+  equal(clients[0].tabs[0].url, "http://foo.com/");
+  // check it matches the title.
+  clients = yield SyncedTabs.getTabClients("test");
+  equal(clients.length, 1);
+  equal(clients[0].tabs.length, 1);
+  equal(clients[0].tabs[0].url, "http://foo.com/");
+});
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -176,8 +176,11 @@ skip-if = debug
 
 [test_healthreport.js]
 skip-if = ! healthreport
 
 [test_warn_on_truncated_response.js]
 
 # FxA migration
 [test_fxa_migration.js]
+
+# Synced tabs.
+[test_syncedtabs.js]