Bug 600993 - Tab sync has no reason to ever touch disk [r=mconnor]
authorPhilipp von Weitershausen <philipp@weitershausen.de>
Thu, 11 Nov 2010 11:00:41 -0800
changeset 57543 4bc2c08b44b04e32081932d6807f73819c8d26a8
parent 57542 0bd3f5510fbaea3758ce89f200ed236423fccddd
child 57544 326225ef1cab3c1ca818020e2666ec84b97dabd9
child 58406 97f781e762191c77f119e6aae01bcd93ec907aa0
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersmconnor
bugs600993
Bug 600993 - Tab sync has no reason to ever touch disk [r=mconnor]
services/sync/modules/engines/tabs.js
services/sync/tests/unit/test_tab_tracker.js
--- a/services/sync/modules/engines/tabs.js
+++ b/services/sync/modules/engines/tabs.js
@@ -15,16 +15,17 @@
  *
  * The Initial Developer of the Original Code is Mozilla.
  * Portions created by the Initial Developer are Copyright (C) 2008
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
  *  Myk Melez <myk@mozilla.org>
  *  Jono DiCarlo <jdicarlo@mozilla.com>
+ *  Philipp von Weitershausen <philipp@weitershausen.de>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
  * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
  * use your version of this file under the terms of the MPL, indicate your
@@ -64,28 +65,37 @@ function TabEngine() {
   this._resetClient();
 }
 TabEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: TabStore,
   _trackerObj: TabTracker,
   _recordObj: TabSetRecord,
 
+  getChangedIDs: function getChangedIDs() {
+    // No need for a proper timestamp (no conflict resolution needed).
+    let changedIDs = {};
+    if (this._tracker.modified)
+      changedIDs[Clients.localID] = 0;
+    return changedIDs;
+  },
+
   // API for use by Weave UI code to give user choices of tabs to open:
   getAllClients: function TabEngine_getAllClients() {
     return this._store._remoteClients;
   },
 
   getClientById: function TabEngine_getClientById(id) {
     return this._store._remoteClients[id];
   },
 
   _resetClient: function TabEngine__resetClient() {
     SyncEngine.prototype._resetClient.call(this);
     this._store.wipe();
+    this._tracker.modified = true;
   },
 
   /* 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
@@ -232,16 +242,24 @@ function TabTracker(name) {
   this.onTab = Utils.bind2(this, this.onTab);
   this._unregisterListeners = Utils.bind2(this, this._unregisterListeners);
 }
 TabTracker.prototype = {
   __proto__: Tracker.prototype,
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver]),
 
+  loadChangedIDs: function loadChangedIDs() {
+    // Don't read changed IDs from disk at start up.
+  },
+
+  clearChangedIDs: function clearChangedIDs() {
+    this.modified = false;
+  },
+
   _topics: ["pageshow", "TabOpen", "TabClose", "TabSelect"],
   _registerListenersForWindow: function registerListenersFW(window) {
     this._log.trace("Registering tab listeners in window");
     for each (let topic in this._topics) {
       window.addEventListener(topic, this.onTab, false);
     }
     window.addEventListener("unload", this._unregisterListeners, false);
   },
@@ -287,28 +305,28 @@ TabTracker.prototype = {
         aSubject.addEventListener("load", function onLoad(event) {
           aSubject.removeEventListener("load", onLoad, false);
           // Only register after the window is done loading to avoid unloads
           self._registerListenersForWindow(aSubject);
         }, false);
         break;
       case "private-browsing":
         if (aData == "enter" && !PBPrefs.get("autostart"))
-          this.clearChangedIDs();
+          this.modified = false;
     }
   },
 
   onTab: function onTab(event) {
     if (Svc.Private.privateBrowsingEnabled && !PBPrefs.get("autostart")) {
       this._log.trace("Ignoring tab event from private browsing.");
       return;
     }
 
     this._log.trace("onTab event: " + event.type);
-    this.addChangedID(Clients.localID);
+    this.modified = true;
 
     // For pageshow events, only give a partial score bump (~.1)
     let chance = .1;
 
     // For regular Tab events, do a full score bump and remember when it changed
     if (event.type != "pageshow") {
       chance = 1;
 
--- a/services/sync/tests/unit/test_tab_tracker.js
+++ b/services/sync/tests/unit/test_tab_tracker.js
@@ -38,21 +38,22 @@ function fakeSvcSession() {
       logs.push({target: target, prop: prop, value: value});
     }
   };
   return logs;
 }
 
 function run_test() {
   let engine = new TabEngine();
-  engine._store.wipe();
 
-  _("Verify we have an empty tracker to work with");
+  _("We assume that tabs have changed at startup.");
   let tracker = engine._tracker;
-  do_check_eq([id for (id in tracker.changedIDs)].length, 0);
+  do_check_true(tracker.modified);
+  do_check_true(Utils.deepEquals([id for (id in engine.getChangedIDs())],
+                                 [Clients.localID]));
 
   let logs;
 
   _("Test listeners are registered on windows");
   logs = fakeSvcWinMediator();
   Svc.Obs.notify("weave:engine:start-tracking");
   do_check_eq(logs.length, 2);
   for each (let log in logs) {
@@ -78,20 +79,33 @@ function run_test() {
     do_check_true(log.remTopics.indexOf("TabSelect") >= 0);
     do_check_true(log.remTopics.indexOf("unload") >= 0);
   }
 
   _("Test tab listener");
   logs = fakeSvcSession();
   let idx = 0;
   for each (let evttype in ["TabOpen", "TabClose", "TabSelect"]) {
+    // Pretend we just synced.
+    tracker.clearChangedIDs();
+    do_check_false(tracker.modified);
+
+    // Send a fake tab event
     tracker.onTab({type: evttype , originalTarget: evttype});
-    do_check_eq([id for (id in tracker.changedIDs)].length, 1);
+    do_check_true(tracker.modified);
+    do_check_true(Utils.deepEquals([id for (id in engine.getChangedIDs())],
+                                   [Clients.localID]));
     do_check_eq(logs.length, idx+1);
     do_check_eq(logs[idx].target, evttype);
     do_check_eq(logs[idx].prop, "weaveLastUsed");
     do_check_true(typeof logs[idx].value == "number");
     idx++;
   }
+
+  // Pretend we just synced.
+  tracker.clearChangedIDs();
+  do_check_false(tracker.modified);
+
   tracker.onTab({type: "pageshow", originalTarget: "pageshow"});
-  do_check_eq([id for (id in tracker.changedIDs)].length, 1);
+  do_check_true(Utils.deepEquals([id for (id in engine.getChangedIDs())],
+                                 [Clients.localID]));
   do_check_eq(logs.length, idx); // test that setTabValue isn't called
 }