--- a/services/sync/modules/engines/addons.js
+++ b/services/sync/modules/engines/addons.js
@@ -39,26 +39,27 @@
var {classes: Cc, interfaces: Ci, utils: Cu} = Components;
Cu.import("resource://services-sync/addonutils.js");
Cu.import("resource://services-sync/addonsreconciler.js");
Cu.import("resource://services-sync/engines.js");
Cu.import("resource://services-sync/record.js");
Cu.import("resource://services-sync/util.js");
Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://services-sync/collection_validator.js");
Cu.import("resource://services-common/async.js");
Cu.import("resource://gre/modules/Preferences.jsm");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "AddonManager",
"resource://gre/modules/AddonManager.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "AddonRepository",
"resource://gre/modules/addons/AddonRepository.jsm");
-this.EXPORTED_SYMBOLS = ["AddonsEngine"];
+this.EXPORTED_SYMBOLS = ["AddonsEngine", "AddonValidator"];
// 7 days in milliseconds.
const PRUNE_ADDON_CHANGES_THRESHOLD = 60 * 60 * 24 * 7 * 1000;
/**
* AddonRecord represents the state of an add-on in an application.
*
* Each add-on has its own record for each application ID it is installed
@@ -171,17 +172,17 @@ AddonsEngine.prototype = {
continue;
}
// Keep newest modified time.
if (id in changes && changeTime < changes[id]) {
continue;
}
- if (!this._store.isAddonSyncable(addons[id])) {
+ if (!this.isAddonSyncable(addons[id])) {
continue;
}
this._log.debug("Adding changed add-on from changes log: " + id);
let addon = addons[id];
changes[addon.guid] = changeTime.getTime() / 1000;
}
@@ -229,16 +230,20 @@ AddonsEngine.prototype = {
* This will synchronously load the reconciler's state from the file
* system (if needed) and refresh the state of the reconciler.
*/
_refreshReconcilerState: function _refreshReconcilerState() {
this._log.debug("Refreshing reconciler state");
let cb = Async.makeSpinningCallback();
this._reconciler.refreshGlobalState(cb);
cb.wait();
+ },
+
+ isAddonSyncable(addon, ignoreRepoCheck) {
+ return this._store.isAddonSyncable(addon, ignoreRepoCheck);
}
};
/**
* This is the primary interface between Sync and the Addons Manager.
*
* In addition to the core store APIs, we provide convenience functions to wrap
* Add-on Manager APIs with Sync-specific semantics.
@@ -528,19 +533,22 @@ AddonsStore.prototype = {
return Async.waitForSyncCallback(cb);
},
/**
* Determines whether an add-on is suitable for Sync.
*
* @param addon
* Addon instance
+ * @param ignoreRepoCheck
+ * Should we skip checking the Addons repository (primarially useful
+ * for testing and validation).
* @return Boolean indicating whether it is appropriate for Sync
*/
- isAddonSyncable: function isAddonSyncable(addon) {
+ isAddonSyncable: function isAddonSyncable(addon, ignoreRepoCheck = false) {
// Currently, we limit syncable add-ons to those that are:
// 1) In a well-defined set of types
// 2) Installed in the current profile
// 3) Not installed by a foreign entity (i.e. installed by the app)
// since they act like global extensions.
// 4) Is not a hotfix.
// 5) The addons XPIProvider doesn't veto it (i.e not being installed in
// the profile directory, or any other reasons it says the addon can't
@@ -587,18 +595,19 @@ AddonsStore.prototype = {
// tests are fixed (but keeping it doesn't hurt either)
if (this._extensionsPrefs.get("hotfix.id", null) == addon.id) {
this._log.debug(addon.id + " not syncable: is a hotfix.");
return false;
}
// If the AddonRepository's cache isn't enabled (which it typically isn't
// in tests), getCachedAddonByID always returns null - so skip the check
- // in that case.
- if (!AddonRepository.cacheEnabled) {
+ // in that case. We also provide a way to specifically opt-out of the check
+ // even if the cache is enabled, which is used by the validators.
+ if (ignoreRepoCheck || !AddonRepository.cacheEnabled) {
return true;
}
let cb = Async.makeSyncCallback();
AddonRepository.getCachedAddonByID(addon.id, cb);
let result = Async.waitForSyncCallback(cb);
if (!result) {
@@ -731,8 +740,74 @@ AddonsTracker.prototype = {
this.reconciler.addChangeListener(this);
},
stopTracking: function() {
this.reconciler.removeChangeListener(this);
this.reconciler.stopListening();
},
};
+
+class AddonValidator extends CollectionValidator {
+ constructor(engine = null) {
+ super("addons", "id", [
+ "addonID",
+ "enabled",
+ "applicationID",
+ "source"
+ ]);
+ this.engine = engine;
+ }
+
+ getClientItems() {
+ return Promise.all([
+ new Promise(resolve =>
+ AddonManager.getAllAddons(resolve)),
+ new Promise(resolve =>
+ AddonManager.getAddonsWithOperationsByTypes(["extension", "theme"], resolve)),
+ ]).then(([installed, addonsWithPendingOperation]) => {
+ // Addons pending install won't be in the first list, but addons pending
+ // uninstall/enable/disable will be in both lists.
+ let all = new Map(installed.map(addon => [addon.id, addon]));
+ for (let addon of addonsWithPendingOperation) {
+ all.set(addon.id, addon);
+ }
+ // Convert to an array since Map.prototype.values returns an iterable
+ return [...all.values()];
+ });
+ }
+
+ normalizeClientItem(item) {
+ let enabled = !item.userDisabled;
+ if (item.pendingOperations & AddonManager.PENDING_ENABLE) {
+ enabled = true;
+ } else if (item.pendingOperations & AddonManager.PENDING_DISABLE) {
+ enabled = false;
+ }
+ return {
+ enabled,
+ id: item.syncGUID,
+ addonID: item.id,
+ applicationID: Services.appinfo.ID,
+ source: "amo", // check item.foreignInstall?
+ original: item
+ };
+ }
+
+ normalizeServerItem(item) {
+ let guid = this.engine._findDupe(item);
+ if (guid) {
+ item.id = guid;
+ }
+ return item;
+ }
+
+ clientUnderstands(item) {
+ return item.applicationID === Services.appinfo.ID;
+ }
+
+ syncedByClient(item) {
+ return !item.original.hidden &&
+ !item.original.isSystem &&
+ !(item.original.pendingOperations & AddonManager.PENDING_UNINSTALL) &&
+ this.engine.isAddonSyncable(item.original, true);
+ }
+}
--- a/services/sync/tests/tps/test_addon_nonrestartless_xpi.js
+++ b/services/sync/tests/tps/test_addon_nonrestartless_xpi.js
@@ -28,70 +28,78 @@ var phases = {
const id = "unsigned-xpi@tests.mozilla.org";
Phase("phase01", [
[Addons.verifyNot, [id]],
[Addons.install, [id]],
[Sync]
]);
Phase("phase02", [
- [Addons.verify, [id], STATE_ENABLED]
+ [Addons.verify, [id], STATE_ENABLED],
+ [Sync]
]);
Phase("phase03", [
[Addons.verifyNot, [id]],
[Sync]
]);
Phase("phase04", [
[Addons.verify, [id], STATE_ENABLED],
+ [Sync]
]);
// Now we disable the add-on
Phase("phase05", [
[EnsureTracking],
[Addons.setEnabled, [id], STATE_DISABLED],
[Sync]
]);
Phase("phase06", [
[Addons.verify, [id], STATE_DISABLED],
+ [Sync]
]);
Phase("phase07", [
[Addons.verify, [id], STATE_ENABLED],
[Sync]
]);
Phase("phase08", [
- [Addons.verify, [id], STATE_DISABLED]
+ [Addons.verify, [id], STATE_DISABLED],
+ [Sync]
]);
// Now we re-enable it again.
Phase("phase09", [
[EnsureTracking],
[Addons.setEnabled, [id], STATE_ENABLED],
[Sync]
]);
Phase("phase10", [
[Addons.verify, [id], STATE_ENABLED],
+ [Sync]
]);
Phase("phase11", [
[Addons.verify, [id], STATE_DISABLED],
[Sync]
]);
Phase("phase12", [
- [Addons.verify, [id], STATE_ENABLED]
+ [Addons.verify, [id], STATE_ENABLED],
+ [Sync]
]);
// And we uninstall it
Phase("phase13", [
[EnsureTracking],
[Addons.verify, [id], STATE_ENABLED],
[Addons.uninstall, [id]],
[Sync]
]);
Phase("phase14", [
- [Addons.verifyNot, [id]]
+ [Addons.verifyNot, [id]],
+ [Sync]
]);
Phase("phase15", [
[Addons.verify, [id], STATE_ENABLED],
[Sync]
]);
Phase("phase16", [
- [Addons.verifyNot, [id]]
+ [Addons.verifyNot, [id]],
+ [Sync]
]);
--- a/services/sync/tests/tps/test_addon_reconciling.js
+++ b/services/sync/tests/tps/test_addon_reconciling.js
@@ -29,16 +29,19 @@ Phase("phase02", [
[Sync],
[Addons.verify, [id], STATE_ENABLED]
]);
// Now we disable in one and uninstall in the other.
Phase("phase03", [
[Sync], // Get GUID updates, potentially.
[Addons.setEnabled, [id], STATE_DISABLED],
+ // We've changed the state, but don't want this profile to sync until phase5,
+ // so if we ran a validation now we'd be expecting to find errors.
+ [Addons.skipValidation]
]);
Phase("phase04", [
[EnsureTracking],
[Addons.uninstall, [id]],
[Sync]
]);
// When we sync, the uninstall should take precedence because it was newer.
--- a/services/sync/tests/tps/test_addon_sanity.js
+++ b/services/sync/tests/tps/test_addon_sanity.js
@@ -20,10 +20,11 @@ Phase("phase1", [
[Addons.verifyNot, [id]],
// But it should be marked for Sync.
[Sync]
]);
Phase("phase2", [
// Add-on should be present after restart
- [Addons.verify, [id], STATE_ENABLED]
+ [Addons.verify, [id], STATE_ENABLED],
+ [Sync] // Sync to ensure everything is initialized enough for the addon validator to run
]);
--- a/services/sync/tests/tps/test_addon_wipe.js
+++ b/services/sync/tests/tps/test_addon_wipe.js
@@ -25,10 +25,11 @@ Phase("phase01", [
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]
+ [Addons.verify, [id2], STATE_ENABLED],
+ [Sync] // Sync to ensure that the addon validator can run without error
]);
--- a/services/sync/tps/extensions/tps/resource/tps.jsm
+++ b/services/sync/tps/extensions/tps/resource/tps.jsm
@@ -21,16 +21,17 @@ Cu.import("resource://gre/modules/AppCon
Cu.import("resource://gre/modules/PlacesUtils.jsm");
Cu.import("resource://services-common/async.js");
Cu.import("resource://services-sync/constants.js");
Cu.import("resource://services-sync/main.js");
Cu.import("resource://services-sync/util.js");
Cu.import("resource://services-sync/bookmark_validator.js");
Cu.import("resource://services-sync/engines/passwords.js");
Cu.import("resource://services-sync/engines/forms.js");
+Cu.import("resource://services-sync/engines/addons.js");
// TPS modules
Cu.import("resource://tps/logger.jsm");
// Module wrappers for tests
Cu.import("resource://tps/modules/addons.jsm");
Cu.import("resource://tps/modules/bookmarks.jsm");
Cu.import("resource://tps/modules/forms.jsm");
Cu.import("resource://tps/modules/history.jsm");
@@ -108,16 +109,17 @@ var TPS = {
_syncErrors: 0,
_syncWipeAction: null,
_tabsAdded: 0,
_tabsFinished: 0,
_test: null,
_triggeredSync: false,
_usSinceEpoch: 0,
_requestedQuit: false,
+ shouldValidateAddons: false,
shouldValidateBookmarks: false,
shouldValidatePasswords: false,
shouldValidateForms: false,
_init: function TPS__init() {
// Check if Firefox Accounts is enabled
let service = Cc["@mozilla.org/weave/service;1"]
.getService(Components.interfaces.nsISupports)
@@ -459,16 +461,17 @@ var TPS = {
}
catch(e) {
DumpPasswords();
throw(e);
}
},
HandleAddons: function (addons, action, state) {
+ this.shouldValidateAddons = true;
for (let entry of addons) {
Logger.logInfo("executing action " + action.toUpperCase() +
" on addon " + JSON.stringify(entry));
let addon = new Addon(this, entry);
switch(action) {
case ACTION_ADD:
addon.install();
break;
@@ -656,92 +659,93 @@ var TPS = {
if (serverRecordDumpStr) {
Logger.logInfo("Server bookmark records:\n" + serverRecordDumpStr + "\n");
}
this.DumpError("Bookmark validation failed", e);
}
Logger.logInfo("Bookmark validation finished");
},
- ValidatePasswords() {
- let serverRecordDumpStr;
- try {
- Logger.logInfo("About to perform password validation");
- let pwEngine = Weave.Service.engineManager.get("passwords");
- let validator = new PasswordValidator();
- let serverRecords = validator.getServerItems(pwEngine);
- let clientRecords = Async.promiseSpinningly(validator.getClientItems());
- serverRecordDumpStr = JSON.stringify(serverRecords);
-
- let { problemData } = validator.compareClientWithServer(clientRecords, serverRecords);
-
- for (let { name, count } of problemData.getSummary()) {
- if (count) {
- Logger.logInfo(`Validation problem: "${name}": ${JSON.stringify(problemData[name])}`);
- }
- Logger.AssertEqual(count, 0, `Password validation error of type ${name}`);
- }
- } catch (e) {
- // Dump the client records (should always be doable)
- DumpPasswords();
- // Dump the server records if gotten them already.
- if (serverRecordDumpStr) {
- Logger.logInfo("Server password records:\n" + serverRecordDumpStr + "\n");
- }
- this.DumpError("Password validation failed", e);
- }
- Logger.logInfo("Password validation finished");
- },
-
- ValidateForms() {
+ ValidateCollection(engineName, ValidatorType) {
let serverRecordDumpStr;
let clientRecordDumpStr;
try {
- Logger.logInfo("About to perform form validation");
- let engine = Weave.Service.engineManager.get("forms");
- let validator = new FormValidator();
+ Logger.logInfo(`About to perform validation for "${engineName}"`);
+ let engine = Weave.Service.engineManager.get(engineName);
+ let validator = new ValidatorType(engine);
let serverRecords = validator.getServerItems(engine);
let clientRecords = Async.promiseSpinningly(validator.getClientItems());
- clientRecordDumpStr = JSON.stringify(clientRecords, undefined, 2);
- serverRecordDumpStr = JSON.stringify(serverRecords, undefined, 2);
+ try {
+ // This substantially improves the logs for addons while not making a
+ // substantial difference for the other two
+ clientRecordDumpStr = JSON.stringify(clientRecords.map(r => {
+ let res = validator.normalizeClientItem(r);
+ delete res.original; // Try and prevent cyclic references
+ return res;
+ }));
+ } catch (e) {
+ // ignore the error, the dump string is just here to make debugging easier.
+ clientRecordDumpStr = "<Cyclic value>";
+ }
+ try {
+ serverRecordDumpStr = JSON.stringify(serverRecords);
+ } catch (e) {
+ // as above
+ serverRecordDumpStr = "<Cyclic value>";
+ }
let { problemData } = validator.compareClientWithServer(clientRecords, serverRecords);
for (let { name, count } of problemData.getSummary()) {
if (count) {
Logger.logInfo(`Validation problem: "${name}": ${JSON.stringify(problemData[name])}`);
}
- Logger.AssertEqual(count, 0, `Form validation error of type ${name}`);
+ Logger.AssertEqual(count, 0, `Validation error for "${engineName}" of type "${name}"`);
}
} catch (e) {
// Dump the client records if possible
if (clientRecordDumpStr) {
- Logger.logInfo("Client forms records:\n" + clientRecordDumpStr + "\n");
+ Logger.logInfo(`Client state for ${engineName}:\n${clientRecordDumpStr}\n`);
}
// Dump the server records if gotten them already.
if (serverRecordDumpStr) {
- Logger.logInfo("Server forms records:\n" + serverRecordDumpStr + "\n");
+ Logger.logInfo(`Server state for ${engineName}:\n${serverRecordDumpStr}\n`);
}
- this.DumpError("Form validation failed", e);
+ this.DumpError(`Validation failed for ${engineName}`, e);
}
- Logger.logInfo("Form validation finished");
+ Logger.logInfo(`Validation finished for ${engineName}`);
+ },
+
+ ValidatePasswords() {
+ return this.ValidateCollection("passwords", PasswordValidator);
+ },
+
+ ValidateForms() {
+ return this.ValidateCollection("forms", FormValidator);
+ },
+
+ ValidateAddons() {
+ return this.ValidateCollection("addons", AddonValidator);
},
RunNextTestAction: function() {
try {
if (this._currentAction >=
this._phaselist[this._currentPhase].length) {
// Run necessary validations and then finish up
if (this.shouldValidateBookmarks) {
this.ValidateBookmarks();
}
if (this.shouldValidatePasswords) {
this.ValidatePasswords();
}
if (this.shouldValidateForms) {
this.ValidateForms();
}
+ if (this.shouldValidateAddons) {
+ this.ValidateAddons();
+ }
// we're all done
Logger.logInfo("test phase " + this._currentPhase + ": " +
(this._errors ? "FAIL" : "PASS"));
this._phaseFinished = true;
this.quit();
return;
}
@@ -1132,16 +1136,19 @@ var Addons = {
TPS.HandleAddons(addons, ACTION_DELETE);
},
verify: function Addons__verify(addons, state) {
TPS.HandleAddons(addons, ACTION_VERIFY, state);
},
verifyNot: function Addons__verifyNot(addons) {
TPS.HandleAddons(addons, ACTION_VERIFY_NOT);
},
+ skipValidation() {
+ TPS.shouldValidateAddons = false;
+ }
};
var Bookmarks = {
add: function Bookmarks__add(bookmarks) {
TPS.HandleBookmarks(bookmarks, ACTION_ADD);
},
modify: function Bookmarks__modify(bookmarks) {
TPS.HandleBookmarks(bookmarks, ACTION_MODIFY);