Bug 1867288 - Load application provided search engine icons from remote settings. r=search-reviewers,mcheang,mconley

Depends on D204529

Differential Revision: https://phabricator.services.mozilla.com/D204263
This commit is contained in:
Mark Banner 2024-03-13 22:14:28 +00:00
parent 9946454fe2
commit 2c3696f010
11 changed files with 446 additions and 67 deletions

View File

@ -553,7 +553,13 @@ export let ContentSearch = {
},
/**
* Converts the engine's icon into an appropriate URL for display at
* Converts the engine's icon into a URL or an ArrayBuffer for passing to the
* content process.
*
* @param {nsISearchEngine} engine
* The engine to get the icon for.
* @returns {string|ArrayBuffer}
* The icon's URL or an ArrayBuffer containing the icon data.
*/
async _getEngineIconURL(engine) {
let url = await engine.getIconURL();
@ -561,14 +567,20 @@ export let ContentSearch = {
return SEARCH_ENGINE_PLACEHOLDER_ICON;
}
// The uri received here can be of two types
// The uri received here can be one of several types:
// 1 - moz-extension://[uuid]/path/to/icon.ico
// 2 - data:image/x-icon;base64,VERY-LONG-STRING
// 3 - blob:
//
// If the URI is not a data: URI, there's no point in converting
// it to an arraybuffer (which is used to optimize passing the data
// accross processes): we can just pass the original URI, which is cheaper.
if (!url.startsWith("data:")) {
// For moz-extension URIs we can pass the URI to the content process and
// use it directly as they can be accessed from there and it is cheaper.
//
// For blob URIs the content process is a different scope and we can't share
// the blob with that scope. Hence we have to create a copy of the data.
//
// For data: URIs we convert to an ArrayBuffer as that is more optimal for
// passing the data across to the content process.
if (!url.startsWith("data:") && !url.startsWith("blob:")) {
return url;
}

View File

@ -12,7 +12,7 @@
<head>
<!-- @CSP: We should remove 'unsafe-inline' from style-src, see Bug 1579160 -->
<meta http-equiv="Content-Security-Policy" content="default-src chrome:; img-src chrome: moz-icon: https: data:; style-src chrome: data: 'unsafe-inline'; object-src 'none'" />
<meta http-equiv="Content-Security-Policy" content="default-src chrome:; img-src chrome: moz-icon: https: blob: data:; style-src chrome: data: 'unsafe-inline'; object-src 'none'" />
<title data-l10n-id="settings-page-title"></title>

View File

@ -101,11 +101,28 @@ add_task(async function test_search_icon() {
let { win, tab } = await openAboutPrivateBrowsing();
await SpecialPowers.spawn(tab, [expectedIconURL], async function (iconURL) {
is(
content.document.body.getAttribute("style"),
`--newtab-search-icon: url(${iconURL});`,
"Should have the correct icon URL for the logo"
let computedStyle = content.window.getComputedStyle(content.document.body);
await ContentTaskUtils.waitForCondition(
() => computedStyle.getPropertyValue("--newtab-search-icon") != "null",
"Search Icon should get set."
);
if (iconURL.startsWith("blob:")) {
// We don't check the data here as `browser_contentSearch.js` performs
// those checks.
Assert.ok(
computedStyle
.getPropertyValue("--newtab-search-icon")
.startsWith("url(blob:"),
"Should have a blob URL for the logo"
);
} else {
Assert.equal(
computedStyle.getPropertyValue("--newtab-search-icon"),
`url(${iconURL})`,
"Should have the correct icon URL for the logo"
);
}
});
await BrowserTestUtils.closeWindow(win);

View File

@ -50,6 +50,14 @@ add_setup(async function () {
await SearchTestUtils.promiseNewSearchEngine({
url: getRootDirectory(gTestPath) + "testEngine_chromeicon.xml",
});
// Install a WebExtension based engine to allow testing passing of plain
// URIs (moz-extension://) to the content process.
await SearchTestUtils.installSearchExtension({
icons: {
16: "favicon.ico",
},
});
});
add_task(async function GetState() {
@ -491,7 +499,7 @@ function iconDataFromURI(uri) {
);
}
if (!uri.startsWith("data:")) {
if (!uri.startsWith("data:") && !uri.startsWith("blob:")) {
plainURIIconTested = true;
return Promise.resolve(uri);
}

View File

@ -24,17 +24,6 @@ ChromeUtils.defineESModuleGetters(this, {
"resource://gre/modules/SearchSuggestionController.sys.mjs",
});
const pageURL = getRootDirectory(gTestPath) + TEST_PAGE_BASENAME;
BrowserTestUtils.registerAboutPage(
registerCleanupFunction,
"test-about-content-search-ui",
pageURL,
Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT |
Ci.nsIAboutModule.URI_MUST_LOAD_IN_CHILD |
Ci.nsIAboutModule.ALLOW_SCRIPT |
Ci.nsIAboutModule.URI_CAN_LOAD_IN_PRIVILEGEDABOUT_PROCESS
);
requestLongerTimeout(2);
function waitForSuggestions() {
@ -261,6 +250,19 @@ let extension1;
let extension2;
add_setup(async function () {
const pageURL = getRootDirectory(gTestPath) + TEST_PAGE_BASENAME;
let cleanupAboutPage;
await BrowserTestUtils.registerAboutPage(
callback => (cleanupAboutPage = callback),
"test-about-content-search-ui",
pageURL,
Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT |
Ci.nsIAboutModule.URI_MUST_LOAD_IN_CHILD |
Ci.nsIAboutModule.ALLOW_SCRIPT |
Ci.nsIAboutModule.URI_CAN_LOAD_IN_PRIVILEGEDABOUT_PROCESS
);
let originalOnMessageSearch = ContentSearch._onMessageSearch;
let originalOnMessageManageEngines = ContentSearch._onMessageManageEngines;
@ -290,8 +292,20 @@ add_setup(async function () {
}
registerCleanupFunction(async () => {
// Ensure tabs are closed before we continue on with the cleanup.
for (let tab of tabs) {
BrowserTestUtils.removeTab(tab);
}
Services.search.restoreDefaultEngines();
await TestUtils.waitForTick();
ContentSearch._onMessageSearch = originalOnMessageSearch;
ContentSearch._onMessageManageEngines = originalOnMessageManageEngines;
if (cleanupAboutPage) {
await cleanupAboutPage();
}
});
await promiseTab();
@ -1096,10 +1110,6 @@ add_task(async function settings() {
await msg("reset");
});
add_task(async function cleanup() {
Services.search.restoreDefaultEngines();
});
function checkState(
actualState,
expectedInputVal,
@ -1147,10 +1157,10 @@ function checkState(
}
var gMsgMan;
var tabs = [];
async function promiseTab() {
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
registerCleanupFunction(() => BrowserTestUtils.removeTab(tab));
tabs.push(tab);
let loadedPromise = BrowserTestUtils.firstBrowserLoaded(window);
openTrustedLinkIn("about:test-about-content-search-ui", "current");

View File

@ -58,11 +58,22 @@ async function ensureIcon(tab, expectedIcon) {
"Search Icon not set."
);
Assert.equal(
computedStyle.getPropertyValue("--newtab-search-icon"),
`url(${icon})`,
"Should have the expected icon"
);
if (icon.startsWith("blob:")) {
// We don't check the data here as `browser_contentSearch.js` performs
// those checks.
Assert.ok(
computedStyle
.getPropertyValue("--newtab-search-icon")
.startsWith("url(blob:"),
"Should have a blob URL"
);
} else {
Assert.equal(
computedStyle.getPropertyValue("--newtab-search-icon"),
`url(${icon})`,
"Should have the expected icon"
);
}
}
);
}

View File

@ -12,9 +12,94 @@ import {
const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
RemoteSettings: "resource://services-settings/remote-settings.sys.mjs",
SearchUtils: "resource://gre/modules/SearchUtils.sys.mjs",
});
/**
* Handles loading application provided search engine icons from remote settings.
*/
class IconHandler {
#iconList = null;
#iconCollection = null;
/**
* Returns the icon for the record that matches the engine identifier
* and the preferred width.
*
* @param {string} engineIdentifier
* The identifier of the engine to match against.
* @param {number} preferredWidth
* The preferred with of the icon.
* @returns {string}
* An object URL that can be used to reference the contents of the specified
* source object.
*/
async getIcon(engineIdentifier, preferredWidth) {
if (!this.#iconList) {
await this.#getIconList();
}
let iconRecords = this.#iconList.filter(r => {
return r.engineIdentifiers.some(i => {
if (i.endsWith("*")) {
return engineIdentifier.startsWith(i.slice(0, -1));
}
return engineIdentifier == i;
});
});
if (!iconRecords.length) {
console.warn("No icon found for", engineIdentifier);
return null;
}
// Default to the first record, in the event we don't have any records
// that match the width.
let iconRecord = iconRecords[0];
for (let record of iconRecords) {
// TODO: Bug 1655070. We should be using the closest size, but for now use
// an exact match.
if (record.imageSize == preferredWidth) {
iconRecord = record;
break;
}
}
let iconURL;
try {
iconURL = await this.#iconCollection.attachments.get(iconRecord);
} catch (ex) {
console.error(ex);
return null;
}
if (!iconURL) {
console.warn("Unable to find the icon for", engineIdentifier);
return null;
}
return URL.createObjectURL(
new Blob([iconURL.buffer]),
iconRecord.attachment.mimetype
);
}
/**
* Obtains the icon list from the remote settings collection.
*/
async #getIconList() {
this.#iconCollection = lazy.RemoteSettings("search-config-icons");
try {
this.#iconList = await this.#iconCollection.get();
} catch (ex) {
console.error(ex);
this.#iconList = [];
}
if (!this.#iconList.length) {
console.error("Failed to obtain search engine icon list records");
}
}
}
/**
* AppProvidedSearchEngine represents a search engine defined by the
* search configuration.
@ -25,6 +110,20 @@ export class AppProvidedSearchEngine extends SearchEngine {
["suggestions", lazy.SearchUtils.URL_TYPE.SUGGEST_JSON],
["trending", lazy.SearchUtils.URL_TYPE.TRENDING_JSON],
]);
static iconHandler = new IconHandler();
/**
* @typedef {?Promise<string>}
* A promise for the blob URL of the icon. We save the promise to avoid
* reentrancy issues.
*/
#blobURLPromise = null;
/**
* @typedef {?string}
* The identifier from the configuration.
*/
#configurationId = null;
/**
* @param {object} options
@ -51,11 +150,23 @@ export class AppProvidedSearchEngine extends SearchEngine {
this._extensionID = extensionId;
this._locale = config.webExtension.locale;
this.#configurationId = config.identifier;
this.#init(config);
this._loadSettings(settings);
}
/**
* Used to clean up the engine when it is removed. This will revoke the blob
* URL for the icon.
*/
async cleanup() {
if (this.#blobURLPromise) {
URL.revokeObjectURL(await this.#blobURLPromise);
this.#blobURLPromise = null;
}
}
/**
* Update this engine based on new config, used during
* config upgrades.
@ -68,7 +179,6 @@ export class AppProvidedSearchEngine extends SearchEngine {
*/
update({ configuration } = {}) {
this._urls = [];
this._iconMapObj = null;
this.#init(configuration);
lazy.SearchUtils.notifyAction(this, lazy.SearchUtils.MODIFIED_TYPE.CHANGED);
}
@ -128,6 +238,25 @@ export class AppProvidedSearchEngine extends SearchEngine {
);
}
/**
* Returns the icon URL for the search engine closest to the preferred width.
*
* @param {number} preferredWidth
* The preferred width of the image.
* @returns {Promise<string>}
* A promise that resolves to the URL of the icon.
*/
async getIconURL(preferredWidth) {
if (this.#blobURLPromise) {
return this.#blobURLPromise;
}
this.#blobURLPromise = AppProvidedSearchEngine.iconHandler.getIcon(
this.#configurationId,
preferredWidth
);
return this.#blobURLPromise;
}
/**
* Creates a JavaScript object that represents this engine.
*
@ -159,38 +288,6 @@ export class AppProvidedSearchEngine extends SearchEngine {
this._telemetryId += `-${engineConfig.telemetrySuffix}`;
}
// Set the main icon URL for the engine.
// let iconURL = searchProvider.favicon_url;
// if (!iconURL) {
// iconURL =
// manifest.icons &&
// extensionBaseURI.resolve(
// lazy.ExtensionParent.IconDetails.getPreferredIcon(manifest.icons).icon
// );
// }
// // Record other icons that the WebExtension has.
// if (manifest.icons) {
// let iconList = Object.entries(manifest.icons).map(icon => {
// return {
// width: icon[0],
// height: icon[0],
// url: extensionBaseURI.resolve(icon[1]),
// };
// });
// for (let icon of iconList) {
// this._addIconToMap(icon.size, icon.size, icon.url);
// }
// }
// this._initWithDetails(config);
// this._sendAttributionRequest = config.sendAttributionRequest ?? false; // TODO check if we need to this?
// if (details.iconURL) {
// this._setIcon(details.iconURL, true);
// }
this._name = engineConfig.name.trim();
this._definedAliases =
engineConfig.aliases?.map(alias => `@${alias}`) ?? [];

View File

@ -2321,6 +2321,10 @@ export class SearchService {
// Use the internal remove - _reloadEngines already deals with default
// engines etc, and we want to avoid adjusting the sort order unnecessarily.
this.#internalRemoveEngine(engine);
if (engine instanceof lazy.AppProvidedSearchEngine) {
await engine.cleanup();
}
} else {
// If we have other engines that use the same extension ID, then
// we do not want to remove the add-on - only remove the engine itself.

View File

@ -429,6 +429,8 @@ export var SearchTestUtils = {
*
* @param {object} [options]
* The options for the manifest.
* @param {object} [options.icons]
* The icons to use for the WebExtension.
* @param {string} [options.id]
* The id to use for the WebExtension.
* @param {string} [options.name]
@ -478,6 +480,10 @@ export var SearchTestUtils = {
},
};
if (options.icons) {
manifest.icons = options.icons;
}
if (options.default_locale) {
manifest.default_locale = options.default_locale;
}

View File

@ -0,0 +1,211 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
/**
* Tests to ensure that icons for application provided engines are correctly
* loaded from remote settings.
*/
"use strict";
// A skeleton configuration that gets filled in from TESTS during `add_setup`.
let CONFIG = [
{
recordType: "defaultEngines",
globalDefault: "engine_no_icon",
specificDefaults: [],
},
{
recordType: "engineOrders",
orders: [],
},
];
let TESTS = [
{
engineId: "engine_no_icon",
expectedIcon: null,
},
{
engineId: "engine_exact_match",
icons: [
{
filename: "remoteIcon.ico",
engineIdentifiers: ["engine_exact_match"],
imageSize: 16,
},
],
expectedIcon: "remoteIcon.ico",
},
{
engineId: "engine_begins_with",
icons: [
{
filename: "remoteIcon.ico",
engineIdentifiers: ["engine_begins*"],
imageSize: 16,
},
],
expectedIcon: "remoteIcon.ico",
},
{
engineId: "engine_non_default_sized_icon",
icons: [
{
filename: "remoteIcon.ico",
engineIdentifiers: ["engine_non_default_sized_icon"],
imageSize: 32,
},
],
expectedIcon: "remoteIcon.ico",
},
{
engineId: "engine_multiple_icons",
icons: [
{
filename: "bigIcon.ico",
engineIdentifiers: ["engine_multiple_icons"],
imageSize: 16,
},
{
filename: "remoteIcon.ico",
engineIdentifiers: ["engine_multiple_icons"],
imageSize: 32,
},
],
expectedIcon: "bigIcon.ico",
},
];
async function getFileDataBuffer(filename) {
let data = await IOUtils.read(
PathUtils.join(do_get_cwd().path, "data", filename)
);
return new TextEncoder().encode(data).buffer;
}
async function mockRecordWithAttachment({
filename,
engineIdentifiers,
imageSize,
}) {
let buffer = await getFileDataBuffer(filename);
let stream = Cc["@mozilla.org/io/arraybuffer-input-stream;1"].createInstance(
Ci.nsIArrayBufferInputStream
);
stream.setData(buffer, 0, buffer.byteLength);
// Generate a hash.
let hasher = Cc["@mozilla.org/security/hash;1"].createInstance(
Ci.nsICryptoHash
);
hasher.init(Ci.nsICryptoHash.SHA256);
hasher.updateFromStream(stream, -1);
let hash = hasher.finish(false);
hash = Array.from(hash, (_, i) =>
("0" + hash.charCodeAt(i).toString(16)).slice(-2)
).join("");
let record = {
id: Services.uuid.generateUUID().toString(),
engineIdentifiers,
imageSize,
attachment: {
hash,
location: `main-workspace/search-config-icons/${filename}`,
filename,
size: buffer.byteLength,
mimetype: "application/json",
},
};
let attachment = {
record,
blob: new Blob([buffer]),
};
return { record, attachment };
}
async function insertRecordIntoCollection(client, db, item) {
let { record, attachment } = await mockRecordWithAttachment(item);
await db.create(record);
await client.attachments.cacheImpl.set(record.id, attachment);
await db.importChanges({}, Date.now());
}
add_setup(async function () {
let client = RemoteSettings("search-config-icons");
let db = client.db;
await db.clear();
for (let test of TESTS) {
CONFIG.push({
identifier: test.engineId,
recordType: "engine",
base: {
name: test.engineId + " name",
urls: {
search: {
base: "https://example.com/" + test.engineId,
searchTermParamName: "q",
},
},
},
variants: [{ environment: { allRegionsAndLocales: true } }],
});
if ("icons" in test) {
for (let icon of test.icons) {
await insertRecordIntoCollection(client, db, {
...icon,
id: test.engineId,
});
}
}
}
await SearchTestUtils.useTestEngines("simple-engines", null, CONFIG);
await Services.search.init();
});
for (let test of TESTS) {
add_task(async function () {
info("Testing engine: " + test.engineId);
let engine = Services.search.getEngineByName(test.engineId + " name");
if (test.expectedIcon) {
let engineIconURL = await engine.getIconURL(16);
Assert.notEqual(
engineIconURL,
null,
"Should have an icon URL for the engine."
);
let response = await fetch(engineIconURL);
let buffer = new Uint8Array(await response.arrayBuffer());
let expectedBuffer = new Uint8Array(
await getFileDataBuffer(test.expectedIcon)
);
Assert.equal(
buffer.length,
expectedBuffer.length,
"Should have received matching buffer lengths for the expected icon"
);
Assert.ok(
buffer.every((value, index) => value === expectedBuffer[index]),
"Should have received matching data for the expected icon"
);
} else {
Assert.equal(
await engine.getIconURL(),
null,
"Should not have an icon URL for the engine."
);
}
});
}

View File

@ -78,6 +78,9 @@ support-files = [
["test_appDefaultEngine.js"]
["test_appProvided_icons.js"]
prefs = ["browser.search.newSearchConfig.enabled=true"]
["test_async.js"]
["test_config_engine_params.js"]