Bug 584040 - Fix leaky tests Part 2 [r=mconnor] (CLOSED TREE)

Make sure we explicitly nullify the cached services in the BookmarksStore & BookmarksTracker.
Also, explicitly finalize the statement if it was created.

services/sync/modules/engines/bookmarks.js

@@ -249,6 +249,17 @@ BookmarksEngine.prototype = {
 
 function BookmarksStore(name) {
   Store.call(this, name);
+
+  // Explicitly nullify our references to our cached services so we don't leak
+  Observers.add("places-shutdown", function() {
+    this.__bms = null;
+    this.__hsvc = null;
+    this.__ls = null;
+    this.__ms = null;
+    this.__ts = null;
+    if (this.__frecencyStm)
+      this.__frecencyStm.finalize();
+  }, this);
 }
 BookmarksStore.prototype = {
   __proto__: Store.prototype,
@@ -273,22 +284,24 @@ BookmarksStore.prototype = {
   get _ls() {
     if (!this.__ls)
       this.__ls = Cc["@mozilla.org/browser/livemark-service;2"].
-        getService(Ci.nsILivemarkService);
+                  getService(Ci.nsILivemarkService);
     return this.__ls;
   },
 
+  __ms: null,
   get _ms() {
-    let ms;
-    try {
-      ms = Cc["@mozilla.org/microsummary/service;1"].
-        getService(Ci.nsIMicrosummaryService);
-    } catch (e) {
-      ms = null;
-      this._log.warn("Could not load microsummary service");
-      this._log.debug(e);
+    if (!this.__ms) {
+      try {
+        this.__ms = Cc["@mozilla.org/microsummary/service;1"].
+                    getService(Ci.nsIMicrosummaryService);
+      } catch (e) {
+        this._log.warn("Could not load microsummary service");
+        this._log.debug(e);
+        // Redefine our getter so we won't keep trying to get the service
+        this.__defineGetter__("_ms", function() null);
+      }
     }
-    this.__defineGetter__("_ms", function() ms);
-    return ms;
+    return this.__ms;
   },
 
   __ts: null,
@@ -835,14 +848,16 @@ BookmarksStore.prototype = {
     return record;
   },
 
+  __frecencyStm: null,
   get _frecencyStm() {
-    this._log.trace("Creating SQL statement: _frecencyStm");
-    let stm = Svc.History.DBConnection.createStatement(
-      "SELECT frecency " +
-      "FROM moz_places " +
-      "WHERE url = :url");
-    this.__defineGetter__("_frecencyStm", function() stm);
-    return stm;
+    if (!this.__frecencyStm) {
+      this._log.trace("Creating SQL statement: _frecencyStm");
+      this.__frecencyStm = Svc.History.DBConnection.createStatement(
+        "SELECT frecency " +
+        "FROM moz_places " +
+        "WHERE url = :url");
+    }
+    return this.__frecencyStm;
   },
 
   _calculateIndex: function _calculateIndex(record) {
@@ -987,22 +1002,30 @@ function BookmarksTracker(name) {
     this.ignoreID(guid);
 
   Svc.Bookmark.addObserver(this, false);
+
+  // Explicitly nullify our references to our cached services so we don't leak
+  Observers.add("places-shutdown", function() {
+    this.__ls = null;
+    this.__bms = null;
+  }, this);
 }
 BookmarksTracker.prototype = {
   __proto__: Tracker.prototype,
 
+  __bms: null,
   get _bms() {
-    let bms = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
-      getService(Ci.nsINavBookmarksService);
-    this.__defineGetter__("_bms", function() bms);
-    return bms;
+    if (!this.__bms)
+      this.__bms = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
+                   getService(Ci.nsINavBookmarksService);
+    return this.__bms;
   },
 
+  __ls: null,
   get _ls() {
-    let ls = Cc["@mozilla.org/browser/livemark-service;2"].
-      getService(Ci.nsILivemarkService);
-    this.__defineGetter__("_ls", function() ls);
-    return ls;
+    if (!this.__ls)
+      this.__ls = Cc["@mozilla.org/browser/livemark-service;2"].
+                  getService(Ci.nsILivemarkService);
+    return this.__ls;
   },
 
   QueryInterface: XPCOMUtils.generateQI([

services/sync/tests/unit/test_bookmark_order.js

@@ -39,9 +39,6 @@ function check(expected) {
 }
 
 function run_test() {
-  //XXXzpao test is disabled (bug 583985)
-  return;
-
   _("Starting with a clean slate of no bookmarks");
   let store = new (new BookmarksEngine())._storeObj();
   store.wipe();

services/sync/tests/unit/test_bookmark_predecessor.js

@@ -3,9 +3,6 @@ Cu.import("resource://services-sync/engines/bookmarks.js");
 Cu.import("resource://services-sync/util.js");
 
 function run_test() {
-  //XXXzpao test is disabled (bug 583985)
-  return;
-
   _("Starting with a clean slate of no bookmarks");
   let store = new (new BookmarksEngine())._storeObj();
   store.wipe();