From 8de35dcc154801f78e8ff24d0e990a136aec738b Mon Sep 17 00:00:00 2001
From: Richard Newman <rnewman@mozilla.com>
Date: Thu, 13 Jun 2013 15:44:28 -0700
Subject: [PATCH] Bug 873861 - Intermittent test_addons_store.js | Test timed
 out. r=gps

---
 services/sync/modules/addonsreconciler.js     | 32 +++++++++++++++----
 .../sync/tests/unit/test_addons_engine.js     | 10 ++++++
 services/sync/tests/unit/test_addons_store.js | 11 +++++++
 3 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/services/sync/modules/addonsreconciler.js b/services/sync/modules/addonsreconciler.js
index 9308a46fca5e..7ae681e6bc5d 100644
--- a/services/sync/modules/addonsreconciler.js
+++ b/services/sync/modules/addonsreconciler.js
@@ -64,8 +64,7 @@ this.EXPORTED_SYMBOLS = ["AddonsReconciler", "CHANGE_INSTALLED",
  * When you are finished with the instance, please call:
  *
  *   reconciler.stopListening();
- *   reconciler.saveStateFile(...);
- *
+ *   reconciler.saveState(...);
  *
  * There are 2 classes of listeners in the AddonManager: AddonListener and
  * InstallListener. This class is a listener for both (member functions just
@@ -124,12 +123,23 @@ AddonsReconciler.prototype = {
   /** Flag indicating whether we are listening to AddonManager events. */
   _listening: false,
 
-  /** Whether state has been loaded from a file.
+  /**
+   * Whether state has been loaded from a file.
    *
    * State is loaded on demand if an operation requires it.
    */
   _stateLoaded: false,
 
+  /**
+   * Define this as false if the reconciler should not persist state
+   * to disk when handling events.
+   *
+   * This allows test code to avoid spinning to write during observer
+   * notifications and xpcom shutdown, which appears to cause hangs on WinXP
+   * (Bug 873861).
+   */
+  _shouldPersist: true,
+
   /** log4moz logger instance */
   _log: null,
 
@@ -384,7 +394,12 @@ AddonsReconciler.prototype = {
         }
       }
 
-      this.saveState(null, callback);
+      // See note for _shouldPersist.
+      if (this._shouldPersist) {
+        this.saveState(null, callback);
+      } else {
+        callback();
+      }
     }.bind(this));
   },
 
@@ -610,9 +625,12 @@ AddonsReconciler.prototype = {
           }
       }
 
-      let cb = Async.makeSpinningCallback();
-      this.saveState(null, cb);
-      cb.wait();
+      // See note for _shouldPersist.
+      if (this._shouldPersist) {
+        let cb = Async.makeSpinningCallback();
+        this.saveState(null, cb);
+        cb.wait();
+      }
     }
     catch (ex) {
       this._log.warn("Exception: " + Utils.exceptionStr(ex));
diff --git a/services/sync/tests/unit/test_addons_engine.js b/services/sync/tests/unit/test_addons_engine.js
index 4244ddc2a45a..4646e821508f 100644
--- a/services/sync/tests/unit/test_addons_engine.js
+++ b/services/sync/tests/unit/test_addons_engine.js
@@ -231,6 +231,12 @@ add_test(function test_disabled_install_semantics() {
   server.stop(advance_test);
 });
 
+add_test(function cleanup() {
+  // There's an xpcom-shutdown hook for this, but let's give this a shot.
+  reconciler.stopListening();
+  run_next_test();
+});
+
 function run_test() {
   initTestLogging("Trace");
   Log4Moz.repository.getLogger("Sync.Engine.Addons").level =
@@ -245,5 +251,9 @@ function run_test() {
 
   reconciler.startListening();
 
+  // Don't flush to disk in the middle of an event listener!
+  // This causes test hangs on WinXP.
+  reconciler._shouldPersist = false;
+
   advance_test();
 }
diff --git a/services/sync/tests/unit/test_addons_store.js b/services/sync/tests/unit/test_addons_store.js
index 4a0e419ab0c2..5d7f6c19af8b 100644
--- a/services/sync/tests/unit/test_addons_store.js
+++ b/services/sync/tests/unit/test_addons_store.js
@@ -73,6 +73,10 @@ function run_test() {
 
   reconciler.startListening();
 
+  // Don't flush to disk in the middle of an event listener!
+  // This causes test hangs on WinXP.
+  reconciler._shouldPersist = false;
+
   run_next_test();
 }
 
@@ -461,3 +465,10 @@ add_test(function test_wipe_and_install() {
   Svc.Prefs.reset("addons.ignoreRepositoryChecking");
   server.stop(run_next_test);
 });
+
+add_test(function cleanup() {
+  // There's an xpcom-shutdown hook for this, but let's give this a shot.
+  reconciler.stopListening();
+  run_next_test();
+});
+