author | Gregory Szorc <gps@mozilla.com> |
Mon, 20 Feb 2012 14:53:03 -0800 | |
changeset 87256 | dec32ab636c6bf0d9afaaa89ee3581d8b7044181 |
parent 87255 | 1e6b5a9bbdccddd000666c7f5724d1aacd74455f |
child 87257 | a22548d04fc3cd10c339a39243a66060c554b08b |
push id | 22103 |
push user | bmo@edmorley.co.uk |
push date | Tue, 21 Feb 2012 12:01:45 +0000 |
treeherder | mozilla-central@4038ffaa5d82 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | Unfocused, rnewman |
bugs | 712542 |
milestone | 13.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/services/sync/modules/engines/addons.js +++ b/services/sync/modules/engines/addons.js @@ -312,22 +312,22 @@ AddonsStore.prototype = { Store.prototype.applyIncoming.call(this, record); }, /** * Provides core Store API to create/install an add-on from a record. */ create: function create(record) { - // Ideally, we'd set syncGUID and userDisabled on install. For now, we - // make the changes post-installation. - // TODO Set syncGUID and userDisabled in one step, during install. - let cb = Async.makeSpinningCallback(); - this.installAddonsFromIDs([record.addonID], cb); + this.installAddons([{ + id: record.addonID, + syncGUID: record.id, + enabled: record.enabled + }], cb); // This will throw if there was an error. This will get caught by the sync // engine and the record will try to be applied later. let results = cb.wait(); let addon; for each (let a in results.addons) { if (a.id == record.addonID) { @@ -337,22 +337,16 @@ AddonsStore.prototype = { } // This should never happen, but is present as a fail-safe. if (!addon) { throw new Error("Add-on not found after install: " + record.addonID); } this._log.info("Add-on installed: " + record.addonID); - this._log.info("Setting add-on Sync GUID to remote: " + record.id); - addon.syncGUID = record.id; - - cb = Async.makeSpinningCallback(); - this.updateUserDisabled(addon, !record.enabled, cb); - cb.wait(); }, /** * Provides core Store API to remove/uninstall an add-on from a record. */ remove: function remove(record) { // If this is called, the payload is empty, so we have to find by GUID. let addon = this.getAddonByGUID(record.id); @@ -677,51 +671,80 @@ AddonsStore.prototype = { addon.iconURL, addon.version ); }, /** * Installs an add-on from an AddonSearchResult instance. * + * The options argument defines extra options to control the install. + * Recognized keys in this map are: + * + * syncGUID - Sync GUID to use for the new add-on. + * enabled - Boolean indicating whether the add-on should be enabled upon + * install. + * * When complete it calls a callback with 2 arguments, error and result. * * If error is falsy, result is an object. If error is truthy, result is * null. * * The result object has the following keys: * * id ID of add-on that was installed. * install AddonInstall that was installed. * addon Addon that was installed. * * @param addon * AddonSearchResult to install add-on from. + * @param options + * Object with additional metadata describing how to install add-on. * @param cb * Function to be invoked with result of operation. */ installAddonFromSearchResult: - function installAddonFromSearchResult(addon, cb) { + function installAddonFromSearchResult(addon, options, cb) { this._log.info("Trying to install add-on from search result: " + addon.id); this.getInstallFromSearchResult(addon, function(error, install) { if (error) { cb(error, null); return; } if (!install) { cb(new Error("AddonInstall not available: " + addon.id), null); return; } try { this._log.info("Installing " + addon.id); + let log = this._log; let listener = { + onInstallStarted: function(install) { + if (!options) { + return; + } + + if (options.syncGUID) { + log.info("Setting syncGUID of " + install.name +": " + + options.syncGUID); + install.addon.syncGUID = options.syncGUID; + } + + // We only need to change userDisabled if it is disabled because + // enabled is the default. + if ("enabled" in options && !options.enabled) { + log.info("Marking add-on as disabled for install: " + + install.name); + install.addon.userDisabled = true; + } + }, onInstallEnded: function(install, addon) { install.removeListener(listener); cb(null, {id: addon.id, install: install, addon: addon}); }, onInstallFailed: function(install) { install.removeListener(listener); @@ -887,42 +910,54 @@ AddonsStore.prototype = { if (!addon.appDisabled) { callback(null, addon); return; } // Else the listener will handle invoking the callback. }, /** - * Installs multiple add-ons specified by their IDs. + * Installs multiple add-ons specified by metadata. + * + * The first argument is an array of objects. Each object must have the + * following keys: + * + * id - public ID of the add-on to install. + * syncGUID - syncGUID for new add-on. + * enabled - boolean indicating whether the add-on should be enabled. * * The callback will be called when activity on all add-ons is complete. The * callback receives 2 arguments, error and result. * * If error is truthy, it contains a string describing the overall error. * * The 2nd argument to the callback is always an object with details on the * overall execution state. It contains the following keys: * * installedIDs Array of add-on IDs that were installed. * installs Array of AddonInstall instances that were installed. * addons Array of Addon instances that were installed. * errors Array of errors encountered. Only has elements if error is * truthy. * - * @param ids - * Array of add-on string IDs to install. + * @param installs + * Array of objects describing add-ons to install. * @param cb * Function to be called when all actions are complete. */ - installAddonsFromIDs: function installAddonsFromIDs(ids, cb) { + installAddons: function installAddons(installs, cb) { if (!cb) { throw new Error("Invalid argument: cb is not defined."); } + let ids = []; + for each (let addon in installs) { + ids.push(addon.id); + } + AddonRepository.getAddonsByIDs(ids, { searchSucceeded: function searchSucceeded(addons, addonsLength, total) { this._log.info("Found " + addonsLength + "/" + ids.length + " add-ons during repository search."); let ourResult = { installedIDs: [], installs: [], @@ -1004,17 +1039,25 @@ AddonsStore.prototype = { if (!expectedInstallCount) { cb(null, ourResult); return; } // Start all the installs asynchronously. They will report back to us // as they finish, eventually triggering the global callback. for each (let addon in toInstall) { - this.installAddonFromSearchResult(addon, installCallback); + let options = {}; + for each (let install in installs) { + if (install.id == addon.id) { + options = install; + break; + } + } + + this.installAddonFromSearchResult(addon, options, installCallback); } }.bind(this), searchFailed: function searchFailed() { cb(new Error("AddonRepository search failed"), null); }.bind(this) });
new file mode 100644 --- /dev/null +++ b/services/sync/tests/unit/addon1-search.xml @@ -0,0 +1,27 @@ +<?xml version="1.0" encoding="UTF-8"?> +<searchresults total_results="1"> + <addon id="5617"> + <name>Non-Restartless Test Extension</name> + <type id="1">Extension</type> + <guid>addon1@tests.mozilla.org</guid> + <slug>addon11</slug> + <version>1.0</version> + + <compatible_applications><application> + <name>Firefox</name> + <application_id>1</application_id> + <min_version>3.6</min_version> + <max_version>*</max_version> + <appID>xpcshell@tests.mozilla.org</appID> + </application></compatible_applications> + <all_compatible_os><os>ALL</os></all_compatible_os> + + <install os="ALL" size="485">http://127.0.0.1:8888/addon1.xpi</install> + <created epoch="1252903662"> + 2009-09-14T04:47:42Z + </created> + <last_updated epoch="1315255329"> + 2011-09-05T20:42:09Z + </last_updated> + </addon> +</searchresults>
--- a/services/sync/tests/unit/test_addons_engine.js +++ b/services/sync/tests/unit/test_addons_engine.js @@ -3,16 +3,22 @@ "use strict"; Cu.import("resource://gre/modules/AddonManager.jsm"); Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://services-sync/addonsreconciler.js"); Cu.import("resource://services-sync/async.js"); Cu.import("resource://services-sync/engines/addons.js"); +Cu.import("resource://services-sync/ext/Preferences.js"); +Cu.import("resource://services-sync/service.js"); + +let prefs = new Preferences(); +prefs.set("extensions.getAddons.get.url", + "http://localhost:8888/search/guid:%IDS%"); loadAddonTestFunctions(); startupManager(); Engines.register(AddonsEngine); let engine = Engines.get("addons"); let reconciler = engine._reconciler; let tracker = engine._tracker; @@ -129,18 +135,115 @@ add_test(function test_get_changed_ids() changes = engine.getChangedIDs(); _(JSON.stringify(changes)); do_check_eq(0, Object.keys(changes).length); advance_test(); }); +add_test(function test_disabled_install_semantics() { + _("Ensure that syncing a disabled add-on preserves proper state."); + + // This is essentially a test for bug 712542, which snuck into the original + // add-on sync drop. It ensures that when an add-on is installed that the + // disabled state and incoming syncGUID is preserved, even on the next sync. + + Svc.Prefs.set("addons.ignoreRepositoryChecking", true); + + const USER = "foo"; + const PASSWORD = "password"; + const PASSPHRASE = "abcdeabcdeabcdeabcdeabcdea"; + const ADDON_ID = "addon1@tests.mozilla.org"; + + Service.username = USER; + Service.password = PASSWORD; + Service.passphrase = PASSPHRASE; + Service.serverURL = TEST_SERVER_URL; + Service.clusterURL = TEST_CLUSTER_URL; + + generateNewKeys(); + + let contents = { + meta: {global: {engines: {addons: {version: engine.version, + syncID: engine.syncID}}}}, + crypto: {}, + addons: {} + }; + + let server = new SyncServer(); + server.registerUser(USER, "password"); + server.createContents(USER, contents); + server.start(); + + let amoServer = new nsHttpServer(); + amoServer.registerFile("/search/guid:addon1%40tests.mozilla.org", + do_get_file("addon1-search.xml")); + + let installXPI = ExtensionsTestPath("/addons/test_install1.xpi"); + amoServer.registerFile("/addon1.xpi", do_get_file(installXPI)); + amoServer.start(8888); + + // Insert an existing record into the server. + let id = Utils.makeGUID(); + let now = Date.now() / 1000; + + let record = encryptPayload({ + id: id, + applicationID: Services.appinfo.ID, + addonID: ADDON_ID, + enabled: false, + deleted: false, + source: "amo", + }); + let wbo = new ServerWBO(id, record, now - 2); + server.insertWBO(USER, "addons", wbo); + + _("Performing sync of add-ons engine."); + engine._sync(); + + // At this point the non-restartless extension should be staged for install. + + // Don't need this server any more. + let cb = Async.makeSpinningCallback(); + amoServer.stop(cb); + cb.wait(); + + // We ensure the reconciler has recorded the proper ID and enabled state. + let addon = reconciler.getAddonStateFromSyncGUID(id); + do_check_neq(null, addon); + do_check_eq(false, addon.enabled); + + // We fake an app restart and perform another sync, just to make sure things + // are sane. + restartManager(); + + engine._sync(); + + // The client should not upload a new record. The old record should be + // retained and unmodified. + let collection = server.getCollection(USER, "addons"); + do_check_eq(1, collection.count()); + + let payload = collection.payloads()[0]; + do_check_neq(null, collection.wbo(id)); + do_check_eq(ADDON_ID, payload.addonID); + do_check_false(payload.enabled); + + server.stop(advance_test); +}); + function run_test() { initTestLogging("Trace"); - Log4Moz.repository.getLogger("Sync.Engine.Addons").level = Log4Moz.Level.Trace; + Log4Moz.repository.getLogger("Sync.Engine.Addons").level = + Log4Moz.Level.Trace; + Log4Moz.repository.getLogger("Sync.Store.Addons").level = Log4Moz.Level.Trace; + Log4Moz.repository.getLogger("Sync.Tracker.Addons").level = + Log4Moz.Level.Trace; Log4Moz.repository.getLogger("Sync.AddonsRepository").level = Log4Moz.Level.Trace; + new SyncTestingInfrastructure(); + reconciler.startListening(); advance_test(); }
--- a/services/sync/tests/unit/test_addons_store.js +++ b/services/sync/tests/unit/test_addons_store.js @@ -449,17 +449,17 @@ add_test(function test_source_uri_rewrit Svc.Prefs.set("addons.ignoreRepositoryChecking", true); // We resort to monkeypatching because of the API design. let oldFunction = store.__proto__.installAddonFromSearchResult; let installCalled = false; store.__proto__.installAddonFromSearchResult = - function testInstallAddon(addon, cb) { + function testInstallAddon(addon, metadata, cb) { do_check_eq("http://127.0.0.1:8888/require.xpi?src=sync", addon.sourceURI.spec); installCalled = true; store.getInstallFromSearchResult(addon, function (error, install) { do_check_eq("http://127.0.0.1:8888/require.xpi?src=sync", @@ -467,17 +467,17 @@ add_test(function test_source_uri_rewrit cb(null, {id: addon.id, addon: addon, install: install}); }); }; let server = createAndStartHTTPServer(HTTP_PORT); let installCallback = Async.makeSpinningCallback(); - store.installAddonsFromIDs(["rewrite@tests.mozilla.org"], installCallback); + store.installAddons([{id: "rewrite@tests.mozilla.org"}], installCallback); installCallback.wait(); do_check_true(installCalled); store.__proto__.installAddonFromSearchResult = oldFunction; Svc.Prefs.reset("addons.ignoreRepositoryChecking"); server.stop(run_next_test); }); @@ -487,17 +487,17 @@ add_test(function test_handle_empty_sour Svc.Prefs.set("addons.ignoreRepositoryChecking", true); let server = createAndStartHTTPServer(HTTP_PORT); const ID = "missing-sourceuri@tests.mozilla.org"; let cb = Async.makeSpinningCallback(); - store.installAddonsFromIDs([ID], cb); + store.installAddons([{id: ID}], cb); let result = cb.wait(); do_check_true("installedIDs" in result); do_check_eq(0, result.installedIDs.length); Svc.Prefs.reset("addons.ignoreRepositoryChecking"); server.stop(run_next_test); });
--- a/services/sync/tps/extensions/tps/modules/addons.jsm +++ b/services/sync/tps/extensions/tps/modules/addons.jsm @@ -128,17 +128,17 @@ Addon.prototype = { // For Install, the id parameter initially passed is really the filename // for the addon's install .xml; we'll read the actual id from the .xml. let cb = Async.makeSpinningCallback(); // We call the store's APIs for installation because it is simpler. If that // API is broken, it should ideally be caught by an xpcshell test. But, if // TPS tests fail, it's all the same: a genuite reported error. let store = Engines.get("addons")._store; - store.installAddonsFromIDs([this.id], cb); + store.installAddons([{id: this.id}], cb); let result = cb.wait(); Logger.AssertEqual(1, result.installedIDs.length, "Exactly 1 add-on was installed."); Logger.AssertEqual(this.id, result.installedIDs[0], "Add-on was installed successfully: " + this.id); }, setEnabled: function setEnabled(flag) {