diff options
Diffstat (limited to 'browser/components/urlbar/private')
8 files changed, 115 insertions, 120 deletions
diff --git a/browser/components/urlbar/private/AddonSuggestions.sys.mjs b/browser/components/urlbar/private/AddonSuggestions.sys.mjs index 35849e5cd1..23311cec1c 100644 --- a/browser/components/urlbar/private/AddonSuggestions.sys.mjs +++ b/browser/components/urlbar/private/AddonSuggestions.sys.mjs @@ -178,7 +178,7 @@ export class AddonSuggestions extends BaseFeature { ); } - getResultCommands(result) { + getResultCommands() { const commands = []; if (this.canShowLessFrequently) { diff --git a/browser/components/urlbar/private/AdmWikipedia.sys.mjs b/browser/components/urlbar/private/AdmWikipedia.sys.mjs index 0e266ced40..3ab5bad09f 100644 --- a/browser/components/urlbar/private/AdmWikipedia.sys.mjs +++ b/browser/components/urlbar/private/AdmWikipedia.sys.mjs @@ -135,7 +135,7 @@ export class AdmWikipedia extends BaseFeature { this.#suggestionsMap = suggestionsMap; } - makeResult(queryContext, suggestion, searchString) { + makeResult(queryContext, suggestion) { let originalUrl; if (suggestion.source == "rust") { // The Rust backend defines `rawUrl` on AMP suggestions, and its value is diff --git a/browser/components/urlbar/private/BaseFeature.sys.mjs b/browser/components/urlbar/private/BaseFeature.sys.mjs index d95ace6940..7155d9b7e6 100644 --- a/browser/components/urlbar/private/BaseFeature.sys.mjs +++ b/browser/components/urlbar/private/BaseFeature.sys.mjs @@ -89,10 +89,10 @@ export class BaseFeature { * This method should initialize or uninitialize any state related to the * feature. * - * @param {boolean} enabled + * @param {boolean} _enabled * Whether the feature should be enabled or not. */ - enable(enabled) {} + enable(_enabled) {} /** * If the feature manages suggestions from remote settings that should be @@ -100,12 +100,12 @@ export class BaseFeature { * method. It should return remote settings suggestions matching the given * search string. * - * @param {string} searchString + * @param {string} _searchString * The search string. * @returns {Array} * An array of matching suggestions, or null if not implemented. */ - async queryRemoteSettings(searchString) { + async queryRemoteSettings(_searchString) { return null; } @@ -114,10 +114,10 @@ export class BaseFeature { * override this method. It should fetch the data and build whatever data * structures are necessary to support the feature. * - * @param {RemoteSettings} rs + * @param {RemoteSettings} _rs * The `RemoteSettings` client object. */ - async onRemoteSettingsSync(rs) {} + async onRemoteSettingsSync(_rs) {} /** * If the feature manages suggestions that either aren't served by Merino or @@ -126,12 +126,12 @@ export class BaseFeature { * given suggestion. A telemetry type uniquely identifies a type of suggestion * as well as the kind of `UrlbarResult` instances created from it. * - * @param {object} suggestion + * @param {object} _suggestion * A suggestion from either remote settings or Merino. * @returns {string} * The suggestion's telemetry type. */ - getSuggestionTelemetryType(suggestion) { + getSuggestionTelemetryType(_suggestion) { return this.merinoProvider; } @@ -143,13 +143,13 @@ export class BaseFeature { * fine to rely on the default implementation here because the suggestion type * will be enabled iff the feature itself is enabled. * - * @param {string} type + * @param {string} _type * A Rust suggestion type name as defined in `suggest.udl`. See also * `rustSuggestionTypes`. * @returns {boolean} * Whether the suggestion type is enabled. */ - isRustSuggestionTypeEnabled(type) { + isRustSuggestionTypeEnabled(_type) { return true; } @@ -158,18 +158,18 @@ export class BaseFeature { * override this method. It should return a new `UrlbarResult` for a given * suggestion, which can come from either remote settings or Merino. * - * @param {UrlbarQueryContext} queryContext + * @param {UrlbarQueryContext} _queryContext * The query context. - * @param {object} suggestion + * @param {object} _suggestion * The suggestion from either remote settings or Merino. - * @param {string} searchString + * @param {string} _searchString * The search string that was used to fetch the suggestion. It may be * different from `queryContext.searchString` due to trimming, lower-casing, * etc. This is included as a param in case it's useful. * @returns {UrlbarResult} * A new result for the suggestion. */ - async makeResult(queryContext, suggestion, searchString) { + async makeResult(_queryContext, _suggestion, _searchString) { return null; } diff --git a/browser/components/urlbar/private/MDNSuggestions.sys.mjs b/browser/components/urlbar/private/MDNSuggestions.sys.mjs index 7547b0adff..c9e7da18af 100644 --- a/browser/components/urlbar/private/MDNSuggestions.sys.mjs +++ b/browser/components/urlbar/private/MDNSuggestions.sys.mjs @@ -91,7 +91,7 @@ export class MDNSuggestions extends BaseFeature { this.#suggestionsMap = suggestionsMap; } - async makeResult(queryContext, suggestion, searchString) { + async makeResult(queryContext, suggestion) { if (!this.isEnabled) { // The feature is disabled on the client, but Merino may still return // mdn suggestions anyway, and we filter them out here. @@ -134,7 +134,7 @@ export class MDNSuggestions extends BaseFeature { ); } - getResultCommands(result) { + getResultCommands() { return [ { l10n: { diff --git a/browser/components/urlbar/private/SuggestBackendJs.sys.mjs b/browser/components/urlbar/private/SuggestBackendJs.sys.mjs index 4a91e41b59..58d0ca5edf 100644 --- a/browser/components/urlbar/private/SuggestBackendJs.sys.mjs +++ b/browser/components/urlbar/private/SuggestBackendJs.sys.mjs @@ -278,7 +278,6 @@ export class SuggestBackendJs extends BaseFeature { #config = {}; #emitter = null; - #logger = null; #onSettingsSync = null; } diff --git a/browser/components/urlbar/private/SuggestBackendRust.sys.mjs b/browser/components/urlbar/private/SuggestBackendRust.sys.mjs index fe54feaee8..2d96e7540f 100644 --- a/browser/components/urlbar/private/SuggestBackendRust.sys.mjs +++ b/browser/components/urlbar/private/SuggestBackendRust.sys.mjs @@ -216,7 +216,21 @@ export class SuggestBackendRust extends BaseFeature { } async #init() { - // Create the store. + // Important note on schema updates: + // + // The first time the Suggest store is accessed after a schema version + // update, its backing database will be deleted and a new empty database + // will be created. The database will remain empty until we tell the store + // to ingest. If we wait to ingest as usual until our ingest timer fires, + // the store will remain empty for up to 24 hours, which means we won't + // serve any suggestions at all during that time. + // + // Therefore we simply always ingest here in `#init()`. We'll sometimes + // ingest unnecessarily but that's better than the alternative. (As a + // reminder, for users who have Suggest enabled `#init()` is called whenever + // the Rust backend is enabled, including on startup.) + + // Initialize the store. let path = this.#storePath; this.logger.info("Initializing SuggestStore: " + path); try { @@ -235,17 +249,20 @@ export class SuggestBackendRust extends BaseFeature { return; } - // Before registering the ingest timer, check the last-update pref, which is - // created by the timer manager the first time we register it. If the pref - // doesn't exist, this is the first time the Rust backend has been enabled - // in this profile. In that case, perform ingestion immediately to make - // automated and manual testing easier. Otherwise we'd need to wait at least - // 30s (`app.update.timerFirstInterval`) for the timer manager to call us - // back (and we'd also need to pass false for `skipFirst` below). + // Log the last ingest time for debugging. let lastIngestSecs = Services.prefs.getIntPref( INGEST_TIMER_LAST_UPDATE_PREF, 0 ); + if (lastIngestSecs) { + this.logger.debug( + `Last ingest time: ${lastIngestSecs}s (${ + Math.round(Date.now() / 1000) - lastIngestSecs + }s ago)` + ); + } else { + this.logger.debug("Last ingest time: none"); + } // Register the ingest timer. lazy.timerManager.registerTimer( @@ -255,14 +272,8 @@ export class SuggestBackendRust extends BaseFeature { true // skipFirst ); - if (lastIngestSecs) { - this.logger.info( - `Last ingest: ${lastIngestSecs}s since epoch. Not ingesting now` - ); - } else { - this.logger.info("Last ingest time not found. Ingesting now"); - await this.#ingest(); - } + // Ingest. + await this.#ingest(); } #uninit() { diff --git a/browser/components/urlbar/private/Weather.sys.mjs b/browser/components/urlbar/private/Weather.sys.mjs index c4dfa8c618..51ff5b2f3f 100644 --- a/browser/components/urlbar/private/Weather.sys.mjs +++ b/browser/components/urlbar/private/Weather.sys.mjs @@ -33,8 +33,8 @@ const NOTIFICATIONS = { }; const RESULT_MENU_COMMAND = { - HELP: "help", INACCURATE_LOCATION: "inaccurate_location", + MANAGE: "manage", NOT_INTERESTED: "not_interested", NOT_RELEVANT: "not_relevant", SHOW_LESS_FREQUENTLY: "show_less_frequently", @@ -171,14 +171,14 @@ export class Weather extends BaseFeature { return ["Weather"]; } - isRustSuggestionTypeEnabled(type) { + isRustSuggestionTypeEnabled() { // When weather keywords are defined in Nimbus, weather suggestions are // served by UrlbarProviderWeather. Return false here so the quick suggest // provider doesn't try to serve them too. return !lazy.UrlbarPrefs.get("weatherKeywords"); } - getSuggestionTelemetryType(suggestion) { + getSuggestionTelemetryType() { return "weather"; } @@ -191,6 +191,16 @@ export class Weather extends BaseFeature { } /** + * @returns {Promise} + * If suggestion fetching is disabled, this will be null. Otherwise, if a + * fetch is pending this will be resolved when it's done; if a fetch is not + * pending then it was resolved when the previous fetch finished. + */ + get fetchPromise() { + return this.#fetchPromise; + } + + /** * @returns {Set} * The set of keywords that should trigger the weather suggestion. This will * be null when the Rust backend is enabled and keywords are not defined by @@ -291,20 +301,6 @@ export class Weather extends BaseFeature { } } - /** - * Returns a promise that resolves when all pending fetches finish, if there - * are pending fetches. If there aren't, the promise resolves when all pending - * fetches starting with the next fetch finish. - * - * @returns {Promise} - */ - waitForFetches() { - if (!this.#waitForFetchesDeferred) { - this.#waitForFetchesDeferred = Promise.withResolvers(); - } - return this.#waitForFetchesDeferred.promise; - } - async onRemoteSettingsSync(rs) { this.logger.debug("Loading weather config from remote settings"); let records = await rs.get({ filters: { type: "weather" } }); @@ -356,7 +352,6 @@ export class Weather extends BaseFeature { { url: suggestion.url, iconId: suggestion.current_conditions.icon_id, - helpUrl: lazy.QuickSuggest.HELP_URL, requestId: suggestion.request_id, dynamicType: WEATHER_DYNAMIC_TYPE, city: suggestion.city_name, @@ -410,17 +405,19 @@ export class Weather extends BaseFeature { url: { textContent: result.payload.url, }, - summaryText: { - l10n: { - id: "firefox-suggest-weather-summary-text", - args: { - currentConditions: result.payload.currentConditions, - forecast: result.payload.forecast, + summaryText: lazy.UrlbarPrefs.get("weatherSimpleUI") + ? { textContent: result.payload.currentConditions } + : { + l10n: { + id: "firefox-suggest-weather-summary-text", + args: { + currentConditions: result.payload.currentConditions, + forecast: result.payload.forecast, + }, + cacheable: true, + excludeArgsFromCacheKey: true, + }, }, - cacheable: true, - excludeArgsFromCacheKey: true, - }, - }, highLow: { l10n: { id: "firefox-suggest-weather-high-low", @@ -453,7 +450,7 @@ export class Weather extends BaseFeature { }; } - getResultCommands(result) { + getResultCommands() { let commands = [ { name: RESULT_MENU_COMMAND.INACCURATE_LOCATION, @@ -494,9 +491,9 @@ export class Weather extends BaseFeature { }, { name: "separator" }, { - name: RESULT_MENU_COMMAND.HELP, + name: RESULT_MENU_COMMAND.MANAGE, l10n: { - id: "urlbar-result-menu-learn-more-about-firefox-suggest", + id: "urlbar-result-menu-manage-firefox-suggest", }, } ); @@ -506,8 +503,8 @@ export class Weather extends BaseFeature { handleCommand(view, result, selType) { switch (selType) { - case RESULT_MENU_COMMAND.HELP: - // "help" is handled by UrlbarInput, no need to do anything here. + case RESULT_MENU_COMMAND.MANAGE: + // "manage" is handled by UrlbarInput, no need to do anything here. break; // selType == "dismiss" when the user presses the dismiss key shortcut. case "dismiss": @@ -616,6 +613,21 @@ export class Weather extends BaseFeature { } async #fetch() { + // Keep a handle on the `MerinoClient` instance that exists at the start of + // this fetch. If fetching stops or this `Weather` instance is uninitialized + // during the fetch, `#merino` will be nulled, and the fetch should stop. We + // can compare `merino` to `#merino` to tell when this occurs. + let merino = this.#merino; + let fetchInstance = (this.#fetchInstance = {}); + + await this.#fetchPromise; + if (fetchInstance != this.#fetchInstance || merino != this.#merino) { + return; + } + await (this.#fetchPromise = this.#fetchHelper({ fetchInstance, merino })); + } + + async #fetchHelper({ fetchInstance, merino }) { this.logger.info("Fetching suggestion"); if (this.#vpnDetected) { @@ -626,36 +638,19 @@ export class Weather extends BaseFeature { // new fetch. this.logger.info("VPN detected, not fetching"); this.#suggestion = null; - if (!this.#pendingFetchCount) { - this.#waitForFetchesDeferred?.resolve(); - this.#waitForFetchesDeferred = null; - } return; } - // This `Weather` instance may be uninitialized while awaiting the fetch or - // even uninitialized and re-initialized a number of times. Multiple fetches - // may also happen at once. Ignore the fetch below if `#merino` changes or - // another fetch happens in the meantime. - let merino = this.#merino; - let instance = (this.#fetchInstance = {}); - this.#restartFetchTimer(); this.#lastFetchTimeMs = Date.now(); - this.#pendingFetchCount++; - let suggestions; - try { - suggestions = await merino.fetch({ - query: "", - providers: [MERINO_PROVIDER], - timeoutMs: this.#timeoutMs, - extraLatencyHistogram: HISTOGRAM_LATENCY, - extraResponseHistogram: HISTOGRAM_RESPONSE, - }); - } finally { - this.#pendingFetchCount--; - } + let suggestions = await merino.fetch({ + query: "", + providers: [MERINO_PROVIDER], + timeoutMs: this.#timeoutMs, + extraLatencyHistogram: HISTOGRAM_LATENCY, + extraResponseHistogram: HISTOGRAM_RESPONSE, + }); // Reset the Merino client's session so different fetches use different // sessions. A single session is intended to represent a single user @@ -665,28 +660,23 @@ export class Weather extends BaseFeature { // to keep it ticking in the meantime. merino.resetSession(); - if (merino != this.#merino || instance != this.#fetchInstance) { - this.logger.info("Fetch finished but is out of date, ignoring"); - } else { - let suggestion = suggestions?.[0]; - if (!suggestion) { - // No suggestion was received. The network may be offline or there may - // be some other problem. Set the suggestion to null: Better to show - // nothing than outdated weather information. When the network comes - // back online, one or more network notifications will be sent, - // triggering a new fetch. - this.logger.info("No suggestion received"); - this.#suggestion = null; - } else { - this.logger.info("Got suggestion"); - this.logger.debug(JSON.stringify({ suggestion })); - this.#suggestion = { ...suggestion, source: "merino" }; - } + if (fetchInstance != this.#fetchInstance || merino != this.#merino) { + this.logger.info("Fetch is out of date, discarding suggestion"); + return; } - if (!this.#pendingFetchCount) { - this.#waitForFetchesDeferred?.resolve(); - this.#waitForFetchesDeferred = null; + let suggestion = suggestions?.[0]; + if (!suggestion) { + // No suggestion was received. The network may be offline or there may be + // some other problem. Set the suggestion to null: Better to show nothing + // than outdated weather information. When the network comes back online, + // one or more network notifications will be sent, triggering a new fetch. + this.logger.info("No suggestion received"); + this.#suggestion = null; + } else { + this.logger.info("Got suggestion"); + this.logger.debug(JSON.stringify({ suggestion })); + this.#suggestion = { ...suggestion, source: "merino" }; } } @@ -865,10 +855,6 @@ export class Weather extends BaseFeature { return this.#merino; } - get _test_pendingFetchCount() { - return this.#pendingFetchCount; - } - async _test_fetch() { await this.#fetch(); } @@ -884,13 +870,12 @@ export class Weather extends BaseFeature { #fetchDelayAfterComingOnlineMs = FETCH_DELAY_AFTER_COMING_ONLINE_MS; #fetchInstance = null; #fetchIntervalMs = FETCH_INTERVAL_MS; + #fetchPromise = null; #fetchTimer = 0; #keywords = null; #lastFetchTimeMs = 0; #merino = null; - #pendingFetchCount = 0; #rsConfig = null; #suggestion = null; #timeoutMs = MERINO_TIMEOUT_MS; - #waitForFetchesDeferred = null; } diff --git a/browser/components/urlbar/private/YelpSuggestions.sys.mjs b/browser/components/urlbar/private/YelpSuggestions.sys.mjs index 546c7ce216..a1ac13177b 100644 --- a/browser/components/urlbar/private/YelpSuggestions.sys.mjs +++ b/browser/components/urlbar/private/YelpSuggestions.sys.mjs @@ -55,7 +55,7 @@ export class YelpSuggestions extends BaseFeature { return !cap || this.showLessFrequentlyCount < cap; } - getSuggestionTelemetryType(suggestion) { + getSuggestionTelemetryType() { return "yelp"; } @@ -127,7 +127,7 @@ export class YelpSuggestions extends BaseFeature { ); } - getResultCommands(result) { + getResultCommands() { let commands = [ { name: RESULT_MENU_COMMAND.INACCURATE_LOCATION, |