nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1897508 - Separate out search engine icon updates into a separate notification subject. r=search-reviewers,settings-reviewers,mcheang

Browse source 
This also avoids sending an engine-update Glean event for the default engine by not handling the engine-icon-update
notification in the update tracking code.

Differential Revision: https://phabricator.services.mozilla.com/D210838
This commit is contained in:
Mark Banner 2024-05-24 09:10:05 +00:00
parent 0600e5f0d8
commit 67039cecb0
10 changed files with 41 additions and 21 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

2
browser/components/extensions/test/browser/browser_ext_search_favicon.js
View file

@ -30,7 +30,7 @@ async function promiseEngineIconLoaded(engineName) {
"browser-search-engine-modified",
(engine, verb) => {
engine.QueryInterface(Ci.nsISearchEngine);
return verb == "engine-changed" && engine.name == engineName;
return verb == "engine-icon-changed" && engine.name == engineName;
}
);
Assert.ok(

1
browser/components/preferences/search.js
View file

@ -749,6 +749,7 @@ class EngineStore {
this.notifyRowCountChanged(gEngineView.lastEngineIndex, 1);
break;
case "engine-changed":
case "engine-icon-changed":
this.updateEngine(engine);
this.notifyRebuildViews();
break;

4
browser/components/urlbar/UrlbarSearchUtils.sys.mjs
View file

@ -131,6 +131,10 @@ class SearchUtils {
/**
* Gets the engine with a given alias.
*
* Note: engines returned from this list may be updated at any time. If you
* are caching the icon or other fields for more than a single engagement of
* the urlbar, consider observing the SEARCH_ENGINE_TOPIC.
*
* @param {string} alias
* A search engine alias. The alias string comparison is case insensitive.
* @param {string} [searchString]

5
toolkit/components/search/AppProvidedSearchEngine.sys.mjs
View file

@ -453,7 +453,10 @@ export class AppProvidedSearchEngine extends SearchEngine {
this.#blobURLPromise = null;
}
this.#blobURLPromise = Promise.resolve(blobURL);
lazy.SearchUtils.notifyAction(this, lazy.SearchUtils.MODIFIED_TYPE.CHANGED);
lazy.SearchUtils.notifyAction(
this,
lazy.SearchUtils.MODIFIED_TYPE.ICON_CHANGED
);
}
/**

2
toolkit/components/search/SearchEngine.sys.mjs
View file

@ -798,7 +798,7 @@ export class SearchEngine {
if (this._engineAddedToStore) {
lazy.SearchUtils.notifyAction(
this,
lazy.SearchUtils.MODIFIED_TYPE.CHANGED
lazy.SearchUtils.MODIFIED_TYPE.ICON_CHANGED
);
}
this._hasPreferredIcon = isPreferred;

11
toolkit/components/search/SearchSettings.sys.mjs
View file

@ -5,6 +5,8 @@
const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
AppProvidedSearchEngine:
"resource://gre/modules/AppProvidedSearchEngine.sys.mjs",
DeferredTask: "resource://gre/modules/DeferredTask.sys.mjs",
ObjectUtils: "resource://gre/modules/ObjectUtils.sys.mjs",
SearchUtils: "resource://gre/modules/SearchUtils.sys.mjs",
@ -501,6 +503,15 @@ export class SearchSettings {
case lazy.SearchUtils.MODIFIED_TYPE.REMOVED:
this._delayedWrite();
break;
case lazy.SearchUtils.MODIFIED_TYPE.ICON_CHANGED:
// Application Provided Search Engines have their icons stored in
// Remote Settings, so we don't need to update the saved settings.
if (
!(engine?.wrappedJSObject instanceof lazy.AppProvidedSearchEngine)
) {
this._delayedWrite();
}
break;
}
break;
case lazy.SearchUtils.TOPIC_SEARCH_SERVICE:

1
toolkit/components/search/SearchUtils.sys.mjs
View file

@ -186,6 +186,7 @@ export var SearchUtils = {
TOPIC_ENGINE_MODIFIED: "browser-search-engine-modified",
MODIFIED_TYPE: {
CHANGED: "engine-changed",
ICON_CHANGED: "engine-icon-changed",
REMOVED: "engine-removed",
ADDED: "engine-added",
DEFAULT: "engine-default",

24
toolkit/components/search/tests/xpcshell/test_appProvided_icons_updates.js
View file

@ -234,8 +234,8 @@ add_task(async function test_icon_added_existing_engine() {
});
await client.db.update(mock.record, Date.now());
let promiseEngineUpdated = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.CHANGED,
let promiseIconChanged = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.ICON_CHANGED,
SearchUtils.TOPIC_ENGINE_MODIFIED
);
@ -250,7 +250,7 @@ add_task(async function test_icon_added_existing_engine() {
SearchTestUtils.idleService._fireObservers("idle");
await promiseEngineUpdated;
await promiseIconChanged;
await assertEngineIcon("engine_no_initial_icon name", "bigIcon.ico");
});
@ -270,8 +270,8 @@ add_task(async function test_icon_updated() {
});
await client.db.update(mock.record, Date.now());
let promiseEngineUpdated = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.CHANGED,
let promiseIconChanged = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.ICON_CHANGED,
SearchUtils.TOPIC_ENGINE_MODIFIED
);
@ -285,7 +285,7 @@ add_task(async function test_icon_updated() {
});
SearchTestUtils.idleService._fireObservers("idle");
await promiseEngineUpdated;
await promiseIconChanged;
await assertEngineIcon("engine_icon_updates name", "bigIcon.ico");
});
@ -296,12 +296,12 @@ add_task(async function test_icon_not_local() {
await assertEngineIcon("engine_icon_not_local name", null);
// A download should have been queued, so fire idle to trigger it.
let promiseEngineUpdated = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.CHANGED,
let promiseIconChanged = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.ICON_CHANGED,
SearchUtils.TOPIC_ENGINE_MODIFIED
);
SearchTestUtils.idleService._fireObservers("idle");
await promiseEngineUpdated;
await promiseIconChanged;
await assertEngineIcon("engine_icon_not_local name", "bigIcon.ico");
});
@ -313,12 +313,12 @@ add_task(async function test_icon_out_of_date() {
await assertEngineIcon("engine_icon_out_of_date name", "remoteIcon.ico");
// A download should have been queued, so fire idle to trigger it.
let promiseEngineUpdated = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.CHANGED,
let promiseIconChanged = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.ICON_CHANGED,
SearchUtils.TOPIC_ENGINE_MODIFIED
);
SearchTestUtils.idleService._fireObservers("idle");
await promiseEngineUpdated;
await promiseIconChanged;
await assertEngineIcon("engine_icon_out_of_date name", "bigIcon.ico");
});

6
toolkit/components/search/tests/xpcshell/test_opensearch_icon.js
View file

@ -36,8 +36,8 @@ add_task(async function test_icon_types() {
SearchUtils.MODIFIED_TYPE.ADDED,
SearchUtils.TOPIC_ENGINE_MODIFIED
);
let promiseEngineChanged = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.CHANGED,
let promiseIconChanged = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.ICON_CHANGED,
SearchUtils.TOPIC_ENGINE_MODIFIED
);
const engineData = {
@ -54,7 +54,7 @@ add_task(async function test_icon_types() {
let engine = await promiseEngineAdded;
// Ensure this is a nsISearchEngine.
engine.QueryInterface(Ci.nsISearchEngine);
await promiseEngineChanged;
await promiseIconChanged;
Assert.ok(await engine.getIconURL(), `${test.name} engine has an icon`);
Assert.ok(

6
toolkit/components/search/tests/xpcshell/test_webextensions_install.js
View file

@ -201,8 +201,8 @@ add_task(async function test_load_favicon_invalid_redirect() {
});
add_task(async function test_load_favicon_redirect() {
let promiseEngineChanged = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.CHANGED,
let promiseIconChanged = SearchTestUtils.promiseSearchNotification(
SearchUtils.MODIFIED_TYPE.ICON_CHANGED,
SearchUtils.TOPIC_ENGINE_MODIFIED
);
@ -216,7 +216,7 @@ add_task(async function test_load_favicon_redirect() {
let engine = await Services.search.getEngineByName("Example");
await promiseEngineChanged;
await promiseIconChanged;
Assert.ok(await engine.getIconURL(), "Should have set an iconURI");
Assert.ok(