Bug 712542 - Apply add-on state before install when installing through Sync; r=Unfocused, r=rnewman

This commit is contained in:
Gregory Szorc 2012-02-20 14:53:03 -08:00
Родитель cbe09696da
Коммит 75b2c89dbf
5 изменённых файлов: 195 добавлений и 22 удалений

Просмотреть файл

@ -317,12 +317,12 @@ AddonsStore.prototype = {
* 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.
@ -342,12 +342,6 @@ AddonsStore.prototype = {
}
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();
},
/**
@ -682,6 +676,13 @@ AddonsStore.prototype = {
/**
* 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
@ -695,11 +696,13 @@ AddonsStore.prototype = {
*
* @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) {
@ -715,8 +718,28 @@ AddonsStore.prototype = {
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);
@ -892,7 +915,14 @@ AddonsStore.prototype = {
},
/**
* 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.
@ -908,16 +938,21 @@ AddonsStore.prototype = {
* 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 +
@ -1009,7 +1044,15 @@ AddonsStore.prototype = {
// 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),

Просмотреть файл

@ -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>

Просмотреть файл

@ -8,6 +8,12 @@ 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();
@ -134,12 +140,109 @@ add_test(function test_get_changed_ids() {
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();

Просмотреть файл

@ -454,7 +454,7 @@ add_test(function test_source_uri_rewrite() {
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);
@ -472,7 +472,7 @@ add_test(function test_source_uri_rewrite() {
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);
@ -492,7 +492,7 @@ add_test(function test_handle_empty_source_uri() {
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);

Просмотреть файл

@ -133,7 +133,7 @@ Addon.prototype = {
// 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.");