From 09061b2d8c28309874140049eb645d560987eeb3 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Sat, 28 Jul 2012 18:19:27 -0700 Subject: [PATCH 1/7] Bug 762606 - Another attempt at stopping tests finishing too soon. r=orange --- .../sync/tests/unit/test_resource_async.js | 124 ++++++++++-------- 1 file changed, 67 insertions(+), 57 deletions(-) diff --git a/services/sync/tests/unit/test_resource_async.js b/services/sync/tests/unit/test_resource_async.js index 34f80ebc093..fcc1f0921e6 100644 --- a/services/sync/tests/unit/test_resource_async.js +++ b/services/sync/tests/unit/test_resource_async.js @@ -155,12 +155,77 @@ let quotaValue; Observers.add("weave:service:quota:remaining", function (subject) { quotaValue = subject; }); -let server; - function run_test() { logger = Log4Moz.repository.getLogger('Test'); Log4Moz.repository.rootLogger.addAppender(new Log4Moz.DumpAppender()); + Svc.Prefs.set("network.numRetries", 1); // speed up test + run_next_test(); +} + +// This apparently has to come first in order for our PAC URL to be hit. +// Don't put any other HTTP requests earlier in the file! +add_test(function test_proxy_auth_redirect() { + _("Ensure that a proxy auth redirect (which switches out our channel) " + + "doesn't break AsyncResource."); + let server = httpd_setup({ + "/open": server_open, + "/pac2": server_pac + }); + + PACSystemSettings.PACURI = "http://localhost:8080/pac2"; + installFakePAC(); + let res = new AsyncResource("http://localhost:8080/open"); + res.get(function (error, result) { + do_check_true(!error); + do_check_true(pacFetched); + do_check_true(fetched); + do_check_eq("This path exists", result); + pacFetched = fetched = false; + uninstallFakePAC(); + server.stop(run_next_test); + }); +}); + +add_test(function test_new_channel() { + _("Ensure a redirect to a new channel is handled properly."); + + let resourceRequested = false; + function resourceHandler(metadata, response) { + resourceRequested = true; + + let body = "Test"; + response.setHeader("Content-Type", "text/plain"); + response.bodyOutputStream.write(body, body.length); + } + + function redirectHandler(metadata, response) { + let body = "Redirecting"; + response.setStatusLine(metadata.httpVersion, 307, "TEMPORARY REDIRECT"); + response.setHeader("Location", "http://localhost:8080/resource"); + response.bodyOutputStream.write(body, body.length); + } + + let server = httpd_setup({"/resource": resourceHandler, + "/redirect": redirectHandler}, + 8080); + + let request = new AsyncResource("http://localhost:8080/redirect"); + request.get(function onRequest(error, content) { + do_check_null(error); + do_check_true(resourceRequested); + do_check_eq(200, content.status); + do_check_true("content-type" in content.headers); + do_check_eq("text/plain", content.headers["content-type"]); + + server.stop(run_next_test); + }); +}); + + +let server; + +add_test(function setup() { server = httpd_setup({ "/open": server_open, "/protected": server_protected, @@ -176,27 +241,7 @@ function run_test() { "/quota-error": server_quota_error }); - Svc.Prefs.set("network.numRetries", 1); // speed up test run_next_test(); -} - -// This apparently has to come first in order for our PAC URL to be hit. -// Don't put any other HTTP requests earlier in the file! -add_test(function test_proxy_auth_redirect() { - _("Ensure that a proxy auth redirect (which switches out our channel) " + - "doesn't break AsyncResource."); - PACSystemSettings.PACURI = "http://localhost:8080/pac2"; - installFakePAC(); - let res = new AsyncResource("http://localhost:8080/open"); - res.get(function (error, result) { - do_check_true(!error); - do_check_true(pacFetched); - do_check_true(fetched); - do_check_eq("This path exists", result); - pacFetched = fetched = false; - uninstallFakePAC(); - run_next_test(); - }); }); add_test(function test_members() { @@ -662,38 +707,3 @@ add_test(function test_uri_construction() { add_test(function eliminate_server() { server.stop(run_next_test); }); - -add_test(function test_new_channel() { - _("Ensure a redirect to a new channel is handled properly."); - - let resourceRequested = false; - function resourceHandler(metadata, response) { - resourceRequested = true; - - let body = "Test"; - response.setHeader("Content-Type", "text/plain"); - response.bodyOutputStream.write(body, body.length); - } - - function redirectHandler(metadata, response) { - let body = "Redirecting"; - response.setStatusLine(metadata.httpVersion, 307, "TEMPORARY REDIRECT"); - response.setHeader("Location", "http://localhost:8080/resource"); - response.bodyOutputStream.write(body, body.length); - } - - let server = httpd_setup({"/resource": resourceHandler, - "/redirect": redirectHandler}, - 8080); - - let request = new AsyncResource("http://localhost:8080/redirect"); - request.get(function onRequest(error, content) { - do_check_null(error); - do_check_true(resourceRequested); - do_check_eq(200, content.status); - do_check_true("content-type" in content.headers); - do_check_eq("text/plain", content.headers["content-type"]); - - server.stop(run_next_test); - }); -}); From 573ed476585498a3d7c4541199a7538fd6affc58 Mon Sep 17 00:00:00 2001 From: Sankha Narayan Guria Date: Mon, 30 Jul 2012 16:05:20 -0700 Subject: [PATCH 2/7] Bug 579604 - Save prefs file after setting up Sync; r=gps --- services/sync/modules/policies.js | 1 + 1 file changed, 1 insertion(+) diff --git a/services/sync/modules/policies.js b/services/sync/modules/policies.js index 7c8e49f1fda..d55501c85a9 100644 --- a/services/sync/modules/policies.js +++ b/services/sync/modules/policies.js @@ -193,6 +193,7 @@ let SyncScheduler = { } break; case "weave:service:setup-complete": + Services.prefs.savePrefFile(null); Svc.Idle.addIdleObserver(this, Svc.Prefs.get("scheduler.idleTime")); break; case "weave:service:start-over": From ec9dee356d98ef6ef24124b85aab4af9e89cc306 Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Mon, 30 Jul 2012 17:05:33 -0700 Subject: [PATCH 3/7] Bug 777989 - Move add-on helper functions out of add-ons engine; r=rnewman --- services/sync/modules/addonutils.js | 472 ++++++++++++++++++ services/sync/modules/engines/addons.js | 417 +--------------- services/sync/tests/unit/test_addon_utils.js | 161 ++++++ services/sync/tests/unit/test_addons_store.js | 128 +---- services/sync/tests/unit/test_load_modules.js | 47 +- services/sync/tests/unit/xpcshell.ini | 1 + 6 files changed, 668 insertions(+), 558 deletions(-) create mode 100644 services/sync/modules/addonutils.js create mode 100644 services/sync/tests/unit/test_addon_utils.js diff --git a/services/sync/modules/addonutils.js b/services/sync/modules/addonutils.js new file mode 100644 index 00000000000..a6625d599b8 --- /dev/null +++ b/services/sync/modules/addonutils.js @@ -0,0 +1,472 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +"use strict"; + +const EXPORTED_SYMBOLS = ["AddonUtils"]; + +const {interfaces: Ci, utils: Cu} = Components; + +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +Cu.import("resource://services-common/log4moz.js"); + +XPCOMUtils.defineLazyModuleGetter(this, "AddonManager", + "resource://gre/modules/AddonManager.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "AddonRepository", + "resource://gre/modules/AddonRepository.jsm"); + +function AddonUtilsInternal() { + this._log = Log4Moz.repository.getLogger("Sync.AddonUtils"); +} +AddonUtilsInternal.prototype = { + /** + * Obtain an AddonInstall object from an AddonSearchResult instance. + * + * The callback will be invoked with the result of the operation. The + * callback receives 2 arguments, error and result. Error will be falsy + * on success or some kind of error value otherwise. The result argument + * will be an AddonInstall on success or null on failure. It is possible + * for the error to be falsy but result to be null. This could happen if + * an install was not found. + * + * @param addon + * AddonSearchResult to obtain install from. + * @param cb + * Function to be called with result of operation. + */ + getInstallFromSearchResult: + function getInstallFromSearchResult(addon, cb, requireSecureURI=true) { + + this._log.debug("Obtaining install for " + addon.id); + + // Verify that the source URI uses TLS. We don't allow installs from + // insecure sources for security reasons. The Addon Manager ensures that + // cert validation, etc is performed. + if (requireSecureURI) { + let scheme = addon.sourceURI.scheme; + if (scheme != "https") { + cb(new Error("Insecure source URI scheme: " + scheme), addon.install); + return; + } + } + + // We should theoretically be able to obtain (and use) addon.install if + // it is available. However, the addon.sourceURI rewriting won't be + // reflected in the AddonInstall, so we can't use it. If we ever get rid + // of sourceURI rewriting, we can avoid having to reconstruct the + // AddonInstall. + AddonManager.getInstallForURL( + addon.sourceURI.spec, + function handleInstall(install) { + cb(null, install); + }, + "application/x-xpinstall", + undefined, + addon.name, + 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. + * requireSecureURI - Boolean indicating whether to require a secure + * URI to install from. This defaults to true. + * + * 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, options, cb) { + this._log.info("Trying to install add-on from search result: " + addon.id); + + if (options.requireSecureURI === undefined) { + options.requireSecureURI = true; + } + + this.getInstallFromSearchResult(addon, function onResult(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 onInstallStarted(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); + + cb(new Error("Install failed: " + install.error), null); + }, + onDownloadFailed: function(install) { + install.removeListener(listener); + + cb(new Error("Download failed: " + install.error), null); + } + }; + install.addListener(listener); + install.install(); + } + catch (ex) { + this._log.error("Error installing add-on: " + Utils.exceptionstr(ex)); + cb(ex, null); + } + }.bind(this), options.requireSecureURI); + }, + + /** + * Uninstalls the Addon instance and invoke a callback when it is done. + * + * @param addon + * Addon instance to uninstall. + * @param cb + * Function to be invoked when uninstall has finished. It receives a + * truthy value signifying error and the add-on which was uninstalled. + */ + uninstallAddon: function uninstallAddon(addon, cb) { + let listener = { + onUninstalling: function(uninstalling, needsRestart) { + if (addon.id != uninstalling.id) { + return; + } + + // We assume restartless add-ons will send the onUninstalled event + // soon. + if (!needsRestart) { + return; + } + + // For non-restartless add-ons, we issue the callback on uninstalling + // because we will likely never see the uninstalled event. + AddonManager.removeAddonListener(listener); + cb(null, addon); + }, + onUninstalled: function(uninstalled) { + if (addon.id != uninstalled.id) { + return; + } + + AddonManager.removeAddonListener(listener); + cb(null, addon); + } + }; + AddonManager.addAddonListener(listener); + addon.uninstall(); + }, + + /** + * 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. + * requireSecureURI - Boolean indicating whether to require a secure + * URI when installing from a remote location. This defaults to + * true. + * + * 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 installs + * Array of objects describing add-ons to install. + * @param cb + * Function to be called when all actions are complete. + */ + 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: [], + addons: [], + errors: [] + }; + + if (!addonsLength) { + cb(null, ourResult); + return; + } + + let expectedInstallCount = 0; + let finishedCount = 0; + let installCallback = function installCallback(error, result) { + finishedCount++; + + if (error) { + ourResult.errors.push(error); + } else { + ourResult.installedIDs.push(result.id); + ourResult.installs.push(result.install); + ourResult.addons.push(result.addon); + } + + if (finishedCount >= expectedInstallCount) { + if (ourResult.errors.length > 0) { + cb(new Error("1 or more add-ons failed to install"), ourResult); + } else { + cb(null, ourResult); + } + } + }.bind(this); + + let toInstall = []; + + // Rewrite the "src" query string parameter of the source URI to note + // that the add-on was installed by Sync and not something else so + // server-side metrics aren't skewed (bug 708134). The server should + // ideally send proper URLs, but this solution was deemed too + // complicated at the time the functionality was implemented. + for each (let addon in addons) { + // sourceURI presence isn't enforced by AddonRepository. So, we skip + // add-ons without a sourceURI. + if (!addon.sourceURI) { + this._log.info("Skipping install of add-on because missing " + + "sourceURI: " + addon.id); + continue; + } + + toInstall.push(addon); + + // We should always be able to QI the nsIURI to nsIURL. If not, we + // still try to install the add-on, but we don't rewrite the URL, + // potentially skewing metrics. + try { + addon.sourceURI.QueryInterface(Ci.nsIURL); + } catch (ex) { + this._log.warn("Unable to QI sourceURI to nsIURL: " + + addon.sourceURI.spec); + continue; + } + + let params = addon.sourceURI.query.split("&").map( + function rewrite(param) { + + if (param.indexOf("src=") == 0) { + return "src=sync"; + } else { + return param; + } + }); + + addon.sourceURI.query = params.join("&"); + } + + expectedInstallCount = toInstall.length; + + 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) { + 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); + }, + }); + }, + + /** + * Update the user disabled flag for an add-on. + * + * The supplied callback will ba called when the operation is + * complete. If the new flag matches the existing or if the add-on + * isn't currently active, the function will fire the callback + * immediately. Else, the callback is invoked when the AddonManager + * reports the change has taken effect or has been registered. + * + * The callback receives as arguments: + * + * (Error) Encountered error during operation or null on success. + * (Addon) The add-on instance being operated on. + * + * @param addon + * (Addon) Add-on instance to operate on. + * @param value + * (bool) New value for add-on's userDisabled property. + * @param cb + * (function) Callback to be invoked on completion. + */ + updateUserDisabled: function updateUserDisabled(addon, value, cb) { + if (addon.userDisabled == value) { + cb(null, addon); + return; + } + + let listener = { + onEnabling: function onEnabling(wrapper, needsRestart) { + this._log.debug("onEnabling: " + wrapper.id); + if (wrapper.id != addon.id) { + return; + } + + // We ignore the restartless case because we'll get onEnabled shortly. + if (!needsRestart) { + return; + } + + AddonManager.removeAddonListener(listener); + cb(null, wrapper); + }.bind(this), + + onEnabled: function onEnabled(wrapper) { + this._log.debug("onEnabled: " + wrapper.id); + if (wrapper.id != addon.id) { + return; + } + + AddonManager.removeAddonListener(listener); + cb(null, wrapper); + }.bind(this), + + onDisabling: function onDisabling(wrapper, needsRestart) { + this._log.debug("onDisabling: " + wrapper.id); + if (wrapper.id != addon.id) { + return; + } + + if (!needsRestart) { + return; + } + + AddonManager.removeAddonListener(listener); + cb(null, wrapper); + }.bind(this), + + onDisabled: function onDisabled(wrapper) { + this._log.debug("onDisabled: " + wrapper.id); + if (wrapper.id != addon.id) { + return; + } + + AddonManager.removeAddonListener(listener); + cb(null, wrapper); + }.bind(this), + + onOperationCancelled: function onOperationCancelled(wrapper) { + this._log.debug("onOperationCancelled: " + wrapper.id); + if (wrapper.id != addon.id) { + return; + } + + AddonManager.removeAddonListener(listener); + cb(new Error("Operation cancelled"), wrapper); + }.bind(this) + }; + + // The add-on listeners are only fired if the add-on is active. If not, the + // change is silently updated and made active when/if the add-on is active. + + if (!addon.appDisabled) { + AddonManager.addAddonListener(listener); + } + + this._log.info("Updating userDisabled flag: " + addon.id + " -> " + value); + addon.userDisabled = !!value; + + if (!addon.appDisabled) { + cb(null, addon); + return; + } + // Else the listener will handle invoking the callback. + }, + +}; + +XPCOMUtils.defineLazyGetter(this, "AddonUtils", function() { + return new AddonUtilsInternal(); +}); diff --git a/services/sync/modules/engines/addons.js b/services/sync/modules/engines/addons.js index 9294f2ab146..0e80e3fffff 100644 --- a/services/sync/modules/engines/addons.js +++ b/services/sync/modules/engines/addons.js @@ -35,6 +35,7 @@ const {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"); @@ -291,10 +292,11 @@ AddonsStore.prototype = { */ create: function create(record) { let cb = Async.makeSpinningCallback(); - this.installAddons([{ - id: record.addonID, - syncGUID: record.id, - enabled: record.enabled + AddonUtils.installAddons([{ + id: record.addonID, + syncGUID: record.id, + enabled: record.enabled, + requireSecureURI: !Svc.Prefs.get("addons.ignoreRepositoryChecking", false), }], cb); // This will throw if there was an error. This will get caught by the sync @@ -332,7 +334,7 @@ AddonsStore.prototype = { this._log.info("Uninstalling add-on: " + addon.id); let cb = Async.makeSpinningCallback(); - this.uninstallAddon(addon, cb); + AddonUtils.uninstallAddon(addon, cb); cb.wait(); }, @@ -607,185 +609,6 @@ AddonsStore.prototype = { return true; }, - /** - * Obtain an AddonInstall object from an AddonSearchResult instance. - * - * The callback will be invoked with the result of the operation. The - * callback receives 2 arguments, error and result. Error will be falsy - * on success or some kind of error value otherwise. The result argument - * will be an AddonInstall on success or null on failure. It is possible - * for the error to be falsy but result to be null. This could happen if - * an install was not found. - * - * @param addon - * AddonSearchResult to obtain install from. - * @param cb - * Function to be called with result of operation. - */ - getInstallFromSearchResult: function getInstallFromSearchResult(addon, cb) { - // We should theoretically be able to obtain (and use) addon.install if - // it is available. However, the addon.sourceURI rewriting won't be - // reflected in the AddonInstall, so we can't use it. If we ever get rid - // of sourceURI rewriting, we can avoid having to reconstruct the - // AddonInstall. - this._log.debug("Obtaining install for " + addon.id); - - // Verify that the source URI uses TLS. We don't allow installs from - // insecure sources for security reasons. The Addon Manager ensures that - // cert validation, etc is performed. - if (!Svc.Prefs.get("addons.ignoreRepositoryChecking", false)) { - let scheme = addon.sourceURI.scheme; - if (scheme != "https") { - cb(new Error("Insecure source URI scheme: " + scheme), addon.install); - } - } - - AddonManager.getInstallForURL( - addon.sourceURI.spec, - function handleInstall(install) { - cb(null, install); - }, - "application/x-xpinstall", - undefined, - addon.name, - 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, 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); - - cb(new Error("Install failed: " + install.error), null); - }, - onDownloadFailed: function(install) { - install.removeListener(listener); - - cb(new Error("Download failed: " + install.error), null); - } - }; - install.addListener(listener); - install.install(); - } - catch (ex) { - this._log.error("Error installing add-on: " + Utils.exceptionstr(ex)); - cb(ex, null); - } - }.bind(this)); - }, - - /** - * Uninstalls the Addon instance and invoke a callback when it is done. - * - * @param addon - * Addon instance to uninstall. - * @param callback - * Function to be invoked when uninstall has finished. It receives a - * truthy value signifying error and the add-on which was uninstalled. - */ - uninstallAddon: function uninstallAddon(addon, callback) { - let listener = { - onUninstalling: function(uninstalling, needsRestart) { - if (addon.id != uninstalling.id) { - return; - } - - // We assume restartless add-ons will send the onUninstalled event - // soon. - if (!needsRestart) { - return; - } - - // For non-restartless add-ons, we issue the callback on uninstalling - // because we will likely never see the uninstalled event. - AddonManager.removeAddonListener(listener); - callback(null, addon); - }, - onUninstalled: function(uninstalled) { - if (addon.id != uninstalled.id) { - return; - } - - AddonManager.removeAddonListener(listener); - callback(null, addon); - } - }; - AddonManager.addAddonListener(listener); - addon.uninstall(); - }, - /** * Update the userDisabled flag on an add-on. * @@ -816,232 +639,8 @@ AddonsStore.prototype = { return; } - let listener = { - onEnabling: function onEnabling(wrapper, needsRestart) { - this._log.debug("onEnabling: " + wrapper.id); - if (wrapper.id != addon.id) { - return; - } - - // We ignore the restartless case because we'll get onEnabled shortly. - if (!needsRestart) { - return; - } - - AddonManager.removeAddonListener(listener); - callback(null, wrapper); - }.bind(this), - - onEnabled: function onEnabled(wrapper) { - this._log.debug("onEnabled: " + wrapper.id); - if (wrapper.id != addon.id) { - return; - } - - AddonManager.removeAddonListener(listener); - callback(null, wrapper); - }.bind(this), - - onDisabling: function onDisabling(wrapper, needsRestart) { - this._log.debug("onDisabling: " + wrapper.id); - if (wrapper.id != addon.id) { - return; - } - - if (!needsRestart) { - return; - } - - AddonManager.removeAddonListener(listener); - callback(null, wrapper); - }.bind(this), - - onDisabled: function onDisabled(wrapper) { - this._log.debug("onDisabled: " + wrapper.id); - if (wrapper.id != addon.id) { - return; - } - - AddonManager.removeAddonListener(listener); - callback(null, wrapper); - }.bind(this), - - onOperationCancelled: function onOperationCancelled(wrapper) { - this._log.debug("onOperationCancelled: " + wrapper.id); - if (wrapper.id != addon.id) { - return; - } - - AddonManager.removeAddonListener(listener); - callback(new Error("Operation cancelled"), wrapper); - }.bind(this) - }; - - // The add-on listeners are only fired if the add-on is active. If not, the - // change is silently updated and made active when/if the add-on is active. - - if (!addon.appDisabled) { - AddonManager.addAddonListener(listener); - } - - this._log.info("Updating userDisabled flag: " + addon.id + " -> " + value); - addon.userDisabled = !!value; - - if (!addon.appDisabled) { - callback(null, addon); - return; - } - // Else the listener will handle invoking the callback. + AddonUtils.updateUserDisabled(addon, value, callback); }, - - /** - * 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 installs - * Array of objects describing add-ons to install. - * @param cb - * Function to be called when all actions are complete. - */ - 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: [], - addons: [], - errors: [] - }; - - if (!addonsLength) { - cb(null, ourResult); - return; - } - - let expectedInstallCount = 0; - let finishedCount = 0; - let installCallback = function installCallback(error, result) { - finishedCount++; - - if (error) { - ourResult.errors.push(error); - } else { - ourResult.installedIDs.push(result.id); - ourResult.installs.push(result.install); - ourResult.addons.push(result.addon); - } - - if (finishedCount >= expectedInstallCount) { - if (ourResult.errors.length > 0) { - cb(new Error("1 or more add-ons failed to install"), ourResult); - } else { - cb(null, ourResult); - } - } - }.bind(this); - - let toInstall = []; - - // Rewrite the "src" query string parameter of the source URI to note - // that the add-on was installed by Sync and not something else so - // server-side metrics aren't skewed (bug 708134). The server should - // ideally send proper URLs, but this solution was deemed too - // complicated at the time the functionality was implemented. - for each (let addon in addons) { - // sourceURI presence isn't enforced by AddonRepository. So, we skip - // add-ons without a sourceURI. - if (!addon.sourceURI) { - this._log.info("Skipping install of add-on because missing " + - "sourceURI: " + addon.id); - continue; - } - - toInstall.push(addon); - - // We should always be able to QI the nsIURI to nsIURL. If not, we - // still try to install the add-on, but we don't rewrite the URL, - // potentially skewing metrics. - try { - addon.sourceURI.QueryInterface(Ci.nsIURL); - } catch (ex) { - this._log.warn("Unable to QI sourceURI to nsIURL: " + - addon.sourceURI.spec); - continue; - } - - let params = addon.sourceURI.query.split("&").map( - function rewrite(param) { - - if (param.indexOf("src=") == 0) { - return "src=sync"; - } else { - return param; - } - }); - - addon.sourceURI.query = params.join("&"); - } - - expectedInstallCount = toInstall.length; - - 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) { - 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) - }); - } }; /** diff --git a/services/sync/tests/unit/test_addon_utils.js b/services/sync/tests/unit/test_addon_utils.js new file mode 100644 index 00000000000..e083f3b641e --- /dev/null +++ b/services/sync/tests/unit/test_addon_utils.js @@ -0,0 +1,161 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +Cu.import("resource://services-sync/addonutils.js"); +Cu.import("resource://services-common/preferences.js"); + +const HTTP_PORT = 8888; +const SERVER_ADDRESS = "http://127.0.0.1:8888"; + +let prefs = new Preferences(); + +prefs.set("extensions.getAddons.get.url", + SERVER_ADDRESS + "/search/guid:%IDS%"); + +loadAddonTestFunctions(); +startupManager(); + +function createAndStartHTTPServer(port=HTTP_PORT) { + try { + let server = new HttpServer(); + + let bootstrap1XPI = ExtensionsTestPath("/addons/test_bootstrap1_1.xpi"); + + server.registerFile("/search/guid:missing-sourceuri%40tests.mozilla.org", + do_get_file("missing-sourceuri.xml")); + + server.registerFile("/search/guid:rewrite%40tests.mozilla.org", + do_get_file("rewrite-search.xml")); + + server.start(port); + + return server; + } catch (ex) { + _("Got exception starting HTTP server on port " + port); + _("Error: " + Utils.exceptionStr(ex)); + do_throw(ex); + } +} + +function run_test() { + initTestLogging("Trace"); + + run_next_test(); +} + +add_test(function test_handle_empty_source_uri() { + _("Ensure that search results without a sourceURI are properly ignored."); + + let server = createAndStartHTTPServer(); + + const ID = "missing-sourceuri@tests.mozilla.org"; + + let cb = Async.makeSpinningCallback(); + AddonUtils.installAddons([{id: ID, requireSecureURI: false}], cb); + let result = cb.wait(); + + do_check_true("installedIDs" in result); + do_check_eq(0, result.installedIDs.length); + + server.stop(run_next_test); +}); + +add_test(function test_ignore_untrusted_source_uris() { + _("Ensures that source URIs from insecure schemes are rejected."); + + let ioService = Cc["@mozilla.org/network/io-service;1"] + .getService(Ci.nsIIOService); + + const bad = ["http://example.com/foo.xpi", + "ftp://example.com/foo.xpi", + "silly://example.com/foo.xpi"]; + + const good = ["https://example.com/foo.xpi"]; + + for (let s of bad) { + let sourceURI = ioService.newURI(s, null, null); + let addon = {sourceURI: sourceURI, name: "bad", id: "bad"}; + + try { + let cb = Async.makeSpinningCallback(); + AddonUtils.getInstallFromSearchResult(addon, cb, true); + cb.wait(); + } catch (ex) { + do_check_neq(null, ex); + do_check_eq(0, ex.message.indexOf("Insecure source URI")); + continue; + } + + // We should never get here if an exception is thrown. + do_check_true(false); + } + + let count = 0; + for (let s of good) { + let sourceURI = ioService.newURI(s, null, null); + let addon = {sourceURI: sourceURI, name: "good", id: "good"}; + + // Despite what you might think, we don't get an error in the callback. + // The install won't work because the underlying Addon instance wasn't + // proper. But, that just results in an AddonInstall that is missing + // certain values. We really just care that the callback is being invoked + // anyway. + let callback = function onInstall(error, install) { + do_check_null(error); + do_check_neq(null, install); + do_check_eq(sourceURI.spec, install.sourceURI.spec); + + count += 1; + + if (count >= good.length) { + run_next_test(); + } + }; + + AddonUtils.getInstallFromSearchResult(addon, callback, true); + } +}); + +add_test(function test_source_uri_rewrite() { + _("Ensure that a 'src=api' query string is rewritten to 'src=sync'"); + + // This tests for conformance with bug 708134 so server-side metrics aren't + // skewed. + + Svc.Prefs.set("addons.ignoreRepositoryChecking", true); + + // We resort to monkeypatching because of the API design. + let oldFunction = AddonUtils.__proto__.installAddonFromSearchResult; + + let installCalled = false; + AddonUtils.__proto__.installAddonFromSearchResult = + function testInstallAddon(addon, metadata, cb) { + + do_check_eq(SERVER_ADDRESS + "/require.xpi?src=sync", + addon.sourceURI.spec); + + installCalled = true; + + AddonUtils.getInstallFromSearchResult(addon, function (error, install) { + do_check_null(error); + do_check_eq(SERVER_ADDRESS + "/require.xpi?src=sync", + install.sourceURI.spec); + + cb(null, {id: addon.id, addon: addon, install: install}); + }, false); + }; + + let server = createAndStartHTTPServer(); + + let installCallback = Async.makeSpinningCallback(); + AddonUtils.installAddons([{id: "rewrite@tests.mozilla.org"}], installCallback); + + installCallback.wait(); + do_check_true(installCalled); + AddonUtils.__proto__.installAddonFromSearchResult = oldFunction; + + Svc.Prefs.reset("addons.ignoreRepositoryChecking"); + server.stop(run_next_test); +}); diff --git a/services/sync/tests/unit/test_addons_store.js b/services/sync/tests/unit/test_addons_store.js index aa519eb21a6..a10d4938bd8 100644 --- a/services/sync/tests/unit/test_addons_store.js +++ b/services/sync/tests/unit/test_addons_store.js @@ -3,8 +3,9 @@ "use strict"; -Cu.import("resource://services-sync/engines/addons.js"); Cu.import("resource://services-common/preferences.js"); +Cu.import("resource://services-sync/addonutils.js"); +Cu.import("resource://services-sync/engines/addons.js"); const HTTP_PORT = 8888; @@ -53,12 +54,6 @@ function createAndStartHTTPServer(port) { server.registerFile("/search/guid:missing-xpi%40tests.mozilla.org", do_get_file("missing-xpi-search.xml")); - server.registerFile("/search/guid:rewrite%40tests.mozilla.org", - do_get_file("rewrite-search.xml")); - - server.registerFile("/search/guid:missing-sourceuri%40tests.mozilla.org", - do_get_file("missing-sourceuri.xml")); - server.start(port); return server; @@ -409,64 +404,6 @@ add_test(function test_ignore_hotfixes() { run_next_test(); }); -add_test(function test_ignore_untrusted_source_uris() { - _("Ensures that source URIs from insecure schemes are rejected."); - - Svc.Prefs.set("addons.ignoreRepositoryChecking", false); - - let ioService = Cc["@mozilla.org/network/io-service;1"] - .getService(Ci.nsIIOService); - - const bad = ["http://example.com/foo.xpi", - "ftp://example.com/foo.xpi", - "silly://example.com/foo.xpi"]; - - const good = ["https://example.com/foo.xpi"]; - - for each (let s in bad) { - let sourceURI = ioService.newURI(s, null, null); - let addon = {sourceURI: sourceURI, name: "foo"}; - - try { - let cb = Async.makeSpinningCallback(); - store.getInstallFromSearchResult(addon, cb); - cb.wait(); - } catch (ex) { - do_check_neq(null, ex); - do_check_eq(0, ex.message.indexOf("Insecure source URI")); - continue; - } - - // We should never get here if an exception is thrown. - do_check_true(false); - } - - let count = 0; - for each (let s in good) { - let sourceURI = ioService.newURI(s, null, null); - let addon = {sourceURI: sourceURI, name: "foo", id: "foo"}; - - // Despite what you might think, we don't get an error in the callback. - // The install won't work because the underlying Addon instance wasn't - // proper. But, that just results in an AddonInstall that is missing - // certain values. We really just care that the callback is being invoked - // anyway. - let callback = function(error, install) { - do_check_eq(null, error); - do_check_neq(null, install); - do_check_eq(sourceURI.spec, install.sourceURI.spec); - - count += 1; - - if (count >= good.length) { - run_next_test(); - } - }; - - store.getInstallFromSearchResult(addon, callback); - } -}); - add_test(function test_wipe() { _("Ensures that wiping causes add-ons to be uninstalled."); @@ -482,64 +419,3 @@ add_test(function test_wipe() { run_next_test(); }); - -add_test(function test_source_uri_rewrite() { - _("Ensure that a 'src=api' query string is rewritten to 'src=sync'"); - - // This tests for conformance with bug 708134 so server-side metrics aren't - // skewed. - - 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, 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", - install.sourceURI.spec); - - cb(null, {id: addon.id, addon: addon, install: install}); - }); - }; - - let server = createAndStartHTTPServer(HTTP_PORT); - - let installCallback = Async.makeSpinningCallback(); - 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); -}); - -add_test(function test_handle_empty_source_uri() { - _("Ensure that search results without a sourceURI are properly ignored."); - - Svc.Prefs.set("addons.ignoreRepositoryChecking", true); - - let server = createAndStartHTTPServer(HTTP_PORT); - - const ID = "missing-sourceuri@tests.mozilla.org"; - - let cb = Async.makeSpinningCallback(); - 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); -}); diff --git a/services/sync/tests/unit/test_load_modules.js b/services/sync/tests/unit/test_load_modules.js index f277358c0e5..830331d20cd 100644 --- a/services/sync/tests/unit/test_load_modules.js +++ b/services/sync/tests/unit/test_load_modules.js @@ -1,27 +1,28 @@ const modules = [ - "addonsreconciler.js", - "constants.js", - "engines/addons.js", - "engines/bookmarks.js", - "engines/clients.js", - "engines/forms.js", - "engines/history.js", - "engines/passwords.js", - "engines/prefs.js", - "engines/tabs.js", - "engines.js", - "identity.js", - "jpakeclient.js", - "keys.js", - "main.js", - "notifications.js", - "policies.js", - "record.js", - "resource.js", - "rest.js", - "service.js", - "status.js", - "util.js", + "addonutils.js", + "addonsreconciler.js", + "constants.js", + "engines/addons.js", + "engines/bookmarks.js", + "engines/clients.js", + "engines/forms.js", + "engines/history.js", + "engines/passwords.js", + "engines/prefs.js", + "engines/tabs.js", + "engines.js", + "identity.js", + "jpakeclient.js", + "keys.js", + "main.js", + "notifications.js", + "policies.js", + "record.js", + "resource.js", + "rest.js", + "service.js", + "status.js", + "util.js", ]; function run_test() { diff --git a/services/sync/tests/unit/xpcshell.ini b/services/sync/tests/unit/xpcshell.ini index 4d27d73fbbf..4c5e262a988 100644 --- a/services/sync/tests/unit/xpcshell.ini +++ b/services/sync/tests/unit/xpcshell.ini @@ -25,6 +25,7 @@ tail = [test_utils_passphrase.js] # We have a number of other libraries that are pretty much standalone. +[test_addon_utils.js] [test_httpd_sync_server.js] [test_jpakeclient.js] # Bug 618233: this test produces random failures on Windows 7. From 1250f9ad596cbe982de5420c4e9d80bfbca33add Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Tue, 31 Jul 2012 18:12:12 -0700 Subject: [PATCH 4/7] Bug 777989 - Update TPS to use new AddonUtils module; r=rnewman --- .../sync/tps/extensions/tps/modules/addons.jsm | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/services/sync/tps/extensions/tps/modules/addons.jsm b/services/sync/tps/extensions/tps/modules/addons.jsm index 010c534b864..24d4fa86b93 100644 --- a/services/sync/tps/extensions/tps/modules/addons.jsm +++ b/services/sync/tps/extensions/tps/modules/addons.jsm @@ -11,7 +11,7 @@ Cu.import("resource://gre/modules/AddonManager.jsm"); Cu.import("resource://gre/modules/AddonRepository.jsm"); Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://services-common/async.js"); -Cu.import("resource://services-sync/engines.js"); +Cu.import("resource://services-sync/addonutils.js"); Cu.import("resource://services-sync/util.js"); Cu.import("resource://tps/logger.jsm"); @@ -59,8 +59,7 @@ Addon.prototype = { Logger.AssertTrue(!!addon, 'could not find addon ' + this.id + ' to uninstall'); cb = Async.makeSpinningCallback(); - let store = Engines.get("addons")._store; - store.uninstallAddon(addon, cb); + AddonUtils.uninstallAddon(addon, cb); cb.wait(); }, @@ -97,11 +96,7 @@ Addon.prototype = { // 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.installAddons([{id: this.id}], cb); + AddonUtils.installAddons([{id: this.id}], cb); let result = cb.wait(); Logger.AssertEqual(1, result.installedIDs.length, "Exactly 1 add-on was installed."); @@ -121,9 +116,8 @@ Addon.prototype = { throw new Error("Unknown flag to setEnabled: " + flag); } - let store = Engines.get("addons")._store; let cb = Async.makeSpinningCallback(); - store.updateUserDisabled(this.addon, userDisabled, cb); + AddonUtils.updateUserDisabled(this.addon, userDisabled, cb); cb.wait(); return true; From c675bfe933a65dd7b1421a5371127d303e855a5d Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Wed, 1 Aug 2012 12:02:48 -0700 Subject: [PATCH 5/7] Bug 777989 - Make Add-on sync TPS tests work again; r=rnewman --- services/sync/modules/addonutils.js | 2 ++ services/sync/services-sync.js | 1 + services/sync/tps/extensions/tps/modules/addons.jsm | 2 +- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/services/sync/modules/addonutils.js b/services/sync/modules/addonutils.js index a6625d599b8..ea693a20fa6 100644 --- a/services/sync/modules/addonutils.js +++ b/services/sync/modules/addonutils.js @@ -10,6 +10,7 @@ const {interfaces: Ci, utils: Cu} = Components; Cu.import("resource://gre/modules/XPCOMUtils.jsm"); Cu.import("resource://services-common/log4moz.js"); +Cu.import("resource://services-sync/util.js"); XPCOMUtils.defineLazyModuleGetter(this, "AddonManager", "resource://gre/modules/AddonManager.jsm"); @@ -18,6 +19,7 @@ XPCOMUtils.defineLazyModuleGetter(this, "AddonRepository", function AddonUtilsInternal() { this._log = Log4Moz.repository.getLogger("Sync.AddonUtils"); + this._log.Level = Log4Moz.Level[Svc.Prefs.get("log.logger.addonutils")]; } AddonUtilsInternal.prototype = { /** diff --git a/services/sync/services-sync.js b/services/sync/services-sync.js index 80b1edbc564..da8a152d6ba 100644 --- a/services/sync/services-sync.js +++ b/services/sync/services-sync.js @@ -54,6 +54,7 @@ pref("services.sync.log.appender.file.logOnError", true); pref("services.sync.log.appender.file.logOnSuccess", false); pref("services.sync.log.appender.file.maxErrorAge", 864000); // 10 days pref("services.sync.log.rootLogger", "Debug"); +pref("services.sync.log.logger.addonutils", "Debug"); pref("services.sync.log.logger.service.main", "Debug"); pref("services.sync.log.logger.status", "Debug"); pref("services.sync.log.logger.authenticator", "Debug"); diff --git a/services/sync/tps/extensions/tps/modules/addons.jsm b/services/sync/tps/extensions/tps/modules/addons.jsm index 24d4fa86b93..69cc43c1705 100644 --- a/services/sync/tps/extensions/tps/modules/addons.jsm +++ b/services/sync/tps/extensions/tps/modules/addons.jsm @@ -96,7 +96,7 @@ Addon.prototype = { // for the addon's install .xml; we'll read the actual id from the .xml. let cb = Async.makeSpinningCallback(); - AddonUtils.installAddons([{id: this.id}], cb); + AddonUtils.installAddons([{id: this.id, requireSecureURI: false}], cb); let result = cb.wait(); Logger.AssertEqual(1, result.installedIDs.length, "Exactly 1 add-on was installed."); From bf3e935a56d30e8ab8aff8d8d0f2d2860c4aa6c2 Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Wed, 1 Aug 2012 12:05:29 -0700 Subject: [PATCH 6/7] Bug 779391 - Move deepCopy into CommonUtils; r=rnewman --- services/common/Makefile.in | 7 +++- services/common/modules-testing/utils.js | 42 +++++++++++++++++++ .../tests/unit/test_utils_deepCopy.js | 7 +++- services/common/tests/unit/xpcshell.ini | 1 + services/sync/tests/unit/head_helpers.js | 23 ---------- .../tests/unit/test_bookmark_livemarks.js | 7 +++- services/sync/tests/unit/test_tab_store.js | 6 ++- services/sync/tests/unit/xpcshell.ini | 1 - 8 files changed, 64 insertions(+), 30 deletions(-) create mode 100644 services/common/modules-testing/utils.js rename services/{sync => common}/tests/unit/test_utils_deepCopy.js (66%) diff --git a/services/common/Makefile.in b/services/common/Makefile.in index 98f6729889e..f29cebc9ca6 100644 --- a/services/common/Makefile.in +++ b/services/common/Makefile.in @@ -32,7 +32,12 @@ libs:: TEST_DIRS += tests -TESTING_JS_MODULES := aitcserver.js storageserver.js +TESTING_JS_MODULES := \ + aitcserver.js \ + storageserver.js \ + modules-testing/utils.js \ + $(NULL) + TESTING_JS_MODULE_DIR := services-common # What follows is a helper to launch a standalone storage server instance. diff --git a/services/common/modules-testing/utils.js b/services/common/modules-testing/utils.js new file mode 100644 index 00000000000..a588b78ddff --- /dev/null +++ b/services/common/modules-testing/utils.js @@ -0,0 +1,42 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +"use strict"; + +const EXPORTED_SYMBOLS = [ + "TestingUtils", +]; + +let TestingUtils = { + /** + * Perform a deep copy of an Array or Object. + */ + deepCopy: function deepCopy(thing, noSort) { + if (typeof(thing) != "object" || thing == null) { + return thing; + } + + if (Array.isArray(thing)) { + let ret = []; + for (let element of thing) { + ret.push(this.deepCopy(element, noSort)); + } + + return ret; + } + + let ret = {}; + let props = [p for (p in thing)]; + + if (!noSort) { + props = props.sort(); + } + + for (let prop of props) { + ret[prop] = this.deepCopy(thing[prop], noSort); + } + + return ret; + }, +}; diff --git a/services/sync/tests/unit/test_utils_deepCopy.js b/services/common/tests/unit/test_utils_deepCopy.js similarity index 66% rename from services/sync/tests/unit/test_utils_deepCopy.js rename to services/common/tests/unit/test_utils_deepCopy.js index 54454097e74..19003ee520a 100644 --- a/services/sync/tests/unit/test_utils_deepCopy.js +++ b/services/common/tests/unit/test_utils_deepCopy.js @@ -1,8 +1,11 @@ -Cu.import("resource://services-sync/util.js"); +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +Cu.import("resource://testing-common/services-common/utils.js"); function run_test() { let thing = {o: {foo: "foo", bar: ["bar"]}, a: ["foo", {bar: "bar"}]}; - let ret = deepCopy(thing); + let ret = TestingUtils.deepCopy(thing); do_check_neq(ret, thing) do_check_neq(ret.o, thing.o); do_check_neq(ret.o.bar, thing.o.bar); diff --git a/services/common/tests/unit/xpcshell.ini b/services/common/tests/unit/xpcshell.ini index 6a614e5d7ed..707ef274697 100644 --- a/services/common/tests/unit/xpcshell.ini +++ b/services/common/tests/unit/xpcshell.ini @@ -6,6 +6,7 @@ tail = [test_load_modules.js] [test_utils_atob.js] +[test_utils_deepCopy.js] [test_utils_encodeBase32.js] [test_utils_encodeBase64URL.js] [test_utils_json.js] diff --git a/services/sync/tests/unit/head_helpers.js b/services/sync/tests/unit/head_helpers.js index e2373c0dc1a..686f97f278a 100644 --- a/services/sync/tests/unit/head_helpers.js +++ b/services/sync/tests/unit/head_helpers.js @@ -380,26 +380,3 @@ RotaryEngine.prototype = { } } }; - -deepCopy: function deepCopy(thing, noSort) { - if (typeof(thing) != "object" || thing == null){ - return thing; - } - let ret; - - if (Array.isArray(thing)) { - ret = []; - for (let i = 0; i < thing.length; i++){ - ret.push(deepCopy(thing[i], noSort)); - } - } else { - ret = {}; - let props = [p for (p in thing)]; - if (!noSort){ - props = props.sort(); - } - props.forEach(function(k) ret[k] = deepCopy(thing[k], noSort)); - } - - return ret; -}; diff --git a/services/sync/tests/unit/test_bookmark_livemarks.js b/services/sync/tests/unit/test_bookmark_livemarks.js index 989dfc8847c..64287e6ee5b 100644 --- a/services/sync/tests/unit/test_bookmark_livemarks.js +++ b/services/sync/tests/unit/test_bookmark_livemarks.js @@ -1,11 +1,14 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + Cu.import("resource://services-common/log4moz.js"); Cu.import("resource://services-sync/record.js"); Cu.import("resource://services-sync/engines.js"); Cu.import("resource://services-sync/engines/bookmarks.js"); Cu.import("resource://services-sync/util.js"); - Cu.import("resource://services-sync/service.js"); Cu.import("resource://gre/modules/PlacesUtils.jsm"); +Cu.import("resource://testing-common/services-common/utils.js"); const DESCRIPTION_ANNO = "bookmarkProperties/description"; @@ -56,7 +59,7 @@ store.wipe(); function makeLivemark(p, mintGUID) { let b = new Livemark("bookmarks", p.id); // Copy here, because tests mutate the contents. - b.cleartext = deepCopy(p); + b.cleartext = TestingUtils.deepCopy(p); if (mintGUID) b.id = Utils.makeGUID(); diff --git a/services/sync/tests/unit/test_tab_store.js b/services/sync/tests/unit/test_tab_store.js index 26219027146..b3547b8995d 100644 --- a/services/sync/tests/unit/test_tab_store.js +++ b/services/sync/tests/unit/test_tab_store.js @@ -1,5 +1,9 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + Cu.import("resource://services-sync/engines/tabs.js"); Cu.import("resource://services-sync/util.js"); +Cu.import("resource://testing-common/services-common/utils.js"); function test_lastUsed() { let store = new TabEngine()._store; @@ -80,7 +84,7 @@ function fakeSessionSvc(url, numtabs) { if (numtabs) { let tabs = obj.windows[0].tabs; for (let i = 0; i < numtabs-1; i++) - tabs.push(deepCopy(tabs[0])); + tabs.push(TestingUtils.deepCopy(tabs[0])); } return JSON.stringify(obj); } diff --git a/services/sync/tests/unit/xpcshell.ini b/services/sync/tests/unit/xpcshell.ini index 4c5e262a988..6ae81469710 100644 --- a/services/sync/tests/unit/xpcshell.ini +++ b/services/sync/tests/unit/xpcshell.ini @@ -11,7 +11,6 @@ tail = # util contains a bunch of functionality used throughout. [test_utils_catch.js] -[test_utils_deepCopy.js] [test_utils_deepEquals.js] [test_utils_deferGetSet.js] [test_utils_deriveKey.js] From b2e6e4bdd517600afffbb58fa16b9398be1a75a5 Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Wed, 1 Aug 2012 13:50:55 -0700 Subject: [PATCH 7/7] Bug 779589 - Move services/common testing-only JS modules into own directory; r=rnewman --HG-- rename : services/common/aitcserver.js => services/common/modules-testing/aitcserver.js rename : services/common/storageserver.js => services/common/modules-testing/storageserver.js --- services/common/Makefile.in | 6 ++++-- services/common/{ => modules-testing}/aitcserver.js | 0 services/common/{ => modules-testing}/storageserver.js | 0 3 files changed, 4 insertions(+), 2 deletions(-) rename services/common/{ => modules-testing}/aitcserver.js (100%) rename services/common/{ => modules-testing}/storageserver.js (100%) diff --git a/services/common/Makefile.in b/services/common/Makefile.in index f29cebc9ca6..fb0b0648cc0 100644 --- a/services/common/Makefile.in +++ b/services/common/Makefile.in @@ -32,12 +32,14 @@ libs:: TEST_DIRS += tests -TESTING_JS_MODULES := \ +testing_modules := \ aitcserver.js \ storageserver.js \ - modules-testing/utils.js \ + utils.js \ $(NULL) +TESTING_JS_MODULES := $(foreach file,$(testing_modules),modules-testing/$(file)) + TESTING_JS_MODULE_DIR := services-common # What follows is a helper to launch a standalone storage server instance. diff --git a/services/common/aitcserver.js b/services/common/modules-testing/aitcserver.js similarity index 100% rename from services/common/aitcserver.js rename to services/common/modules-testing/aitcserver.js diff --git a/services/common/storageserver.js b/services/common/modules-testing/storageserver.js similarity index 100% rename from services/common/storageserver.js rename to services/common/modules-testing/storageserver.js