From 15b3f3976e6056892ce161274a19a31b691910c4 Mon Sep 17 00:00:00 2001
From: Gregory Szorc <gps@mozilla.com>
Date: Fri, 21 Sep 2012 11:22:59 -0700
Subject: [PATCH] Bug 792990 - Properly handle add-ons when resetting Sync;
 r=rnewman

Due to a bug in the add-on sync implementation, resetting Sync would
cause all add-ons to be uninstalled and not replaced with the server
data.
---
 services/sync/modules/engines/addons.js       | 22 +++++++++---
 services/sync/tests/tps/all_tests.json        |  3 +-
 services/sync/tests/tps/test_addon_wipe.js    | 34 +++++++++++++++++++
 services/sync/tests/unit/test_addons_store.js | 26 ++++++++++++++
 4 files changed, 79 insertions(+), 6 deletions(-)
 create mode 100644 services/sync/tests/tps/test_addon_wipe.js

diff --git a/services/sync/modules/engines/addons.js b/services/sync/modules/engines/addons.js
index b86326480f0f..b0b7e2c10287 100644
--- a/services/sync/modules/engines/addons.js
+++ b/services/sync/modules/engines/addons.js
@@ -337,15 +337,27 @@ AddonsStore.prototype = {
   update: function update(record) {
     let addon = this.getAddonByID(record.addonID);
 
-    // This should only happen if there is a race condition between an add-on
-    // being uninstalled locally and Sync calling this function after it
-    // determines an add-on exists.
+    // update() is called if !this.itemExists. And, since itemExists consults
+    // the reconciler only, we need to take care of some corner cases.
+    //
+    // First, the reconciler could know about an add-on that was uninstalled
+    // and no longer present in the add-ons manager.
     if (!addon) {
-      this._log.warn("Requested to update record but add-on not found: " +
-                     record.addonID);
+      this.create(record);
       return;
     }
 
+    // It's also possible that the add-on is non-restartless and has pending
+    // install/uninstall activity.
+    //
+    // We wouldn't get here if the incoming record was for a deletion. So,
+    // check for pending uninstall and cancel if necessary.
+    if (addon.pendingOperations & AddonManager.PENDING_UNINSTALL) {
+      addon.cancelUninstall();
+
+      // We continue with processing because there could be state or ID change.
+    }
+
     let cb = Async.makeSpinningCallback();
     this.updateUserDisabled(addon, !record.enabled, cb);
     cb.wait();
diff --git a/services/sync/tests/tps/all_tests.json b/services/sync/tests/tps/all_tests.json
index e4f576df3cff..fdcbe182102a 100644
--- a/services/sync/tests/tps/all_tests.json
+++ b/services/sync/tests/tps/all_tests.json
@@ -25,7 +25,8 @@
     "test_addon_sanity.js",
     "test_addon_restartless_xpi.js",
     "test_addon_nonrestartless_xpi.js",
-    "test_addon_reconciling.js"
+    "test_addon_reconciling.js",
+    "test_addon_wipe.js"
   ]
 }
 
diff --git a/services/sync/tests/tps/test_addon_wipe.js b/services/sync/tests/tps/test_addon_wipe.js
new file mode 100644
index 000000000000..2aafbd6bf382
--- /dev/null
+++ b/services/sync/tests/tps/test_addon_wipe.js
@@ -0,0 +1,34 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// This test ensures that a client wipe followed by an "initial" sync will
+// restore add-ons. This test should expose flaws in the reconciling logic,
+// specifically around AddonsReconciler. This test is in response to bug
+// 792990.
+
+EnableEngines(["addons"]);
+
+let phases = {
+  "phase01": "profile1",
+  "phase02": "profile1",
+  "phase03": "profile1"
+};
+
+const id1 = "restartless-xpi@tests.mozilla.org";
+const id2 = "unsigned-xpi@tests.mozilla.org";
+
+Phase("phase01", [
+  [Addons.install, [id1]],
+  [Addons.install, [id2]],
+  [Sync]
+]);
+Phase("phase02", [
+  [Addons.verify, [id1], STATE_ENABLED],
+  [Addons.verify, [id2], STATE_ENABLED],
+  [Sync, SYNC_WIPE_CLIENT],
+  [Sync]
+]);
+Phase("phase03", [
+  [Addons.verify, [id1], STATE_ENABLED],
+  [Addons.verify, [id2], STATE_ENABLED]
+]);
diff --git a/services/sync/tests/unit/test_addons_store.js b/services/sync/tests/unit/test_addons_store.js
index d5444639fcd4..21b2a5426f97 100644
--- a/services/sync/tests/unit/test_addons_store.js
+++ b/services/sync/tests/unit/test_addons_store.js
@@ -420,3 +420,29 @@ add_test(function test_wipe() {
 
   run_next_test();
 });
+
+add_test(function test_wipe_and_install() {
+  _("Ensure wipe followed by install works.");
+
+  // This tests the reset sync flow where remote data is replaced by local. The
+  // receiving client will see a wipe followed by a record which should undo
+  // the wipe.
+  let installed = installAddon("test_bootstrap1_1");
+
+  let record = createRecordForThisApp(installed.syncGUID, installed.id, true,
+                                      false);
+
+  Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
+  store.wipe();
+
+  let deleted = getAddonFromAddonManagerByID(installed.id);
+  do_check_null(deleted);
+
+  store.applyIncoming(record);
+
+  let fetched = getAddonFromAddonManagerByID(record.addonID);
+  do_check_true(!!fetched);
+
+  Svc.Prefs.reset("addons.ignoreRepositoryChecking");
+  run_next_test();
+});