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

@@ -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();

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"
   ]
 }
 

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]
+]);

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();
+});