Bug 636673 - Avoid accessing 'arguments' in code that's called a lot. r=rnewman a=blocking-fennec
authorPhilipp von Weitershausen <philipp@weitershausen.de>
Wed, 02 Mar 2011 15:27:16 -0800
changeset 63307 bf00bb0056ce
parent 63306 e88c9b6c4ebd
child 63308 ab61ff6bdf9e
child 63339 847b56a02ae8
push idunknown
push userunknown
push dateunknown
reviewersrnewman, blocking-fennec
bugs636673
Bug 636673 - Avoid accessing 'arguments' in code that's called a lot. r=rnewman a=blocking-fennec
services/sync/modules/engines.js
services/sync/modules/engines/bookmarks.js
services/sync/modules/engines/history.js
services/sync/modules/util.js
services/sync/tests/unit/test_bookmark_livemarks.js
services/sync/tests/unit/test_bookmark_smart_bookmarks.js
services/sync/tests/unit/test_bookmark_store.js
services/sync/tests/unit/test_bookmark_tracker.js
services/sync/tests/unit/test_utils_anno.js
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -369,96 +369,17 @@ Engine.prototype = {
 
   sync: function Engine_sync() {
     if (!this.enabled)
       return;
 
     if (!this._sync)
       throw "engine does not implement _sync method";
 
-    let times = {};
-    let wrapped = {};
-    // Find functions in any point of the prototype chain
-    for (let _name in this) {
-      let name = _name;
-
-      // Ignore certain constructors/functions
-      if (name.search(/^_(.+Obj|notify)$/) == 0)
-        continue;
-
-      // Only track functions but skip the constructors
-      if (typeof this[name] == "function") {
-        times[name] = [];
-        wrapped[name] = this[name];
-
-        // Wrap the original function with a start/stop timer
-        this[name] = function() {
-          let start = Date.now();
-          try {
-            return wrapped[name].apply(this, arguments);
-          }
-          finally {
-            times[name].push(Date.now() - start);
-          }
-        };
-      }
-    }
-
-    try {
-      this._notify("sync", this.name, this._sync)();
-    }
-    finally {
-      // Restore original unwrapped functionality
-      for (let [name, func] in Iterator(wrapped))
-        this[name] = func;
-
-      let stats = {};
-      for (let [name, time] in Iterator(times)) {
-        // Figure out stats on the times unless there's nothing
-        let num = time.length;
-        if (num == 0)
-          continue;
-
-        // Track the min/max/sum of the values
-        let stat = {
-          num: num,
-          sum: 0
-        };
-        time.forEach(function(val) {
-          if (stat.min == null || val < stat.min)
-            stat.min = val;
-          if (stat.max == null || val > stat.max)
-            stat.max = val;
-          stat.sum += val;
-        });
-
-        stat.avg = Number((stat.sum / num).toFixed(2));
-        stats[name] = stat;
-      }
-
-      stats.toString = function() {
-        let sums = [];
-        for (let [name, stat] in Iterator(this))
-          if (stat.sum != null)
-            sums.push(name.replace(/^_/, "") + " " + stat.sum);
-
-        // Order certain functions first before any other random ones
-        let nameOrder = ["sync", "processIncoming", "uploadOutgoing",
-          "syncStartup", "syncFinish"];
-        let getPos = function(str) {
-          let pos = nameOrder.indexOf(str.split(" ")[0]);
-          return pos != -1 ? pos : Infinity;
-        };
-        let order = function(a, b) getPos(a) > getPos(b);
-
-        return "Total (ms): " + sums.sort(order).join(", ");
-      };
-
-      this._log.debug(stats);
-    }
+    this._notify("sync", this.name, this._sync)();
   },
 
   /**
    * Get rid of any local meta-data
    */
   resetClient: function Engine_resetClient() {
     if (!this._resetClient)
       throw "engine does not implement _resetClient method";
@@ -697,67 +618,70 @@ SyncEngine.prototype = {
       if (failed.length) {
         this.toFetch = Utils.arrayUnion(failed, this.toFetch);
         count.failed += failed.length;
         this._log.debug("Records that failed to apply: " + failed);
         failed = [];
       }
     }
 
-    newitems.recordHandler = Utils.bind2(this, function(item) {
+    // Not binding this method to 'this' for performance reasons. It gets
+    // called for every incoming record.
+    let self = this;
+    newitems.recordHandler = function(item) {
       // Grab a later last modified if possible
-      if (this.lastModified == null || item.modified > this.lastModified)
-        this.lastModified = item.modified;
+      if (self.lastModified == null || item.modified > self.lastModified)
+        self.lastModified = item.modified;
 
       // Track the collection for the WBO.
-      item.collection = this.name;
+      item.collection = self.name;
       
       // Remember which records were processed
       handled.push(item.id);
 
       try {
         try {
           item.decrypt();
         } catch (ex if (Utils.isHMACMismatch(ex) &&
-                        this.handleHMACMismatch(item))) {
+                        self.handleHMACMismatch(item))) {
           // Let's try handling it.
           // If the callback returns true, try decrypting again, because
           // we've got new keys.
-          this._log.info("Trying decrypt again...");
+          self._log.info("Trying decrypt again...");
           item.decrypt();
         }       
       } catch (ex) {
-        this._log.warn("Error decrypting record: " + Utils.exceptionStr(ex));
+        self._log.warn("Error decrypting record: " + Utils.exceptionStr(ex));
         failed.push(item.id);
         return;
       }
 
       let shouldApply;
       try {
-        shouldApply = this._reconcile(item);
+        shouldApply = self._reconcile(item);
       } catch (ex) {
-        this._log.warn("Failed to reconcile incoming record " + item.id);
-        this._log.warn("Encountered exception: " + Utils.exceptionStr(ex));
+        self._log.warn("Failed to reconcile incoming record " + item.id);
+        self._log.warn("Encountered exception: " + Utils.exceptionStr(ex));
         failed.push(item.id);
         return;
       }
 
       if (shouldApply) {
         count.applied++;
         applyBatch.push(item);
       } else {
         count.reconciled++;
-        this._log.trace("Skipping reconciled incoming item " + item.id);
+        self._log.trace("Skipping reconciled incoming item " + item.id);
       }
 
-      if (applyBatch.length == this.applyIncomingBatchSize) {
-        doApplyBatch.call(this);
+      if (applyBatch.length == self.applyIncomingBatchSize) {
+        doApplyBatch.call(self);
       }
-      this._sleep(0);
-    });
+      self._sleep(0);
+    };
 
     // Only bother getting data from the server if there's new things
     if (this.lastModified == null || this.lastModified > this.lastSync) {
       let resp = newitems.get();
       doApplyBatchAndPersistFailed.call(this);
       if (!resp.success) {
         resp.failureCode = ENGINE_DOWNLOAD_FAIL;
         throw resp;
@@ -924,16 +848,17 @@ SyncEngine.prototype = {
       this._handleDupe(item, dupeId);
 
     // Apply the incoming item (now that the dupe is the right id)
     return true;
   },
 
   // Upload outgoing records
   _uploadOutgoing: function SyncEngine__uploadOutgoing() {
+    this._log.trace("Uploading local changes to server.");
     if (this._modifiedIDs.length) {
       this._log.trace("Preparing " + this._modifiedIDs.length +
                       " outgoing records");
 
       // collection we'll upload
       let up = new Collection(this.engineURL);
       let count = 0;
 
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -42,21 +42,33 @@
 const EXPORTED_SYMBOLS = ['BookmarksEngine', "PlacesItem", "Bookmark",
                           "BookmarkFolder", "BookmarkMicsum", "BookmarkQuery",
                           "Livemark", "BookmarkSeparator"];
 
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 
-const GUID_ANNO = "sync/guid";
-const MOBILE_ANNO = "mobile/bookmarksRoot";
-const PARENT_ANNO = "sync/parent";
+const ALLBOOKMARKS_ANNO    = "AllBookmarks";
+const DESCRIPTION_ANNO     = "bookmarkProperties/description";
+const SIDEBAR_ANNO         = "bookmarkProperties/loadInSidebar";
+const STATICTITLE_ANNO     = "bookmarks/staticTitle";
+const FEEDURI_ANNO         = "livemark/feedURI";
+const SITEURI_ANNO         = "livemark/siteURI";
+const GENERATORURI_ANNO    = "microsummary/generatorURI";
+const MOBILEROOT_ANNO      = "mobile/bookmarksRoot";
+const MOBILE_ANNO          = "MobileBookmarks";
+const EXCLUDEBACKUP_ANNO   = "places/excludeFromBackup";
+const SMART_BOOKMARKS_ANNO = "Places/SmartBookmark";
+const GUID_ANNO            = "sync/guid";
+const PARENT_ANNO          = "sync/parent";
+const ANNOS_TO_TRACK = [DESCRIPTION_ANNO, SIDEBAR_ANNO, STATICTITLE_ANNO,
+                       FEEDURI_ANNO, SITEURI_ANNO, GENERATORURI_ANNO];
+
 const SERVICE_NOT_SUPPORTED = "Service not supported on this platform";
-const SMART_BOOKMARKS_ANNO = "Places/SmartBookmark";
 const FOLDER_SORTINDEX = 1000000;
 
 try {
   Cu.import("resource://gre/modules/PlacesUtils.jsm");
 }
 catch(ex) {
   Cu.import("resource://gre/modules/utils.js");
 }
@@ -67,19 +79,19 @@ Cu.import("resource://services-sync/util
 
 Cu.import("resource://services-sync/main.js");      // For access to Service.
 
 function PlacesItem(collection, id, type) {
   CryptoWrapper.call(this, collection, id);
   this.type = type || "item";
 }
 PlacesItem.prototype = {
-  decrypt: function PlacesItem_decrypt() {
+  decrypt: function PlacesItem_decrypt(keyBundle) {
     // Do the normal CryptoWrapper decrypt, but change types before returning
-    let clear = CryptoWrapper.prototype.decrypt.apply(this, arguments);
+    let clear = CryptoWrapper.prototype.decrypt.call(this, keyBundle);
 
     // Convert the abstract places item to the actual object type
     if (!this.deleted)
       this.__proto__ = this.getTypeObject(this.type).prototype;
 
     return clear;
   },
 
@@ -182,30 +194,30 @@ function archiveBookmarks() {
   catch(ex) {}
 }
 
 let kSpecialIds = {
 
   // Special IDs. Note that mobile can attempt to create a record on
   // dereference; special accessors are provided to prevent recursion within
   // observers.
-  get guids()
-    ["menu", "places", "tags", "toolbar", "unfiled", "mobile"],
+  guids: ["menu", "places", "tags", "toolbar", "unfiled", "mobile"],
 
   // Create the special mobile folder to store mobile bookmarks.
   createMobileRoot: function createMobileRoot() {
     let root = Svc.Bookmark.placesRoot;
     let mRoot = Svc.Bookmark.createFolder(root, "mobile", -1);
-    Utils.anno(mRoot, MOBILE_ANNO, 1);
+    Svc.Annos.setItemAnnotation(mRoot, MOBILEROOT_ANNO, 1, 0,
+                                Svc.Annos.EXPIRE_NEVER);
     return mRoot;
   },
 
   findMobileRoot: function findMobileRoot(create) {
     // Use the (one) mobile root if it already exists.
-    let root = Svc.Annos.getItemsWithAnnotation(MOBILE_ANNO, {});
+    let root = Svc.Annos.getItemsWithAnnotation(MOBILEROOT_ANNO, {});
     if (root.length != 0)
       return root[0];
 
     if (create)
       return this.createMobileRoot();
 
     return null;
   },
@@ -267,17 +279,17 @@ BookmarksEngine.prototype = {
         let key;
         let id = this._store.idForGUID(guid);
         switch (Svc.Bookmark.getItemType(id)) {
           case Svc.Bookmark.TYPE_BOOKMARK:
 
             // Smart bookmarks map to their annotation value.
             let queryId;
             try {
-              queryId = Utils.anno(id, SMART_BOOKMARKS_ANNO);
+              queryId = Svc.Annos.getItemAnnotation(id, SMART_BOOKMARKS_ANNO);
             } catch(ex) {}
             
             if (queryId)
               key = "q" + queryId;
             else
               key = "b" + Svc.Bookmark.getBookmarkURI(id).spec + ":" +
                     Svc.Bookmark.getItemTitle(id);
             break;
@@ -539,17 +551,17 @@ BookmarksStore.prototype = {
         return;
       }
     }
   },
   
   applyIncoming: function BStore_applyIncoming(record) {
     // Don't bother with pre and post-processing for deletions.
     if (record.deleted) {
-      Store.prototype.applyIncoming.apply(this, arguments);
+      Store.prototype.applyIncoming.call(this, record);
       return;
     }
 
     // For special folders we're only interested in child ordering.
     if ((record.id in kSpecialIds) && record.children) {
       this._log.debug("Processing special node: " + record.id);
       // Reorder children later
       this._childrenToOrder[record.id] = record.children;
@@ -572,41 +584,43 @@ BookmarksStore.prototype = {
       record._orphan = false;
     } else {
       this._log.trace("Record " + record.id +
                       " is an orphan: could not find parent " + parentGUID);
       record._orphan = true;
     }
 
     // Do the normal processing of incoming records
-    Store.prototype.applyIncoming.apply(this, arguments);
+    Store.prototype.applyIncoming.call(this, record);
 
     // Do some post-processing if we have an item
     let itemId = this.idForGUID(record.id);
     if (itemId > 0) {
       // Move any children that are looking for this folder as a parent
       if (record.type == "folder") {
         this._reparentOrphans(itemId);
         // Reorder children later
         if (record.children)
           this._childrenToOrder[record.id] = record.children;
       }
 
       // Create an annotation to remember that it needs reparenting.
-      if (record._orphan)
-        Utils.anno(itemId, PARENT_ANNO, parentGUID);
+      if (record._orphan) {
+        Svc.Annos.setItemAnnotation(itemId, PARENT_ANNO, parentGUID, 0,
+                                    Svc.Annos.EXPIRE_NEVER);
+      }
     }
   },
 
   /**
    * Find all ids of items that have a given value for an annotation
    */
   _findAnnoItems: function BStore__findAnnoItems(anno, val) {
     return Svc.Annos.getItemsWithAnnotation(anno, {}).filter(function(id)
-      Utils.anno(id, anno) == val);
+      Svc.Annos.getItemAnnotation(id, anno) == val);
   },
 
   /**
    * For the provided parent item, attach its children to it
    */
   _reparentOrphans: function _reparentOrphans(parentId) {
     // Find orphans and reunite with this folder parent
     let parentGUID = this.GUIDForId(parentId);
@@ -678,57 +692,67 @@ BookmarksStore.prototype = {
     let newId;
     switch (record.type) {
     case "bookmark":
     case "query":
     case "microsummary": {
       let uri = Utils.makeURI(record.bmkUri);
       newId = this._bms.insertBookmark(record._parent, uri,
                                        Svc.Bookmark.DEFAULT_INDEX, record.title);
-      this._log.debug(["created bookmark", newId, "under", record._parent,
-                       "as", record.title, record.bmkUri].join(" "));
+      this._log.debug("created bookmark " + newId + " under " + record._parent
+                      + " as " + record.title + " " + record.bmkUri);
 
       // Smart bookmark annotations are strings.
       if (record.queryId) {
-        Utils.anno(newId, SMART_BOOKMARKS_ANNO, record.queryId);
+        Svc.Annos.setItemAnnotation(newId, SMART_BOOKMARKS_ANNO, record.queryId,
+                                    0, Svc.Annos.EXPIRE_NEVER);
       }
 
       if (Utils.isArray(record.tags)) {
         this._tagURI(uri, record.tags);
       }
       this._bms.setKeywordForBookmark(newId, record.keyword);
-      if (record.description)
-        Utils.anno(newId, "bookmarkProperties/description", record.description);
+      if (record.description) {
+        Svc.Annos.setItemAnnotation(newId, DESCRIPTION_ANNO,
+                                    record.description, 0,
+                                    Svc.Annos.EXPIRE_NEVER);
+      }
 
-      if (record.loadInSidebar)
-        Utils.anno(newId, "bookmarkProperties/loadInSidebar", true);
+      if (record.loadInSidebar) {
+        Svc.Annos.setItemAnnotation(newId, SIDEBAR_ANNO, true, 0,
+                                    Svc.Annos.EXPIRE_NEVER);
+      }
 
       if (record.type == "microsummary") {
         this._log.debug("   \-> is a microsummary");
-        Utils.anno(newId, "bookmarks/staticTitle", record.staticTitle || "");
+        Svc.Annos.setItemAnnotation(newId, STATICTITLE_ANNO,
+                                    record.staticTitle || "", 0,
+                                    Svc.Annos.EXPIRE_NEVER);
         let genURI = Utils.makeURI(record.generatorUri);
         if (this._ms) {
           try {
             let micsum = this._ms.createMicrosummary(uri, genURI);
             this._ms.setMicrosummary(newId, micsum);
           }
           catch(ex) { /* ignore "missing local generator" exceptions */ }
         }
         else
           this._log.warn("Can't create microsummary -- not supported.");
       }
     } break;
     case "folder":
       newId = this._bms.createFolder(record._parent, record.title,
                                      Svc.Bookmark.DEFAULT_INDEX);
-      this._log.debug(["created folder", newId, "under", record._parent,
-                       "as", record.title].join(" "));
+      this._log.debug("created folder " + newId + " under " + record._parent
+                      + " as " + record.title);
 
-      if (record.description)
-        Utils.anno(newId, "bookmarkProperties/description", record.description);
+      if (record.description) {
+        Svc.Annos.setItemAnnotation(newId, DESCRIPTION_ANNO, record.description,
+                                    0, Svc.Annos.EXPIRE_NEVER);
+      }
 
       // record.children will be dealt with in _orderChildren.
       break;
     case "livemark":
       let siteURI = null;
       if (!record.feedUri) {
         this._log.debug("No feed URI: skipping livemark record " + record.id);
         return;
@@ -749,18 +773,17 @@ BookmarksStore.prototype = {
                                                 Svc.Bookmark.DEFAULT_INDEX);
       this._log.debug("Created livemark " + newId + " under " + record._parent +
                       " as " + record.title + ", " + record.siteUri + ", " + 
                       record.feedUri + ", GUID " + record.id);
       break;
     case "separator":
       newId = this._bms.insertSeparator(record._parent,
                                         Svc.Bookmark.DEFAULT_INDEX);
-      this._log.debug(["created separator", newId, "under", record._parent]
-                      .join(" "));
+      this._log.debug("created separator " + newId + " under " + record._parent);
       break;
     case "item":
       this._log.debug(" -> got a generic places item.. do nothing?");
       return;
     default:
       this._log.error("_create: Unknown item type: " + record.type);
       return;
     }
@@ -852,40 +875,44 @@ BookmarksStore.prototype = {
           this._tagURI(this._bms.getBookmarkURI(itemId), val);
         }
         break;
       case "keyword":
         this._bms.setKeywordForBookmark(itemId, val);
         break;
       case "description":
         val = val || "";
-        Utils.anno(itemId, "bookmarkProperties/description", val);
+        Svc.Annos.setItemAnnotation(itemId, DESCRIPTION_ANNO, val, 0,
+                                    Svc.Annos.EXPIRE_NEVER);
         break;
       case "loadInSidebar":
-        if (val)
-          Utils.anno(itemId, "bookmarkProperties/loadInSidebar", true);
-        else
-          Svc.Annos.removeItemAnnotation(itemId, "bookmarkProperties/loadInSidebar");
+        if (val) {
+          Svc.Annos.setItemAnnotation(itemId, SIDEBAR_ANNO, true, 0,
+                                      Svc.Annos.EXPIRE_NEVER);
+        } else {
+          Svc.Annos.removeItemAnnotation(itemId, SIDEBAR_ANNO);
+        }
         break;
       case "generatorUri": {
         try {
           let micsumURI = this._bms.getBookmarkURI(itemId);
           let genURI = Utils.makeURI(val);
           if (this._ms == SERVICE_NOT_SUPPORTED)
             this._log.warn("Can't create microsummary -- not supported.");
           else {
             let micsum = this._ms.createMicrosummary(micsumURI, genURI);
             this._ms.setMicrosummary(itemId, micsum);
           }
         } catch (e) {
           this._log.debug("Could not set microsummary generator URI: " + e);
         }
       } break;
       case "queryId":
-        Utils.anno(itemId, SMART_BOOKMARKS_ANNO, val);
+        Svc.Annos.setItemAnnotation(itemId, SMART_BOOKMARKS_ANNO, val, 0,
+                                    Svc.Annos.EXPIRE_NEVER);
         break;
       case "siteUri":
         this._ls.setSiteURI(itemId, Utils.makeURI(val));
         break;
       case "feedUri":
         this._ls.setFeedURI(itemId, Utils.makeURI(val));
         break;
       }
@@ -943,29 +970,29 @@ BookmarksStore.prototype = {
     } catch(e) {
       this._log.warn("Could not parse URI \"" + uri + "\": " + e);
     }
     return this._ts.getTagsForURI(uri, {});
   },
 
   _getDescription: function BStore__getDescription(id) {
     try {
-      return Utils.anno(id, "bookmarkProperties/description");
+      return Svc.Annos.getItemAnnotation(id, DESCRIPTION_ANNO);
     } catch (e) {
       return null;
     }
   },
 
   _isLoadInSidebar: function BStore__isLoadInSidebar(id) {
-    return Svc.Annos.itemHasAnnotation(id, "bookmarkProperties/loadInSidebar");
+    return Svc.Annos.itemHasAnnotation(id, SIDEBAR_ANNO);
   },
 
   _getStaticTitle: function BStore__getStaticTitle(id) {
     try {
-      return Utils.anno(id, "bookmarks/staticTitle");
+      return Svc.Annos.getItemAnnotation(id, STATICTITLE_ANNO);
     } catch (e) {
       return "";
     }
   },
 
   __childGUIDsStm: null,
   get _childGUIDsStm() {
     if (this.__childGUIDsStm) {
@@ -1040,17 +1067,17 @@ BookmarksStore.prototype = {
               record.folderName = this._bms.getItemTitle(folder);
               this._log.trace("query id: " + folder + " = " + record.folderName);
             }
           }
           catch(ex) {}
           
           // Persist the Smart Bookmark anno, if found.
           try {
-            let anno = Utils.anno(placeId, SMART_BOOKMARKS_ANNO);
+            let anno = Svc.Annos.getItemAnnotation(placeId, SMART_BOOKMARKS_ANNO);
             if (anno != null) {
               this._log.trace("query anno: " + SMART_BOOKMARKS_ANNO +
                               " = " + anno);
               record.queryId = anno;
             }
           }
           catch(ex) {}
         }
@@ -1194,17 +1221,17 @@ BookmarksStore.prototype = {
     } else {
       stmt = false;
     }
     return this.__setGUIDStm = stmt;
   },
 
   // Some helper functions to handle GUIDs
   _setGUID: function _setGUID(id, guid) {
-    if (arguments.length == 1)
+    if (!guid)
       guid = Utils.makeGUID();
 
     // If we can, set the GUID on moz_bookmarks and do not do any other work.
     let (stmt = this._setGUIDStm) {
       if (stmt) {
         stmt.params.guid = guid;
         stmt.params.item_id = id;
         Utils.queryAsync(stmt);
@@ -1329,18 +1356,18 @@ BookmarksStore.prototype = {
     if (kSpecialIds.isSpecialGUID(guid))
       return kSpecialIds.specialIdForGUID(guid, !noCreate);
 
     let stmt = this._idForGUIDStm;
     // guid might be a String object rather than a string.
     stmt.params.guid = guid.toString();
 
     let results = Utils.queryAsync(stmt, this._idForGUIDCols);
-    this._log.trace("Rows matching GUID " + guid + ": " +
-                    results.map(function(x) x.item_id));
+    this._log.trace("Number of rows matching GUID " + guid + ": "
+                    + results.length);
     
     // Here's the one we care about: the first.
     let result = results[0];
     
     if (!result)
       return -1;
     
     if (!this._haveGUIDColumn) {
@@ -1565,17 +1592,17 @@ BookmarksTracker.prototype = {
     // Ignore unconditionally if the engine tells us to
     if (this.ignoreAll)
       return true;
 
     // Ensure that the mobile bookmarks query is correct in the UI
     this._ensureMobileQuery();
 
     // Make sure to remove items that have the exclude annotation
-    if (Svc.Annos.itemHasAnnotation(itemId, "places/excludeFromBackup")) {
+    if (Svc.Annos.itemHasAnnotation(itemId, EXCLUDEBACKUP_ANNO)) {
       this.removeChangedID(this._GUIDForId(itemId));
       return true;
     }
 
     // Get the folder id if we weren't given one
     if (folder == null)
       folder = this._bms.getFolderIdForItem(itemId);
 
@@ -1609,40 +1636,42 @@ BookmarksTracker.prototype = {
     this._addId(itemId);
     let folder = Svc.Bookmark.getFolderIdForItem(itemId);
     this._addId(folder);
   },
 
   _ensureMobileQuery: function _ensureMobileQuery() {
     let anno = "PlacesOrganizer/OrganizerQuery";
     let find = function(val) Svc.Annos.getItemsWithAnnotation(anno, {}).filter(
-      function(id) Utils.anno(id, anno) == val);
+      function(id) Svc.Annos.getItemAnnotation(id, anno) == val);
 
     // Don't continue if the Library isn't ready
-    let all = find("AllBookmarks");
+    let all = find(ALLBOOKMARKS_ANNO);
     if (all.length == 0)
       return;
 
     // Disable handling of notifications while changing the mobile query
     this.ignoreAll = true;
 
-    let mobile = find("MobileBookmarks");
+    let mobile = find(MOBILE_ANNO);
     let queryURI = Utils.makeURI("place:folder=" + kSpecialIds.mobile);
     let title = Str.sync.get("mobile.label");
 
     // Don't add OR do remove the mobile bookmarks if there's nothing
     if (Svc.Bookmark.getIdForItemAt(kSpecialIds.mobile, 0) == -1) {
       if (mobile.length != 0)
         Svc.Bookmark.removeItem(mobile[0]);
     }
     // Add the mobile bookmarks query if it doesn't exist
     else if (mobile.length == 0) {
       let query = Svc.Bookmark.insertBookmark(all[0], queryURI, -1, title);
-      Utils.anno(query, anno, "MobileBookmarks");
-      Utils.anno(query, "places/excludeFromBackup", 1);
+      Svc.Annos.setItemAnnotation(query, anno, MOBILE_ANNO, 0,
+                                  Svc.Annos.EXPIRE_NEVER);
+      Svc.Annos.setItemAnnotation(query, EXCLUDEBACKUP_ANNO, 1, 0,
+                                  Svc.Annos.EXPIRE_NEVER);
     }
     // Make sure the existing title is correct
     else if (Svc.Bookmark.getItemTitle(mobile[0]) != title)
       Svc.Bookmark.setItemTitle(mobile[0], title);
 
     this.ignoreAll = false;
   },
 
@@ -1656,20 +1685,17 @@ BookmarksTracker.prototype = {
       this._log.trace("onItemChanged for " + GUID_ANNO +
                       ": probably needs a new one.");
       this._idForGUID(this._GUIDForId(itemId));
       this._addId(itemId);
       return;
     }
 
     // ignore annotations except for the ones that we sync
-    let annos = ["bookmarkProperties/description",
-      "bookmarkProperties/loadInSidebar", "bookmarks/staticTitle",
-      "livemark/feedURI", "livemark/siteURI", "microsummary/generatorURI"];
-    if (isAnno && annos.indexOf(property) == -1)
+    if (isAnno && ANNOS_TO_TRACK.indexOf(property) == -1)
       return;
 
     // Ignore favicon changes to avoid unnecessary churn
     if (property == "favicon")
       return;
 
     this._log.trace("onItemChanged: " + itemId +
                     (", " + property + (isAnno? " (anno)" : "")) +
--- a/services/sync/modules/engines/history.js
+++ b/services/sync/modules/engines/history.js
@@ -218,17 +218,17 @@ HistoryStore.prototype = {
 
     return this.__setGUIDStm = stmt;
   },
 
   // Some helper functions to handle GUIDs
   setGUID: function setGUID(uri, guid) {
     uri = uri.spec ? uri.spec : uri;
 
-    if (arguments.length == 1)
+    if (!guid)
       guid = Utils.makeGUID();
 
     // If we can, set the GUID on moz_places and do not do any other work.
     let (stmt = this._setGUIDStm) {
       if (stmt) {
         stmt.params.guid = guid;
         stmt.params.page_url = uri;
         Utils.queryAsync(stmt);
--- a/services/sync/modules/util.js
+++ b/services/sync/modules/util.js
@@ -305,43 +305,16 @@ let Utils = {
     return Utils.encodeBase64url(Utils.generateRandomBytes(9));
   },
 
   _base64url_regex: /^[-abcdefghijklmnopqrstuvwxyz0123456789_]{12}$/i,
   checkGUID: function checkGUID(guid) {
     return !!guid && this._base64url_regex.test(guid);
   },
 
-  anno: function anno(id, anno, val, expire) {
-    // Figure out if we have a bookmark or page
-    let annoFunc = (typeof id == "number" ? "Item" : "Page") + "Annotation";
-
-    // Convert to a nsIURI if necessary
-    if (typeof id == "string")
-      id = Utils.makeURI(id);
-
-    if (id == null)
-      throw "Null id for anno! (invalid uri)";
-
-    switch (arguments.length) {
-      case 2:
-        // Get the annotation with 2 args
-        return Svc.Annos["get" + annoFunc](id, anno);
-      case 3:
-        expire = "NEVER";
-        // Fallthrough!
-      case 4:
-        // Convert to actual EXPIRE value
-        expire = Svc.Annos["EXPIRE_" + expire];
-
-        // Set the annotation with 3 or 4 args
-        return Svc.Annos["set" + annoFunc](id, anno, val, 0, expire);
-    }
-  },
-
   ensureOneOpen: let (windows = {}) function ensureOneOpen(window) {
     // Close the other window if it exists
     let url = window.location.href;
     let other = windows[url];
     if (other != null)
       other.close();
 
     // Save the new window for future closure
--- a/services/sync/tests/unit/test_bookmark_livemarks.js
+++ b/services/sync/tests/unit/test_bookmark_livemarks.js
@@ -83,17 +83,18 @@ function test_livemark_descriptions(next
   // Attempt to provoke an error by messing around with the description.
   record.description = null;
   doRecord(makeLivemark(record));
   record.description = "";
   doRecord(makeLivemark(record));
   
   // Attempt to provoke an error by adding a bad description anno.
   let id = store.idForGUID(record.id);
-  Utils.anno(id, DESCRIPTION_ANNO, "");
+  Svc.Annos.setItemAnnotation(id, DESCRIPTION_ANNO, "", 0,
+                              Svc.Annos.EXPIRE_NEVER);
 
   next();
 }
 
 function test_livemark_invalid(next) {
   _("Livemarks considered invalid by nsLivemarkService are skipped.");
   
   _("Parent is 0, which is invalid. Will be set to unfiled.");
--- a/services/sync/tests/unit/test_bookmark_smart_bookmarks.js
+++ b/services/sync/tests/unit/test_bookmark_smart_bookmarks.js
@@ -78,17 +78,18 @@ function test_annotation_uploaded() {
                   "&maxResults=10");
   let title = "Most Visited";
 
   let mostVisitedID = newSmartBookmark(parent, uri, -1, title, "MostVisited");
 
   _("New item ID: " + mostVisitedID);
   do_check_true(!!mostVisitedID);
 
-  let annoValue = Utils.anno(mostVisitedID, SMART_BOOKMARKS_ANNO);
+  let annoValue = Svc.Annos.getItemAnnotation(mostVisitedID,
+                                              SMART_BOOKMARKS_ANNO);
   _("Anno: " + annoValue);
   do_check_eq("MostVisited", annoValue);
 
   let guid = store.GUIDForId(mostVisitedID);
   _("GUID: " + guid);
   do_check_true(!!guid);
 
   _("Create record object and verify that it's sane.");
@@ -152,26 +153,27 @@ function test_annotation_uploaded() {
     engine.sync();
 
     _("Verify that the Places DB now has an annotated bookmark.");
     _("Our count has increased again.");
     do_check_eq(smartBookmarkCount(), startCount + 1);
 
     _("Find by GUID and verify that it's annotated.");
     let newID = store.idForGUID(serverGUID);
-    let newAnnoValue = Utils.anno(newID, SMART_BOOKMARKS_ANNO);
+    let newAnnoValue = Svc.Annos.getItemAnnotation(newID, SMART_BOOKMARKS_ANNO);
     do_check_eq(newAnnoValue, "MostVisited");
     do_check_eq(Svc.Bookmark.getBookmarkURI(newID).spec, uri.spec);
     
     _("Test updating.");
     let newRecord = store.createRecord(serverGUID);
     do_check_eq(newRecord.queryId, newAnnoValue);
     newRecord.queryId = "LeastVisited";
     store.update(newRecord);
-    do_check_eq("LeastVisited", Utils.anno(newID, SMART_BOOKMARKS_ANNO));
+    do_check_eq("LeastVisited",
+                Svc.Annos.getItemAnnotation(newID, SMART_BOOKMARKS_ANNO));
     
 
   } finally {
     // Clean up.
     store.wipe();
     server.stop(do_test_finished);
     Svc.Prefs.resetBranch("");
     Records.clearCache();
--- a/services/sync/tests/unit/test_bookmark_store.js
+++ b/services/sync/tests/unit/test_bookmark_store.js
@@ -30,17 +30,17 @@ function test_bookmark_create() {
     store.applyIncoming(fxrecord);
 
     _("Verify it has been created correctly.");
     let id = store.idForGUID(fxrecord.id);
     do_check_eq(store.GUIDForId(id), fxrecord.id);
     do_check_eq(Svc.Bookmark.getItemType(id), Svc.Bookmark.TYPE_BOOKMARK);
     do_check_true(Svc.Bookmark.getBookmarkURI(id).equals(fxuri));
     do_check_eq(Svc.Bookmark.getItemTitle(id), fxrecord.title);
-    do_check_eq(Utils.anno(id, "bookmarkProperties/description"),
+    do_check_eq(Svc.Annos.getItemAnnotation(id, "bookmarkProperties/description"),
                 fxrecord.description);
     do_check_eq(Svc.Bookmark.getFolderIdForItem(id),
                 Svc.Bookmark.toolbarFolder);
     do_check_eq(Svc.Bookmark.getKeywordForBookmark(id), fxrecord.keyword);
 
     _("Have the store create a new record object. Verify that it has the same data.");
     let newrecord = store.createRecord(fxrecord.id);
     do_check_true(newrecord instanceof Bookmark);
@@ -64,17 +64,17 @@ function test_bookmark_create() {
     _("Verify it has been created correctly.");
     id = store.idForGUID(tbrecord.id);
     do_check_eq(store.GUIDForId(id), tbrecord.id);
     do_check_eq(Svc.Bookmark.getItemType(id), Svc.Bookmark.TYPE_BOOKMARK);
     do_check_true(Svc.Bookmark.getBookmarkURI(id).equals(tburi));
     do_check_eq(Svc.Bookmark.getItemTitle(id), null);
     let error;
     try {
-      Utils.anno(id, "bookmarkProperties/description");
+      Svc.Annos.getItemAnnotation(id, "bookmarkProperties/description");
     } catch(ex) {
       error = ex;
     }
     do_check_eq(error.result, Cr.NS_ERROR_NOT_AVAILABLE);
     do_check_eq(Svc.Bookmark.getFolderIdForItem(id),
                 Svc.Bookmark.toolbarFolder);
     do_check_eq(Svc.Bookmark.getKeywordForBookmark(id), null);
   } finally {
@@ -84,30 +84,31 @@ function test_bookmark_create() {
 }
 
 function test_bookmark_update() {
   try {
     _("Create a bookmark whose values we'll change.");
     let bmk1_id = Svc.Bookmark.insertBookmark(
       Svc.Bookmark.toolbarFolder, fxuri, Svc.Bookmark.DEFAULT_INDEX,
       "Get Firefox!");
-    Utils.anno(bmk1_id, "bookmarkProperties/description", "Firefox is awesome.");
+    Svc.Annos.setItemAnnotation(bmk1_id, "bookmarkProperties/description",
+                                "Firefox is awesome.", 0, Svc.Annos.EXPIRE_NEVER);
     Svc.Bookmark.setKeywordForBookmark(bmk1_id, "firefox");
     let bmk1_guid = store.GUIDForId(bmk1_id);
 
     _("Update the record with some null values.");
     let record = store.createRecord(bmk1_guid);
     record.title = null;
     record.description = null;
     record.keyword = null;
     record.tags = null;
     store.applyIncoming(record);
 
     _("Verify that the values have been cleared.");
-    do_check_eq(Utils.anno(bmk1_id, "bookmarkProperties/description"), "");
+    do_check_eq(Svc.Annos.getItemAnnotation(bmk1_id, "bookmarkProperties/description"), "");
     do_check_eq(Svc.Bookmark.getItemTitle(bmk1_id), "");
     do_check_eq(Svc.Bookmark.getKeywordForBookmark(bmk1_id), null);
   } finally {
     _("Clean up.");
     store.wipe();
   }
 }
 
@@ -301,52 +302,54 @@ function test_orphan() {
     _("Add a new bookmark locally.");
     let bmk1_id = Svc.Bookmark.insertBookmark(
       Svc.Bookmark.toolbarFolder, fxuri, Svc.Bookmark.DEFAULT_INDEX,
       "Get Firefox!");
     let bmk1_guid = store.GUIDForId(bmk1_id);
     do_check_eq(Svc.Bookmark.getFolderIdForItem(bmk1_id), Svc.Bookmark.toolbarFolder);
     let error;
     try {
-      Utils.anno(bmk1_id, PARENT_ANNO);
+      Svc.Annos.getItemAnnotation(bmk1_id, PARENT_ANNO);
     } catch(ex) {
       error = ex;
     }
     do_check_eq(error.result, Cr.NS_ERROR_NOT_AVAILABLE);
 
     _("Apply a server record that is the same but refers to non-existent folder.");
     let record = store.createRecord(bmk1_guid);
     record.parentid = "non-existent";
     store.applyIncoming(record);
 
     _("Verify that bookmark has been flagged as orphan, has not moved.");
     do_check_eq(Svc.Bookmark.getFolderIdForItem(bmk1_id), Svc.Bookmark.toolbarFolder);
-    do_check_eq(Utils.anno(bmk1_id, PARENT_ANNO), "non-existent");
+    do_check_eq(Svc.Annos.getItemAnnotation(bmk1_id, PARENT_ANNO), "non-existent");
 
   } finally {
     _("Clean up.");
     store.wipe();
   }
 }
 
 function test_reparentOrphans() {
   try {
     let folder1_id = Svc.Bookmark.createFolder(
       Svc.Bookmark.toolbarFolder, "Folder1", 0);
     let folder1_guid = store.GUIDForId(folder1_id);
 
     _("Create a bogus orphan record and write the record back to the store to trigger _reparentOrphans.");
-    Utils.anno(folder1_id, PARENT_ANNO, folder1_guid);
+    Svc.Annos.setItemAnnotation(folder1_id, PARENT_ANNO, folder1_guid, 0,
+                                Svc.Annos.EXPIRE_NEVER);
     let record = store.createRecord(folder1_guid);
     record.title = "New title for Folder 1";
     store._childrenToOrder = {};
     store.applyIncoming(record);
 
     _("Verify that is has been marked as an orphan even though it couldn't be moved into itself.");
-    do_check_eq(Utils.anno(folder1_id, PARENT_ANNO), folder1_guid);
+    do_check_eq(Svc.Annos.getItemAnnotation(folder1_id, PARENT_ANNO),
+                folder1_guid);
 
   } finally {
     _("Clean up.");
     store.wipe();
   }
 }
 
 // Copying a bookmark in the UI also copies its annotations, including the GUID
@@ -373,17 +376,17 @@ function test_copying_avoid_duplicate_gu
     fxrecord.parentName    = "Bookmarks Toolbar";
     fxrecord.parentid      = "toolbar";
     store.applyIncoming(fxrecord);
 
     _("Verify it has been created correctly.");
     let id = store.idForGUID(fxrecord.id);
     do_check_eq(store.GUIDForId(id), fxrecord.id);
     do_check_true(Svc.Bookmark.getBookmarkURI(id).equals(fxuri));
-    do_check_eq(Utils.anno(id, "bookmarkProperties/description"),
+    do_check_eq(Svc.Annos.getItemAnnotation(id, "bookmarkProperties/description"),
                 fxrecord.description);
     
     _("Copy the record as happens in the UI: with the same GUID.");
     let parentID = Svc.Bookmark.getFolderIdForItem(id);
     let copy = Svc.Bookmark.insertBookmark(parentID, fxuri, -1, fxrecord.title);
     Svc.Bookmark.setItemLastModified(copy, Svc.Bookmark.getItemLastModified(id));
     Svc.Bookmark.setItemDateAdded(copy, Svc.Bookmark.getItemDateAdded(id));
     store._setGUID(copy, fxrecord.id);
@@ -392,18 +395,19 @@ function test_copying_avoid_duplicate_gu
     
     _("Calling idForGUID fixes things.");
     _("GUID before: " + store.GUIDForId(copy));
     do_check_eq(store.idForGUID(fxrecord.id), id);           // Oldest wins.
     _("GUID now: " + store.GUIDForId(copy));
     do_check_neq(store.GUIDForId(copy), store.GUIDForId(id));
     
     _("Verify that the anno itself has changed.");
-    do_check_neq(Utils.anno(copy, "sync/guid"), fxrecord.id);
-    do_check_eq(Utils.anno(copy, "sync/guid"), store.GUIDForId(copy));
+    do_check_neq(Svc.Annos.getItemAnnotation(copy, "sync/guid"), fxrecord.id);
+    do_check_eq(Svc.Annos.getItemAnnotation(copy, "sync/guid"),
+                store.GUIDForId(copy));
     
   } finally {
     _("Clean up.");
     store.wipe();
   }
 }
 
 function run_test() {
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -278,17 +278,18 @@ function test_guid_stripping() {
     let victim = createBmk();
 
     _("Suspect: " + suspect + ", victim: " + victim);
 
     let suspectGUID = store.GUIDForId(suspect);
     let victimGUID  = store.GUIDForId(victim);
     _("Set the GUID on one entry to be the same as another.");
     do_check_neq(suspectGUID, victimGUID);
-    Utils.anno(suspect, SYNC_GUID_ANNO, store.GUIDForId(victim));
+    Svc.Annos.setItemAnnotation(suspect, SYNC_GUID_ANNO, store.GUIDForId(victim),
+                                0, Svc.Annos.EXPIRE_NEVER);
 
     _("Tracker changed it to something else.");
     let newGUID = store.GUIDForId(suspect);
     do_check_neq(newGUID, victimGUID);
     do_check_neq(newGUID, suspectGUID);
 
     _("Victim GUID remains unchanged.");
     do_check_eq(victimGUID, store.GUIDForId(victim));
deleted file mode 100644
--- a/services/sync/tests/unit/test_utils_anno.js
+++ /dev/null
@@ -1,41 +0,0 @@
-_("Make sure various combinations of anno arguments do the right get/set for pages/items");
-Cu.import("resource://services-sync/util.js");
-
-function run_test() {
-  _("create a bookmark to a url so it exists");
-  let url = "about:";
-  let bmkid = Svc.Bookmark.insertBookmark(Svc.Bookmark.unfiledBookmarksFolder,
-                                          Utils.makeURI(url), -1, "");
-
-  _("set an anno on the bookmark ");
-  Utils.anno(bmkid, "anno", "hi");
-  do_check_eq(Utils.anno(bmkid, "anno"), "hi");
-
-  _("set an anno on a url");
-  Utils.anno(url, "tation", "hello");
-  do_check_eq(Utils.anno(url, "tation"), "hello");
-
-  _("make sure getting it also works with a nsIURI");
-  let uri = Utils.makeURI(url);
-  do_check_eq(Utils.anno(uri, "tation"), "hello");
-
-  _("make sure annotations get updated");
-  Utils.anno(uri, "tation", "bye!");
-  do_check_eq(Utils.anno(url, "tation"), "bye!");
-
-  _("sanity check that the item anno is still there");
-  do_check_eq(Utils.anno(bmkid, "anno"), "hi");
-
-  _("invalid uris don't get annos");
-  let didThrow = false;
-  try {
-    Utils.anno("foo/bar/baz", "bad");
-  }
-  catch(ex) {
-    didThrow = true;
-  }
-  do_check_true(didThrow);
-
-  _("cleaning up the bookmark we created");
-  Svc.Bookmark.removeItem(bmkid);
-}