Bug 1525729 - Stop blocking extension startup on searchInitialized r=aswan

Depends on D23311

Differential Revision: https://phabricator.services.mozilla.com/D19701

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Rob Wu 2019-03-13 18:48:57 +00:00
Родитель 8fd0fcc7e8
Коммит f55e72505d
11 изменённых файлов: 258 добавлений и 59 удалений

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

@ -244,8 +244,8 @@ global.TabContext = class extends EventEmitter {
// This promise is used to wait for the search service to be initialized.
// None of the code in the WebExtension modules requests that initialization.
// It is assumed that it is started at some point. If tests start to fail
// because this promise never resolves, that's likely the cause.
// It is assumed that it is started at some point. That might never happen,
// e.g. if the application shuts down before the search service initializes.
XPCOMUtils.defineLazyGetter(global, "searchInitialized", () => {
if (Services.search.isInitialized) {
return Promise.resolve();

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

@ -79,6 +79,13 @@ async function handleInitialHomepagePopup(extensionId, homepageUrl) {
homepagePopup.addObserver(extensionId);
}
// When an extension starts up, a search engine may asynchronously be
// registered, without blocking the startup. When an extension is
// uninstalled, we need to wait for this registration to finish
// before running the uninstallation handler.
// Map[extension id -> Promise]
var pendingSearchSetupTasks = new Map();
this.chrome_settings_overrides = class extends ExtensionAPI {
static async processDefaultSearchSetting(action, id) {
await ExtensionSettingsStore.initialize();
@ -131,7 +138,11 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
]);
}
static onUninstall(id) {
static async onUninstall(id) {
let searchStartupPromise = pendingSearchSetupTasks.get(id);
if (searchStartupPromise) {
await searchStartupPromise;
}
// Note: We do not have to deal with homepage here as it is managed by
// the ExtensionPreferencesManager.
return Promise.all([
@ -190,68 +201,90 @@ this.chrome_settings_overrides = class extends ExtensionAPI {
});
}
if (manifest.chrome_settings_overrides.search_provider) {
await searchInitialized;
extension.callOnClose({
close: () => {
if (extension.shutdownReason == "ADDON_DISABLE") {
chrome_settings_overrides.processDefaultSearchSetting("disable", extension.id);
chrome_settings_overrides.removeEngine(extension.id);
// Registering a search engine can potentially take a long while,
// or not complete at all (when searchInitialized is never resolved),
// so we are deliberately not awaiting the returned promise here.
let searchStartupPromise =
this.processSearchProviderManifestEntry().finally(() => {
if (pendingSearchSetupTasks.get(extension.id) === searchStartupPromise) {
pendingSearchSetupTasks.delete(extension.id);
}
},
});
});
let searchProvider = manifest.chrome_settings_overrides.search_provider;
let engineName = searchProvider.name.trim();
if (searchProvider.is_default) {
// Save the promise so we can await at onUninstall.
pendingSearchSetupTasks.set(extension.id, searchStartupPromise);
}
}
async processSearchProviderManifestEntry() {
await searchInitialized;
let {extension} = this;
if (!extension) {
Cu.reportError(`Extension shut down before search provider was registered`);
return;
}
extension.callOnClose({
close: () => {
if (extension.shutdownReason == "ADDON_DISABLE") {
chrome_settings_overrides.processDefaultSearchSetting("disable", extension.id);
chrome_settings_overrides.removeEngine(extension.id);
}
},
});
let {manifest} = extension;
let searchProvider = manifest.chrome_settings_overrides.search_provider;
let engineName = searchProvider.name.trim();
if (searchProvider.is_default) {
let engine = Services.search.getEngineByName(engineName);
let defaultEngines = await Services.search.getDefaultEngines();
if (engine && defaultEngines.some(defaultEngine => defaultEngine.name == engineName)) {
// Needs to be called every time to handle reenabling, but
// only sets default for install or enable.
await this.setDefault(engineName);
// For built in search engines, we don't do anything further
return;
}
}
await this.addSearchEngine();
if (searchProvider.is_default) {
if (extension.startupReason === "ADDON_INSTALL") {
// Don't ask if it already the current engine
let engine = Services.search.getEngineByName(engineName);
let defaultEngines = await Services.search.getDefaultEngines();
if (engine && defaultEngines.some(defaultEngine => defaultEngine.name == engineName)) {
// Needs to be called every time to handle reenabling, but
// only sets default for install or enable.
await this.setDefault(engineName);
// For built in search engines, we don't do anything further
return;
}
}
await this.addSearchEngine();
if (searchProvider.is_default) {
if (extension.startupReason === "ADDON_INSTALL") {
// Don't ask if it already the current engine
let engine = Services.search.getEngineByName(engineName);
let defaultEngine = await Services.search.getDefault();
if (defaultEngine.name != engine.name) {
let subject = {
wrappedJSObject: {
// This is a hack because we don't have the browser of
// the actual install. This means the popup might show
// in a different window. Will be addressed in a followup bug.
browser: windowTracker.topWindow.gBrowser.selectedBrowser,
name: this.extension.name,
icon: this.extension.iconURL,
currentEngine: defaultEngine.name,
newEngine: engineName,
respond(allow) {
if (allow) {
ExtensionSettingsStore.addSetting(
extension.id, DEFAULT_SEARCH_STORE_TYPE, DEFAULT_SEARCH_SETTING_NAME, engineName, () => defaultEngine.name);
Services.search.defaultEngine = Services.search.getEngineByName(engineName);
}
},
let defaultEngine = await Services.search.getDefault();
if (defaultEngine.name != engine.name) {
let subject = {
wrappedJSObject: {
// This is a hack because we don't have the browser of
// the actual install. This means the popup might show
// in a different window. Will be addressed in a followup bug.
browser: windowTracker.topWindow.gBrowser.selectedBrowser,
name: this.extension.name,
icon: this.extension.iconURL,
currentEngine: defaultEngine.name,
newEngine: engineName,
respond(allow) {
if (allow) {
ExtensionSettingsStore.addSetting(
extension.id, DEFAULT_SEARCH_STORE_TYPE, DEFAULT_SEARCH_SETTING_NAME, engineName, () => defaultEngine.name);
Services.search.defaultEngine = Services.search.getEngineByName(engineName);
}
},
};
Services.obs.notifyObservers(subject, "webextension-defaultsearch-prompt");
}
} else {
// Needs to be called every time to handle reenabling, but
// only sets default for install or enable.
this.setDefault(engineName);
},
};
Services.obs.notifyObservers(subject, "webextension-defaultsearch-prompt");
}
} else if (ExtensionSettingsStore.hasSetting(extension.id,
DEFAULT_SEARCH_STORE_TYPE,
DEFAULT_SEARCH_SETTING_NAME)) {
// is_default has been removed, but we still have a setting. Remove it.
chrome_settings_overrides.processDefaultSearchSetting("removeSetting", extension.id);
} else {
// Needs to be called every time to handle reenabling, but
// only sets default for install or enable.
this.setDefault(engineName);
}
} else if (ExtensionSettingsStore.hasSetting(extension.id,
DEFAULT_SEARCH_STORE_TYPE,
DEFAULT_SEARCH_SETTING_NAME)) {
// is_default has been removed, but we still have a setting. Remove it.
chrome_settings_overrides.processDefaultSearchSetting("removeSetting", extension.id);
}
}

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

@ -2,9 +2,13 @@
/* vim: set sts=2 sw=2 et tw=80: */
"use strict";
const {AddonTestUtils} = ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm");
const SEARCH_TERM = "test";
const SEARCH_URL = "https://localhost/?q={searchTerms}";
AddonTestUtils.initMochitest(this);
add_task(async function test_search() {
async function background(SEARCH_TERM) {
function awaitSearchResult() {
@ -57,6 +61,7 @@ add_task(async function test_search() {
useAddonManager: "temporary",
});
await extension.startup();
await AddonTestUtils.waitForSearchProviderStartup(extension);
let addonEngines = await extension.awaitMessage("engines");
let engines = (await Services.search.getEngines()).filter(engine => !engine.hidden);

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

@ -6,9 +6,13 @@
ChromeUtils.defineModuleGetter(this, "AddonManager",
"resource://gre/modules/AddonManager.jsm");
const {AddonTestUtils} = ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm");
const EXTENSION1_ID = "extension1@mozilla.com";
const EXTENSION2_ID = "extension2@mozilla.com";
AddonTestUtils.initMochitest(this);
var defaultEngineName;
async function restoreDefaultEngine() {
@ -37,6 +41,7 @@ add_task(async function test_extension_setting_default_engine() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
is((await Services.search.getDefault()).name, "DuckDuckGo", "Default engine is DuckDuckGo");
@ -150,10 +155,12 @@ add_task(async function test_extension_setting_multiple_default_engine() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
is((await Services.search.getDefault()).name, "DuckDuckGo", "Default engine is DuckDuckGo");
await ext2.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext2);
is((await Services.search.getDefault()).name, "Twitter", "Default engine is Twitter");
@ -196,10 +203,12 @@ add_task(async function test_extension_setting_multiple_default_engine_reversed(
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
is((await Services.search.getDefault()).name, "DuckDuckGo", "Default engine is DuckDuckGo");
await ext2.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext2);
is((await Services.search.getDefault()).name, "Twitter", "Default engine is Twitter");
@ -229,6 +238,7 @@ add_task(async function test_user_changing_default_engine() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
is((await Services.search.getDefault()).name, "DuckDuckGo", "Default engine is DuckDuckGo");
@ -263,6 +273,7 @@ add_task(async function test_user_change_with_disabling() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
is((await Services.search.getDefault()).name, "DuckDuckGo", "Default engine is DuckDuckGo");
@ -328,6 +339,7 @@ add_task(async function test_two_addons_with_first_disabled_before_second() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
is((await Services.search.getDefault()).name, "DuckDuckGo", "Default engine is DuckDuckGo");
@ -339,6 +351,7 @@ add_task(async function test_two_addons_with_first_disabled_before_second() {
is((await Services.search.getDefault()).name, defaultEngineName, `Default engine is ${defaultEngineName}`);
await ext2.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext2);
is((await Services.search.getDefault()).name, "Twitter", "Default engine is Twitter");
@ -396,10 +409,12 @@ add_task(async function test_two_addons_with_first_disabled() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
is((await Services.search.getDefault()).name, "DuckDuckGo", "Default engine is DuckDuckGo");
await ext2.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext2);
is((await Services.search.getDefault()).name, "Twitter", "Default engine is Twitter");
@ -464,10 +479,12 @@ add_task(async function test_two_addons_with_second_disabled() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
is((await Services.search.getDefault()).name, "DuckDuckGo", "Default engine is DuckDuckGo");
await ext2.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext2);
is((await Services.search.getDefault()).name, "Twitter", "Default engine is Twitter");

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

@ -65,6 +65,7 @@ add_task(async function test_overrides_update_removal() {
let prefPromise = promisePrefChanged(HOMEPAGE_URI);
await extension.startup();
await AddonTestUtils.waitForSearchProviderStartup(extension);
await prefPromise;
equal(extension.version, "1.0", "The installed addon has the expected version.");

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

@ -42,6 +42,7 @@ add_task(async function test_extension_adding_engine() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
let engine = Services.search.getEngineByName("MozSearch");
ok(engine, "Engine should exist.");
@ -83,6 +84,7 @@ add_task(async function test_extension_adding_engine_with_spaces() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
let engine = Services.search.getEngineByName("MozSearch");
ok(engine, "Engine should exist.");
@ -116,6 +118,7 @@ add_task(async function test_upgrade_default_position_engine() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
let engine = Services.search.getEngineByName("MozSearch");
await Services.search.setDefault(engine);
@ -139,6 +142,7 @@ add_task(async function test_upgrade_default_position_engine() {
},
useAddonManager: "temporary",
});
await AddonTestUtils.waitForSearchProviderStartup(ext1);
engine = Services.search.getEngineByName("MozSearch");
equal(Services.search.defaultEngine, engine, "Default engine should still be MozSearch");
@ -170,6 +174,7 @@ add_task(async function test_extension_post_params() {
});
await ext1.startup();
await AddonTestUtils.waitForSearchProviderStartup(ext1);
let engine = Services.search.getEngineByName("MozSearch");
ok(engine, "Engine should exist.");

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

@ -60,6 +60,7 @@ add_task(async function test_extension_setting_moz_params() {
useAddonManager: "permanent",
});
await extension.startup();
await AddonTestUtils.waitForSearchProviderStartup(extension);
equal(extension.extension.isPrivileged, true, "extension is priviledged");
let engine = Services.search.getEngineByName("MozParamsTest");
@ -113,6 +114,7 @@ add_task(async function test_extension_setting_moz_params_fail() {
useAddonManager: "permanent",
});
await extension.startup();
await AddonTestUtils.waitForSearchProviderStartup(extension);
equal(extension.extension.isPrivileged, false, "extension is not priviledged");
let engine = Services.search.getEngineByName("MozParamsTest");
let expectedURL = engine.getSubmission("test", null, "contextmenu").uri.spec;

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

@ -0,0 +1,95 @@
/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
/* vim: set sts=2 sw=2 et tw=80: */
"use strict";
const {AddonTestUtils} = ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm");
// Lazily import ExtensionParent to allow AddonTestUtils.createAppInfo to
// override Services.appinfo.
ChromeUtils.defineModuleGetter(this, "ExtensionParent",
"resource://gre/modules/ExtensionParent.jsm");
AddonTestUtils.init(this);
AddonTestUtils.overrideCertDB();
AddonTestUtils.createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "42", "42");
add_task(async function shutdown_during_search_provider_startup() {
await AddonTestUtils.promiseStartupManager();
let extension = ExtensionTestUtils.loadExtension({
useAddonManager: "permanent",
manifest: {
chrome_settings_overrides: {
search_provider: {
name: "dummy name",
search_url: "https://example.com/",
},
},
},
});
info("Starting up search extension");
await extension.startup();
let extStartPromise = AddonTestUtils.waitForSearchProviderStartup(extension, {
// Search provider registration is expected to be pending because the search
// service has not been initialized yet.
expectPending: true,
});
let initialized = false;
ExtensionParent.apiManager.global.searchInitialized.then(() => {
initialized = true;
});
await extension.addon.disable();
info("Extension managed to shut down despite the uninitialized search");
// Initialize search after extension shutdown to check that it does not cause
// any problems, and that the test can continue to test uninstall behavior.
Assert.ok(!initialized, "Search service should not have been initialized");
extension.addon.enable();
await extension.awaitStartup();
// Check that uninstall is blocked until the search registration at startup
// has finished. This registration only finished once the search service is
// initialized.
let uninstallingPromise = new Promise(resolve => {
let Management = ExtensionParent.apiManager;
Management.on("uninstall", function listener(eventName, {id}) {
Management.off("uninstall", listener);
Assert.equal(id, extension.id, "Expected extension");
resolve();
});
});
let extRestartPromise = AddonTestUtils.waitForSearchProviderStartup(extension, {
// Search provider registration is expected to be pending again,
// because the search service has still not been initialized yet.
expectPending: true,
});
let uninstalledPromise = extension.addon.uninstall();
let uninstalled = false;
uninstalledPromise.then(() => { uninstalled = true; });
await uninstallingPromise;
Assert.ok(!uninstalled, "Uninstall should not be finished yet");
Assert.ok(!initialized, "Search service should still be uninitialized");
await Services.search.init();
Assert.ok(initialized, "Search service should be initialized");
// After initializing the search service, the search provider registration
// promises should settle eventually.
// Despite the interrupted startup, the promise should still resolve without
// an error.
await extStartPromise;
// The extension that is still active. The promise should just resolve.
await extRestartPromise;
// After initializing the search service, uninstall should eventually finish.
await uninstalledPromise;
await AddonTestUtils.promiseShutdownManager();
});

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

@ -10,6 +10,7 @@
[test_ext_history.js]
[test_ext_settings_overrides_search.js]
[test_ext_settings_overrides_search_mozParam.js]
[test_ext_settings_overrides_shutdown.js]
[test_ext_url_overrides_newtab.js]
[test_ext_url_overrides_newtab_update.js]

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

@ -9,6 +9,9 @@ XPCOMUtils.defineLazyServiceGetter(this, "aboutNewTabService",
"nsIAboutNewTabService");
XPCOMUtils.defineLazyPreferenceGetter(this, "proxyType", PROXY_PREF);
const {AddonTestUtils} = ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm");
AddonTestUtils.initMochitest(this);
const TEST_DIR = gTestPath.substr(0, gTestPath.lastIndexOf("/"));
const CHROME_URL_ROOT = TEST_DIR + "/";
const PERMISSIONS_URL = "chrome://browser/content/preferences/sitePermissions.xul";
@ -468,6 +471,7 @@ add_task(async function testExtensionControlledDefaultSearch() {
let messageShown = waitForMessageShown("browserDefaultSearchExtensionContent");
await originalExtension.startup();
await AddonTestUtils.waitForSearchProviderStartup(originalExtension);
await messageShown;
let addon = await AddonManager.getAddonByID(extensionId);
@ -512,6 +516,7 @@ add_task(async function testExtensionControlledDefaultSearch() {
manifest: Object.assign({}, manifest, {version: "2.0"}),
});
await updatedExtension.startup();
await AddonTestUtils.waitForSearchProviderStartup(updatedExtension);
addon = await AddonManager.getAddonByID(extensionId);
// Verify the extension is updated and search engine didn't change.

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

@ -1474,6 +1474,41 @@ var AddonTestUtils = {
});
},
/**
* Wait until an extension with a search provider has been loaded.
* This should be called after the extension has started, but before shutdown.
*
* @param {object} extension
* The return value of ExtensionTestUtils.loadExtension.
* For browser tests, see mochitest/tests/SimpleTest/ExtensionTestUtils.js
* For xpcshell tests, see toolkit/components/extensions/ExtensionXPCShellUtils.jsm
* @param {object} [options]
* Optional options.
* @param {boolean} [options.expectPending = false]
* Whether to expect the search provider to still be starting up.
*/
async waitForSearchProviderStartup(extension, {expectPending = false} = {}) {
// In xpcshell tests, equal/ok are defined in the global scope.
let {equal, ok} = this.testScope;
if (!equal || !ok) {
// In mochitests, these are available via Assert.jsm.
let {Assert} = this.testScope;
equal = Assert.equal.bind(Assert);
ok = Assert.ok.bind(Assert);
}
equal(extension.state, "running", "Search provider extension should be running");
ok(extension.id, "Extension ID of search provider should be set");
// The map of promises from browser/components/extensions/parent/ext-chrome-settings-overrides.js
let {pendingSearchSetupTasks} = Management.global;
let searchStartupPromise = pendingSearchSetupTasks.get(extension.id);
if (expectPending) {
ok(searchStartupPromise, "Search provider registration should be in progress");
}
return searchStartupPromise;
},
/**
* Initializes the URLPreloader, which is required in order to load
* built_in_addons.json. This has the side-effect of setting