1831656 - Refactor quick suggest remote settings management

Status: VERIFIED FIXED

(Firefox :: Address Bar, task, P1)

Comment 1

[Drew Willcoxon :adw]

Attachment: Bug 1831656 - Refactor quick suggest remote settings management.

This refactors quick suggest remote settings management so it will be easier to
support new types of remote settings suggestions. It builds on D177191.

Currently, RemoteSettingsClient only handles adM and Wikipedia suggestions. It
would be possible for it to support more suggestions types by adding a series of
if statements, one per suggestion type (to check if the suggestion type is
enabled), with each statement fetching a specific suggestion type's data.
However, that doesn't scale elegantly.

It would also be possible for RemoteSettingsClient to be agnostic about the
suggestions in remote settings. IOW it doesn't need to care that different types
of suggestions are stored in RS. It could treat them all as one big map from
keywords to suggestions. However, that would require uniformity and stability in
how suggestions are stored in RS, and since the Suggest project is not mature,
that's not the case right now, and it won't be the case any time soon.

Instead, this revision moves suggestion-specific logic from RemoteSettingsClient
to BaseFeature classes specific to each suggestion type. Each BaseFeature
registers itself with remote settings using the new QuickSuggestRemoteSettings
module, fetches its specific RS data, and builds whatever data structure is
appropriate for serving its suggestions. I expect most features to use a simple
map from keywords to suggestions, like adM/Wikipedia does, so I factored out the
map-related code into a new SuggestionsMap class. The weather feature is an
exception because only its keywords are stored in RS, not the suggestion itself.

UrlbarProviderQuickSuggest accesses remote settings suggestions by going through
QuickSuggestRemoteSettings. It only needs to make one call.

This design should work well when the cross-platform Rust component is ready.
We should only need to modify UrlbarProviderQuickSuggest so it fetches
suggestions from the Rust component instead of QuickSuggestRemoteSettings.

Depends on D177191

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52f34c3a861d
Refactor quick suggest remote settings management. r=daisuke

Comment 3

[Drew Willcoxon :adw]

For QA verification, there are no particular STR since this is a refactoring of how Firefox Suggest remote settings suggestion are handled. We only want to make sure remote settings sponsored/nonsponsored suggestions are still shown properly.

You can set the following prefs to force Firefox to use remote settings suggestions and no Merino suggestions (these values are the defaults):

browser.urlbar.quicksuggest.remoteSettings.enabled = true
browser.urlbar.quicksuggest.dataCollection.enabled = false

Comment 4

[Drew Willcoxon :adw]

Comment on attachment 9331930 [details]
Bug 1831656 - Refactor quick suggest remote settings management.

Beta/Release Uplift Approval Request

• User impact if declined: This bug is necessary for the Firefox Suggest weather suggestion feature we intend to ship in 114.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Please see comment 3
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Medium risk because this is a non-trivial refactoring of Firefox Suggest code. However, this code is well tested and it affects only the Firefox Suggest feature, which is currently limited to US users.
• String changes made/needed:
• Is Android affected?: No

Comment 5

[Drew Willcoxon :adw]

[Tracking Requested - why for this release]: This bug is necessary for the Firefox Suggest weather suggestion feature we intend to ship in 114.

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/52f34c3a861d

Comment 7

[Carmen Fat [:cfat] - Ecosystem QA]

We have verified this issue on the latest Firefox Nightly 115.0a1 (Build ID: 20230508214159), on Windows 10 x64, macOS 12.6.1, and Ubuntu 20.04 x64.
We made sure that the 2 preferences are set accordingly:
browser.urlbar.quicksuggest.remoteSettings.enabled = true
browser.urlbar.quicksuggest.dataCollection.enabled = false
We verified that the weather suggestion, sponsored, and non-sponsored suggestions are correctly triggered and displayed.

• One question though: when interacting with the weather result, we noticed that the contextual services event mention Merino as the source for the engagement event
  (contextservices.quicksuggest engagement click {"match_type": "firefox-suggest", "position": "2", "suggestion_type": "weather", "source": "merino"} ).
• Is this due to the fact that only the keywords triggering the weather result are kept in Remote Settings, but the actual weather result is delivered via Merino?

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9331930 [details]
Bug 1831656 - Refactor quick suggest remote settings management.

Approved for 114 beta 2, thanks.

Comment 9

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/18f0c5eb8b02

Comment 10

[Drew Willcoxon :adw]

Yes, the weather suggestion is always served from Merino, even when other suggestions are served from remote settings.

The browser.urlbar.quicksuggest.dataCollection.enabled pref controls whether Firefox sends the user's search string to Merino in relation to the usual sponsored/non-sponsored suggestions and the newer types of suggestions we're developing -- dynamic Wikipedia, navigational suggestions, addons, Pocket, etc. The weather suggestion is in its own special category because Firefox doesn't need to send the search string to Merino to fetch it. Instead, Firefox downloads a list of keywords from remote settings, does the keyword matching on the client, and then fetches the suggestion from Merino without sending it a search string.

Thanks for checking!

Comment 11

[Carmen Fat [:cfat] - Ecosystem QA]

Thank you Drew for the clarification.
We have verified this issue on the latest Firefox Beta 114.0b2 (Build ID: 20230509180058), on Windows 10 x64, macOS 12.6.1, and Ubuntu 20.04 x64.

• We made sure that the 2 preferences are set accordingly:
  browser.urlbar.quicksuggest.remoteSettings.enabled = true
  browser.urlbar.quicksuggest.dataCollection.enabled = false
• We verified that the weather suggestion, sponsored, and non-sponsored suggestions are correctly triggered and displayed.