Commit Graph

50 Commits

Author SHA1 Message Date
Drew Willcoxon
1817ce0847 Bug 1952588 - Vendor application-services to 138 for Suggest geo expansion. r=bdk,daisuke
This vendors the `desktop-138` branch [1] of application-services. The `main`
branch currently does not have some PRs that desktop needs because they break
mobile, and we need this on desktop ASAP.

This patch is larger than usual because the vendor includes some major changes
to the application-services `suggest` component, including new and changed API.
As a consequence this patch includes the following important changes:

## New `suggest` API & uniffi

`SuggestStoreBuilder::remote_settings_service` and `RemoteSettingsService::new`
are exposed to JS as synchronous functions. There's no need for them to be
off-main-thread.

## Telemetry

The labels of `suggest.ingest_time`, `ingest_download_time` and `query_time` had
to be updated due to changes in the Rust component. These are minor updates that
don't need a data review.

## Urlbar

I had to make the following changes to urlbar. I tried to keep them to a minimum
for now. There are opportunities for improvements in follow-ups.

* Appropriate minimal integration changes to `SuggestBackendRust` for creating
  the `SuggestStore` and setting up the RS config
* The Rust component uses new RS collections, which breaks tests. I tried to fix
  them without touching too many lines. There are definitely opportunities to
  improve these tests and test helpers that I'd like to come back to.
* A fix to `RemoteSettingsServer` that's required due to the new RS client used
  by the Rust component

## Late writes due to the new RS client & `AsyncShutdown`

This is a urlbar change but it's worth calling out separately. I pushed all
these changes to tryserver, and there was a failure at the end of the browser
Suggest tests due to `LateWriteObserver` [2].

The late write happens when the app exits: `SuggestStore` is dropped, which
causes the new app-services RS client to drop its Sqlite connection, which
causes Sqlite to sync the RS client's DB to disk. This hasn't been a problem
before because `suggest` currently uses the old RS client, which doesn't keep a
DB. (`suggest` does have its own separate Sqlite DB, and I didn't investigate
why this isn't a problem for it, mainly because it makes sense that the new RS
client would sync its DB when it's dropped and that might be considered a "late
write" when it happens on app shutdown.)

According to the stack in the log, `SuggestStore` is dropped by
`nsCycleCollector`. I can't see how `SuggestBackendRust.#store` is involved in a
cycle, and I don't know if something in this patch is causing a cycle where
there wasn't one before. Maybe there always was. And I don't know if the cycle
is what's causing the all this to happen too late on shutdown. Maybe it's
unrelated. (I'll paste the stack in a Phabricator comment.)

The `SuggestStore` is definitely kept alive until
`AsyncShutdown.profileBeforeChange` since we have a barrier for that phase that
calls `interrupt()` on it. Maybe that's simply the problem and we're using a
phase that's too late in shutdown. But again I don't know why it wouldn't also
be a problem for Suggest's own DB.

The only fix I found is to replace `AsyncShutdown.profileBeforeChange` with
either `quitApplicationGranted` or `profileChangeTeardown`, and then null out
`#store` in the callback (after we call `interrupt()` on it). I assume that
fixes it because `profileBeforeChange` runs later than those other two phases.

So I replaced `profileBeforeChange` with `profileChangeTeardown`. I don't know
which of `quitApplicationGranted` or `profileChangeTeardown` is better. I think
it probably doesn't matter. I chose `profileChangeTeardown` because it's closer
to `profileBeforeChange`. (The order is: `quitApplicationGranted`,
`profileChangeTeardown`, `profileBeforeChange`.)

[1] https://github.com/mozilla/application-services/tree/desktop-138
[2] https://treeherder.mozilla.org/jobs?repo=try&revision=1639f87aa46f1afaf50901d80c8282861700019b

Differential Revision: https://phabricator.services.mozilla.com/D240919
2025-03-12 20:20:09 +00:00
Drew Willcoxon
44aeb0a311 Bug 1943211 - Make SuggestFeature.shouldEnable check enablingPreferences. r=daisuke
This patch makes it unnecessary for most features to have to override
`shouldEnable`. Instead, the Suggest framework will check `enablingPreferences`
plus a new property called `additionalEnablingPredicate`. Summary:

* Include Nimbus variables in `enablingPreferences`
* Add a new property to `SuggestFeature` called `additionalEnablingPredicate`
* Make `shouldEnable` call `enablingPreferences` and
  `additionalEnablingPredicate`
* Remove `shouldEnable` implementations on most features

Differential Revision: https://phabricator.services.mozilla.com/D235251
2025-01-23 21:53:29 +00:00
Drew Willcoxon
31209ed783 Bug 1941990 - Allow SuggestProvider to manage at most one Rust suggestion type. r=daisuke
With D234449 fixed, we can change `SuggestProvider.rustSuggestionTypes` (plural)
to `rustSuggestionType` (singular). We can remove `isRustSuggestionTypeEnabled`
too because it's assumed that if the feature itself is enabled then its Rust
suggestions are too. I don't expect we'll ever need to break that assumption,
but if we do, we can just add back `isRustSuggestionTypeEnabled` at that time.

Depends on D234449

Differential Revision: https://phabricator.services.mozilla.com/D234450
2025-01-17 02:34:38 +00:00
Drew Willcoxon
18185b4a92 Bug 1941694 - Remove the quick_suggest_ingest_time telemetry metric. r=daisuke
Depends on D234258

Differential Revision: https://phabricator.services.mozilla.com/D234440
2025-01-17 02:34:37 +00:00
Drew Willcoxon
ca65a9a803 Bug 1936951 - Rename BaseFeature to SuggestFeature and factor it out into SuggestProvider and SuggestBackend. r=daisuke
This does several things, and I'll update D231863 so it builds on it:

Rename `BaseFeature` to `SuggestFeature` since only Suggest uses it and it's
pretty deeply integrated with `QuickSuggest` at this point.

Factor out all the suggestions/provider-specific methods into a new
`SuggestProvider` subclass. All features that manage suggestions are now
subclasses of `SuggestProvider` instead of `BaseFeature`.

Add a new subclass called `SuggestBackend` that codifies the expected backend
methods.

Rename `Weather` to `WeatherSuggestions` to be more consistent with the other
feature subclasses.

Update a bunch of comments so hopefully they're clearer and more accurate. I
also replaced the term "quick suggest" with "Suggest".

It doesn't make any substantive changes beyond the above.

Differential Revision: https://phabricator.services.mozilla.com/D232022
2024-12-17 18:37:03 +00:00
Drew Willcoxon
2033165d91 Bug 1932502 - Remove the Suggest JS backend: Part 2: Remove the backend. r=daisuke
This is part 2 of 4, the main part. It removes the backend and the code in
`BaseFeature` and feature subclasses that hook into it. As a consequence it also
makes the other following changes:

* Remove `QuickSuggest.backend` (eventually I'd like to add a Merino backend and
  then add a new `QuickSuggest.backends` list that has the Rust, Merino, and ML
  backends)
* Add `QuickSuggest.config`
* Replace uses of `QuickSuggest.backend.config` and `QuickSuggest.jsBackend.config`
  with `QuickSuggest.config`
* Remove the `quickSuggestRustEnabled` Nimbus variable
* Update most remaining tests so that they do not assume the JS backend exists

I left the `browser.urlbar.quicksuggest.rustEnabled` pref. I would like to
preserve the ability to toggle off Rust suggestions while keeping other parts of
Suggest enabled even if we don't need it now. That seems like a wise thing to do
and is similar to how both Merino and the ML backends can be toggled separately.

Depends on D231465

Differential Revision: https://phabricator.services.mozilla.com/D231466
2024-12-11 20:50:45 +00:00
Drew Willcoxon
fd7c157a55 Bug 1925459 - Prevent the Suggest Rust component from using a dummy remote settings URL in tests. r=daisuke
This error is harmless and originates in the `remote_settings` Rust component,
which the `suggest` Rust component relies on. The exact line that raises the
error is [1]. The equivalent Searchfox link of the vendored code is [2].

The problem is that `url` is `data:,#remote-settings-dummy/v1`, the dummy RS
server URL that's set by the xpcshell and browser test harnesses. `Url::join()`
doesn't work on "cannot-be-a-base" URLs like `data` URIs [3].

That's why this error is logged only in non-Suggest tests: Suggest remains
enabled for those tests, but of course they don't use the mock RS server that's
used for Suggest tests, so while they're running Suggest and `remote_settings`
are trying to use that dummy URL.

This patch allows the RS config to be set to null in `SuggestBackendRust`. In
that case, the backend won't try to create the Suggest store at all, effectively
disabling Rust suggestions. Rust suggestions were already effectively disabled
in non-Suggest tests due to this URL error -- that, plus the fact that there's
no RS server to serve Rust suggestions during non-Suggest tests.

[1] d3a5b91d44/components/remote_settings/src/config.rs (L80)
[2] https://searchfox.org/mozilla-central/rev/7fb746f0be47ce0135af7bca9fffdb5cd1c4b1d5/third_party/rust/remote_settings/src/config.rs#80
[3] https://docs.rs/url/latest/url/struct.Url.html#method.join

Differential Revision: https://phabricator.services.mozilla.com/D231195
2024-12-05 20:07:08 +00:00
Drew Willcoxon
27e74e304d Bug 1932807 - Stop using JSON.stringify() in urlbar logging. r=urlbar-reviewers,Standard8
This also downgrades and removes some logging that I myself added that hasn't
proven to be useful. I reworded messages I wrote in a few places too.

I didn't touch `SuggestBackendJs.sys.mjs` because it's unused and will likely be
removed soon (bug 1932502).

Depends on D229847

Differential Revision: https://phabricator.services.mozilla.com/D229868
2024-11-22 22:20:57 +00:00
Drew Willcoxon
2be848b5c2 Bug 1931964 - Use the Suggest geonames API to match locations in Yelp suggestions. r=daisuke
This integrates Yelp suggestions with city/region detection from the Rust
component [1]. That includes both Rust Yelp suggestions and ML Yelp suggestions.
The reason for fixing this bug is to improve ML suggestions by reducing false
positives, but there's no reason we can't also do city/region matching for Rust
suggestions too.

With this patch, location detection and matching in Yelp suggestions now works
just like weather suggestions.

There are two benefits to city/region matching: (1) We can ignore queries with
"invalid" cities/regions, (2) we can do prefix matching, so for example if you
type "ramen in ne", we can match that to "ramen in New York, NY". Of course,
"invalid" doesn't always mean truly invalid. It just means we don't have a match
in our city/region database, which does not contain every city in the world.

This doesn't change how we handle queries that don't contain a location. In
those cases we'll still use geolocation.

This also builds on D229720 so that if multiple cities match the query, we'll
choose the one that best matches the user's location.

[1] [Documentation here](https://mozilla.github.io/application-services/book/rust-docs/suggest/struct.SuggestStore.html#method.fetch_geonames), [source here](c348fba48a/components/suggest/src/geoname.rs (L194))

Depends on D229720

Differential Revision: https://phabricator.services.mozilla.com/D229847
2024-11-22 22:20:57 +00:00
Drew Willcoxon
aa56c40785 Bug 1926381 - Integrate MLSuggest with UrlbarProviderQuickSuggest and implement Yelp ML suggestions. r=daisuke
This adds a new Suggest backend for ML-based suggestions called
`SuggestBackendMl`. Before, with the JS and Rust backends, only one backend was
enabled at a time, but both the ML and Rust backends can be enabled at the same
time since we will want to serve suggestions from both for the foreseeable
future. Features can support ML suggestions by implementing the new
`BaseFeature.mlIntent` getter and handling ML suggestions in `makeResult()`.
Each feature can decide whether it supports ML suggestions and whether they
should be preferred over Rust suggestions.

I've updated the Yelp feature to hook into this, since Yelp suggestions are
supported by the ML model that Chidam is working on. If ML is enabled, then the
feature will only serve ML suggestions. I'm not sure if that's what we want long
term, but for now that will make it clear to people which backend is being used
while we develop this feature.

The `quickSuggestMlEnabled` variable/pref determines whether the ML backend is
enabled. The `yelpMlEnabled` variable/pref determines whether Yelp ML
suggestions are enabled. We can create similar variable/prefs for each feature
that supports ML suggestions so that they can be toggled independently of each
other.

Other changes:

Move the `is_sponsored` logic out of the Rust backend and into the provider.
Otherwise it would need to be duplicated in the ML backend too.

Depends on D224523

Differential Revision: https://phabricator.services.mozilla.com/D226736
2024-10-24 09:52:00 +00:00
Ben Dean-Kawamura
b37f9a1085 Bug 1911639 - Record Suggest metrics, r=adw
Differential Revision: https://phabricator.services.mozilla.com/D224162
2024-10-03 15:35:10 +00:00
Ben Dean-Kawamura
16ea6863b4 Bug 1921280 - Vendor in the latest app-services commit, r=adw,urlbar-reviewers
This brought in a breaking change: `Suggestion.icon` is now the `bytes`
type.  To accomidate that, added UniFFI support for bytes and updated
the SuggestBackendRust code since that field is now a Uint8Array on the
JS side.

Differential Revision: https://phabricator.services.mozilla.com/D223774
2024-09-30 14:07:55 +00:00
Drew Willcoxon
247ce81e9b Bug 1915317 - Integrate Rust exposure suggestions with desktop. r=daisuke
This integrates Rust exposure suggestions with desktop. Exposure suggestions are
a part of the replacement for the existing potential exposures feature
(bug 1881875). When we want to test potential exposures in the future, we can
add new exposure suggestions to remote settings and tell the Rust component to
ingest them and return them in queries. When the Rust component returns an
exposure suggestion, desktop will record it in exposure telemetry. (The other
part of the replacement is keyword exposure telemetry in bug 1915507 D220501).

More details about the design of exposure suggestions here:

* Bug 1893086
* https://github.com/mozilla/application-services/pull/6343

The way desktop tells the Rust component about different types of exposure
suggestions is through the new "provider constraints" feature. `ingest()` and
`query()` can take a `SuggestionProviderConstraints` object that changes what's
ingested and queried. Therefore, we need to re-ingest exposure suggestions when
the provider constraints change. Right now, exposure suggestions are the only
kind of suggestions that use provider constraints.

Depends on D220359

Differential Revision: https://phabricator.services.mozilla.com/D220359
2024-09-06 04:30:20 +00:00
Drew Willcoxon
887c7ed38f Bug 1915291 - Keep track of stale Suggest suggestion types and ingest only stale types when their feature is updated. r=daisuke
This keeps track of the enabled suggestion types that have been ingested while
the app is running. I call those types "fresh". In the code, they're stored in
the `#ingestedSuggestionTypes` set. Types that have not been ingested and types
that are disabled are called "stale". When a feature is updated, we update the
staleness bookkeeping in `#ingestedSuggestionTypes` and ingest only enabled
stale types.

This is similar to how each feature has an `#isEnabled` property. It's our
cached value of the last-known state that we can compare to the current state.

Depends on D219943

Differential Revision: https://phabricator.services.mozilla.com/D220352
2024-08-29 03:59:14 +00:00
Drew Willcoxon
67a3eeb74e Bug 1907434 - Update desktop's Fakespot suggestion integration with the Rust component. r=daisuke
Depends on D216488

Differential Revision: https://phabricator.services.mozilla.com/D216493
2024-07-17 01:25:44 +00:00
Drew Willcoxon
5b6f606320 Bug 1907696 - Change the Suggest ingest strategy so only enabled suggestion types are ingested. r=daisuke
Please see the bug for context. This changes how ingest works. Right now we
ingest all "default" suggestion types at once. This patch causes a suggestion
type to be ingested only when it's enabled.

Before this patch, there are two times ingest happens, on startup and when the
ingest timer fires.

With this patch, there are now a few times ingest happens:

* On startup (as before). `SuggestBackendRust` will ingest all registered and
  enabled suggestion types.
* When the ingest timer fires (as before). Similar to startup, all registered
  and enabled types will be ingested.
* When a suggestion type becomes enabled. I added this functionality to
  `BaseFeature.update()` so that each feature gets this behavior automatically.

It's worth saying more about the startup case. On startup, all `BaseFeature`s'
initializations race each other. `SuggestBackendRust` is itself a `BaseFeature`,
so there's no guarantee it will be initialized before other `BaseFeature`s that
implement Rust suggestions. If any `BaseFeature`s are initialized before
`SuggestBackendRust`, they'll try to ingest but nothing will happen because
`SuggestBackendRust` won't be initialized yet. Once `SuggestBackendRust` is
initialized, it will ingest suggestions for all registered `BaseFeature`s that
already tried and failed to ingest. And after `SuggestBackendRust` initializes,
any `BaseFeature`s that are initialized later will be able to successfully
ingest.

Differential Revision: https://phabricator.services.mozilla.com/D216488
2024-07-17 01:25:44 +00:00
Ben Dean-Kawamura
3c9e7d3c64 Bug 1907973 - Load FTS5 extension for Suggest, r=nanj,urlbar-reviewers,adw
We need to do this now that we're creating FTS tables. I believe this
should fix the vendoring issues.  The following tests were failing
before and now are succeeding on my machine:

./mach test browser/components/urlbar/tests/quicksuggest/unit
./mach test browser/components/search/test/browser/telemetry/browser_search_telemetry_sources.js

Differential Revision: https://phabricator.services.mozilla.com/D216598
2024-07-15 20:48:11 +00:00
Drew Willcoxon
e6f441eebf Bug 1907227 - Add telemetry probe for Rust ingest times. r=daisuke,urlbar-reviewers
This adds a new Glean timing distribution called `quick_suggest_ingest_time` to
the `urlbar` namespace. The distribution is updated on each successful ingest. I
don't think it makes sense to record failed ingests because they would likely
skew the data.

Depends on D216238

Differential Revision: https://phabricator.services.mozilla.com/D216241
2024-07-12 00:25:49 +00:00
Drew Willcoxon
a452c2bc8f Bug 1895110 - Add a profileBeforeChange shutdown blocker to interrupt the Rust Suggest store. r=daisuke
This adds a `profileBeforeChange` async shutdown blocker that interrupts the
Suggest store. It calls `interrupt(InterruptKind.READ_WRITE)` so that both
ingests and queries are interrupted. The new `interrupt()` API is defined in
suggest.udl here:

https://searchfox.org/mozilla-central/rev/b11735b86bb4d416c918e2b2413456561beff50c/third_party/rust/suggest/src/suggest.udl#149-153

`InterruptKind` is defined in the generated UniFFI JS bindings here:

https://searchfox.org/mozilla-central/rev/b11735b86bb4d416c918e2b2413456561beff50c/toolkit/components/uniffi-bindgen-gecko-js/components/generated/RustSuggest.sys.mjs#1115-1119

This also modifies the one existing `interrupt()` call (when queries are
canceled) to pass `InterruptKind.READ`. According to the `interrupt()` doc,
passing nothing is deprecated.

I simplified the first task in test_rust_ingest.js. Now that Rust is enabled by
default, there's no need to test a first run where Rust starts out disabled but
then becomes enabled. Now the task only tests disabling it and then re-enabling
it. This also means Rust is enabled for the rest of the test, which makes the
test easier to work with.

Depends on D203824

Differential Revision: https://phabricator.services.mozilla.com/D214180
2024-06-21 06:44:02 +00:00
Lina Butler
4259d07cde Bug 1880183 - Adopt the SuggestStoreBuilder API. r=adw,bdk
The Suggest Rust component currently stores the SQLite database in the
"local profile" directory, since it doesn't contain any user data.

Now that the component supports remembering dismissed suggestions,
we'll want to store the database in the "profile" directory instead,
since it'll contain persistent user data.

This commit prepares us for that future by specifying the profile
directory as the data path when we build the store. This commit also
adopts the `SuggestStoreBuilder#remoteSettings{Server, BucketName}()`
APIs.

Depends on D201774

Differential Revision: https://phabricator.services.mozilla.com/D203824
2024-06-20 23:01:42 +00:00
Daisuke Akatsuka
5dacf1db0e Bug 1895423: Ingest suggestions while idling r=adw
Differential Revision: https://phabricator.services.mozilla.com/D209633
2024-05-07 22:33:02 +00:00
Daisuke Akatsuka
a1930be19e Bug 1882967: Create icon image by its blob and mimetype r=adw
Differential Revision: https://phabricator.services.mozilla.com/D203237
2024-03-21 10:26:53 +00:00
Drew Willcoxon
3d5ff2451f Bug 1884014 - Make the Suggest Rust backend always ingest on startup. r=lina
This modifies the Rust backend so it always ingests whenever it's enabled,
including on every startup.

Ideally we would test a schema update, but since the current schema version is
hardcoded Rust, I don't think that's possible.

Differential Revision: https://phabricator.services.mozilla.com/D203855
2024-03-07 02:51:01 +00:00
Drew Willcoxon
402313354b Bug 1880416 - Fix problems with ingest in the Suggest Rust backend. r=daisuke
This makes sure only one ingest happens at a time. If a new ingest starts while
one is ongoing, it will await the ingest promise and then start. If many ingests
try to start at the same time, the last one will continue and the others will
bail due to `#ingestInstance`.

Depends on D201912

Differential Revision: https://phabricator.services.mozilla.com/D201927
2024-02-15 05:52:59 +00:00
Drew Willcoxon
ed36ae547a Bug 1880144 - Enable Rust Suggest by default. r=daisuke
This turned out to be a huge pain.

Many tests didn't work with the Rust backend. Generally I tried to keep tests
and tasks that truly need the JS backend, and I skip them when Rust is enabled.
I also added checks for `quickSuggestRustEnabled` so that tests don't assume the
Rust backend is enabled. I don't want to remove anything related to the JS
backend until the Rust backend is shipped in Release and we're confident we
don't need the JS backend anymore.

Also, browser tests didn't work properly because by the time
`QuickSuggestTestUtils.ensureQuickSuggestInit()` runs, the Rust backend has
already initialized, and `_test_remoteSettingsConfig` is not defined. To fix
that, I added a new `_test_setRemoteSettingsConfig()` function that recreates
the store with the new RS test config.

I also added some new helpers for generating remote settings data and for making
expected results in xpcshell tests.

Differential Revision: https://phabricator.services.mozilla.com/D201800
2024-02-14 20:36:08 +00:00
Drew Willcoxon
5903331bc7 Bug 1878444 - Update quick suggest config to account for Rust. r=daisuke
There are two types of config in Rust Suggest: global and per-provider. Right
now global contains `show_less_frequently_cap`, and the only provider config is
for weather, which contains `min_keyword_length`. This patch uses both of them.

It makes sense to me to fetch config as part of the ingest process, so that's
what I've done. I'm not sure when else would be a good time. It's async so it's
not too convenient to do it on demand.

In RS records and attachement JSON, we use snake_case, but Rust converts it to
camelCase (actually UniFFI does, I think). That means the JS backend will use
snake_case for all config, but the Rust backend will use camelCase. I wrote a
helper for converting snake to camel so we can always just use camel.

Depends on D200755

Differential Revision: https://phabricator.services.mozilla.com/D201023
2024-02-13 06:16:19 +00:00
Daisuke Akatsuka
8f73bbab2e Bug 1874624: Add SVG mimetype for Yelp icon blob r=adw
Differential Revision: https://phabricator.services.mozilla.com/D201182
2024-02-09 12:48:32 +00:00
Mark Hammond
0ec2fd36f8 Bug 1875848 - UniFFI generated record constructors now take a POJO. r=lina,bdk,adw
Differential Revision: https://phabricator.services.mozilla.com/D200075
2024-02-08 17:34:58 +00:00
Drew Willcoxon
a2cb610df6 Bug 1878441 - Update the weather suggestions implementation to account for Rust. r=daisuke
This patch requires https://github.com/mozilla/application-services/pull/6089,
which hasn't been vendored yet, so it will be hard to test unless you locally
vendor. I've done that and verified that weather tests still pass.

Weather suggestions in Rust work like this: Rust stores the weather keywords in
the `keywords` table, and when the query matches a keyword, it returns a dummy
`Weather` suggestion that only contains a score. That means weather suggestions
are handled nearly like every other type of suggestion, except when Firefox
receives a weather suggestion from Rust, it must replace it with the actual
suggestion from Merino.

We also need to continue to support weather keywords defined in Nimbus. For
that, this patch continues to use `UrlbarProviderWeather` because I don't want
to add a special case to `UrlbarProviderQuickSuggest` just for this one type of
suggestion. When we stop experimenting with weather, we can remove that
provider.

I moved all the common code from `UrlbarProviderWeather` to `Weather` so both
providers can use it.

Some of the tests check `minKeywordLength` and can't use Rust yet. I'll handle
that in bug 1878444.

Depends on D200105

Differential Revision: https://phabricator.services.mozilla.com/D200755
2024-02-08 01:47:05 +00:00
Natalia Csoregi
882dcd96ab Backed out changeset ef78defe4be2 (bug 1875848) for causing quicksuggest failures. CLOSED TREE 2024-02-08 03:45:03 +02:00
Mark Hammond
a45645fcbf Bug 1875848 - UniFFI generated record constructors now take a POJO. r=lina,bdk,adw
Differential Revision: https://phabricator.services.mozilla.com/D200075
2024-02-07 23:16:26 +00:00
Drew Willcoxon
a4f3e9fe53 Bug 1878987 - Part 3: Update RemoteSettingsConfig argument order. r=lina
This bitrots D200075 but since we want to ship Rust Suggest in desktop in 124,
I'd like to vendor ASAP so we can start landing desktop patches, and we need to
fix this argument order to do that.

Depends on D200892

Differential Revision: https://phabricator.services.mozilla.com/D200893
2024-02-07 20:07:25 +00:00
Daisuke Akatsuka
5e7e476a76 Bug 1878814: Handle Yelp suggestion as sponsored r=adw,fluent-reviewers
Differential Revision: https://phabricator.services.mozilla.com/D200751
2024-02-07 02:01:25 +00:00
Daisuke Akatsuka
d7d4769a47 Bug 1875952: Make RemoteSettings devtools environment available in the Suggest Rust backend r=adw
Depends on D199304

Differential Revision: https://phabricator.services.mozilla.com/D199305
2024-01-24 00:57:08 +00:00
Daisuke Akatsuka
eb79c672da Bug 1868922: Pass user's query as is to Rust component to keep string cases r=adw
Differential Revision: https://phabricator.services.mozilla.com/D198513
2024-01-19 02:44:41 +00:00
Cristian Tuns
87fcff935b Backed out 4 changesets (bug 1868922, bug 1855375, bug 1874990) for causing xpcshell failures in /test_tab_quickwrite.js CLOSED TREE
Backed out changeset d558120aba19 (bug 1868922)
Backed out changeset e2c1399903d8 (bug 1855375)
Backed out changeset de119e6a8ced (bug 1874990)
Backed out changeset 3d1ef7c11154 (bug 1874990)
2024-01-18 12:07:55 -05:00
Daisuke Akatsuka
03a5a6291f Bug 1868922: Pass user's query as is to Rust component to keep string cases r=adw
Differential Revision: https://phabricator.services.mozilla.com/D198513
2024-01-18 15:54:42 +00:00
Drew Willcoxon
3f0ad75711 Bug 1874072 - Don't query the Suggest Rust component for disabled suggestion types. r=daisuke
Please see the bug for context.

Differential Revision: https://phabricator.services.mozilla.com/D198216
2024-01-11 04:23:33 +00:00
Drew Willcoxon
ab05a8bad7 Bug 1861540 - Part 2: Update tests to use the local remote settings server. r=daisuke
This modifies tests to use the server. There are a few important points to call
out:

This means tests are now using the real Rust component, and we need to make sure
the test RS data is valid and matches what Rust expects. For example, I had to
add `icon` properties to suggestions and set the `advertiser` to "Wikipedia" for
non-sponsored suggestions. Otherwise Rust hits an error on ingest. I also
removed some test cases because they tested behaviors that are impossible with
Rust, for example Pocket keywords that are duplicated in the high- and
low-confidence arrays.

We need to be careful to wait until Suggest is done syncing from remote settings
regardless of whether it's using the JS or Rust backend. I added a way to force
the backends to sync. That way, tests can force a sync, wait for it to finish,
and be sure that all sync activity is done.

A common pattern in tests is to call `ensureQuickSuggestInit()` and then set
Suggest-related prefs (or vice versa). This is a little problematic because both
`ensureQuickSuggestInit()` and setting prefs can cause Suggest to start a sync.
It's more problematic now that we're not mocking remote settings or Rust. So I
combined the two by adding a `prefs` param to `ensureQuickSuggestInit()`. That
way, tests can be sure that all syncing is done once that function returns.

Depends on D192037, D192124

Differential Revision: https://phabricator.services.mozilla.com/D192038
2023-10-30 23:33:12 +00:00
Cristian Tuns
9f3bb56869 Backed out 2 changesets (bug 1861540) for causing xpc shell failures in test_quicksuggest_addons.js CLOSED TREE
Backed out changeset 2a0da4c5f326 (bug 1861540)
Backed out changeset b67e855993cb (bug 1861540)
2023-10-27 19:35:54 -04:00
Drew Willcoxon
629eb836c5 Bug 1861540 - Part 2: Update tests to use the local remote settings server. r=daisuke
This modifies tests to use the server. There are a few important points to call
out:

This means tests are now using the real Rust component, and we need to make sure
the test RS data is valid and matches what Rust expects. For example, I had to
add `icon` properties to suggestions and set the `advertiser` to "Wikipedia" for
non-sponsored suggestions. Otherwise Rust hits an error on ingest. I also
removed some test cases because they tested behaviors that are impossible with
Rust, for example Pocket keywords that are duplicated in the high- and
low-confidence arrays.

We need to be careful to wait until Suggest is done syncing from remote settings
regardless of whether it's using the JS or Rust backend. I added a way to force
the backends to sync. That way, tests can force a sync, wait for it to finish,
and be sure that all sync activity is done.

A common pattern in tests is to call `ensureQuickSuggestInit()` and then set
Suggest-related prefs (or vice versa). This is a little problematic because both
`ensureQuickSuggestInit()` and setting prefs can cause Suggest to start a sync.
It's more problematic now that we're not mocking remote settings or Rust. So I
combined the two by adding a `prefs` param to `ensureQuickSuggestInit()`. That
way, tests can be sure that all syncing is done once that function returns.

Depends on D192037

Differential Revision: https://phabricator.services.mozilla.com/D192038
2023-10-27 22:31:55 +00:00
Drew Willcoxon
a316dabe80 Bug 1859341 - Modify desktop Suggest integration so it uses addon and Pocket suggestions from Rust. r=daisuke
This builds on the recent vendor that was done in bug 1859069 and D191004 in
particular.

The parameter to the Rust `SuggestStore.query()` function now takes an object
with a `providers` property, instead of separate boolean sponsored and
nonsponsored properties. `providers` is an array whose items are integers that
identify Rust providers (defined in [the RustSuggest bindings here](https://searchfox.org/mozilla-central/rev/c3c92500ef8ffab4fa53d4b2d5c5e2b49025e89b/toolkit/components/uniffi-bindgen-gecko-js/components/generated/RustSuggest.sys.mjs#953-958)).

`MockRustSuggest` in `QuickSuggestTestUtils.sys.mjs` is becoming kind of a pain
to keep updated. More tasks in `test_quicksuggest_pocket.js` should use Rust
than the ones I've updated here, but they don't because I would need to
(re)implement the prefix-matching behavior in `MockRustSuggest`. In a follow-up
I'd like to explore using a mock remote settings server instead so that the real
Rust component can be tested.

Differential Revision: https://phabricator.services.mozilla.com/D191351
2023-10-20 02:52:56 +00:00
Norisz Fay
578a0551ac Backed out changeset 1b9473fe1ebe (bug 1859341) for causing bc failures on browser_glean_telemetry_engagement_selected_result.js CLOSED TREE 2023-10-20 05:30:32 +03:00
Drew Willcoxon
22d28243e8 Bug 1859341 - Modify desktop Suggest integration so it uses addon and Pocket suggestions from Rust. r=daisuke
This builds on the recent vendor that was done in bug 1859069 and D191004 in
particular.

The parameter to the Rust `SuggestStore.query()` function now takes an object
with a `providers` property, instead of separate boolean sponsored and
nonsponsored properties. `providers` is an array whose items are integers that
identify Rust providers (defined in [the RustSuggest bindings here](https://searchfox.org/mozilla-central/rev/c3c92500ef8ffab4fa53d4b2d5c5e2b49025e89b/toolkit/components/uniffi-bindgen-gecko-js/components/generated/RustSuggest.sys.mjs#953-958)).

`MockRustSuggest` in `QuickSuggestTestUtils.sys.mjs` is becoming kind of a pain
to keep updated. More tasks in `test_quicksuggest_pocket.js` should use Rust
than the ones I've updated here, but they don't because I would need to
(re)implement the prefix-matching behavior in `MockRustSuggest`. In a follow-up
I'd like to explore using a mock remote settings server instead so that the real
Rust component can be tested.

Differential Revision: https://phabricator.services.mozilla.com/D191351
2023-10-20 01:42:21 +00:00
Drew Willcoxon
61acdfca1f Bug 1859069 - Part 3: Fix urlbar consumers for the updated suggest JS bindings. r=lina
As the TODO's say, I'd like to clean this up and add support for AMO and Pocket,
but right now let's just get the status quo working.

Depends on D191003

Differential Revision: https://phabricator.services.mozilla.com/D191004
2023-10-14 00:37:13 +00:00
Drew Willcoxon
57b0f17880 Bug 1858549 - Remove the quickSuggestRemoteSettingsEnabled Nimbus variable and fallback pref. r=daisuke
Depends on D190742

Differential Revision: https://phabricator.services.mozilla.com/D190745
2023-10-12 15:50:50 +00:00
Drew Willcoxon
17f4082056 Bug 1857396 - Use the update timer manager to periodically perform ingestion in the Suggest Rust component. r=daisuke
This uses the update timer manager (`nsIUpdateTimerManager`) to periodically
ingest. This is the same mechanism the desktop remote settings client uses.
Please see the bug for more info on the update timer manager. I left some notes
there.

I chose the same 24-hour interval that the remote settings client uses. So
overall, this should give us pretty much the same update behavior we have with
remote settings in the JS backend.

Differential Revision: https://phabricator.services.mozilla.com/D190384
2023-10-10 02:28:54 +00:00
Drew Willcoxon
0d60a186b7 Bug 1855884 - Handle icons from the Suggest Rust component. r=dao
The new Rust implementation of Suggest stores icons for some suggestion types in
its Sqlite database, and it returns these icons to consumers as byte arrays. To
show these icons in the view quickly and without any overhead, we can create
`Blob` URLs from the arrays. Blob URLs need to be revoked when we're done with
them to avoid leaking the backing data.

This patch implements some simple lifetime management of blob URLs in the view.
The first time the view shows a result, it creates a blob URL for the result's
icon. The view caches the URL while it remains open, so as the user continues to
type and possibly match the same result many times, the view will use the same
blob URL each time. When the view closes, it revokes the URL. This seems like a
reasonable, natural lifetime for these URLs, and the implementation is simple.

Depends on D189452

Differential Revision: https://phabricator.services.mozilla.com/D189615
2023-10-05 04:36:12 +00:00
Drew Willcoxon
29454d2d80 Bug 1855615 - Make sure Suggest Glean telemetry is tested with the Rust backend enabled. r=daisuke
This updates the engagement Glean test so it checks Rust results.

It also adds a getter to `SuggestBackendRust` that returns a promise that's
resolved once the backend's initialization is fully complete. Setting
`quicksuggest.rustEnabled` will either trigger the Rust backend to perform
initialization and ingestion, or it will trigger the JS backend to re-sync all
features. Either way, tests need to wait until all of that is done.

Differential Revision: https://phabricator.services.mozilla.com/D189452
2023-09-29 01:02:33 +00:00
Drew Willcoxon
336e2ba93f Bug 1854060 - Initial integration of the Suggest Rust component into desktop urlbar. r=daisuke
This builds on D188681 and adds a new `BaseFeature` called `SuggestBackendRust`.
When `quickSuggestRustEnabled` is true, `UrlbarProviderQuickSuggest` will use
`SuggestBackendRust` to fetch remote settings suggestions; otherwise it will use
`SuggestBackendJs`.

The Rust component is already integrated into desktop Firefox (bug 1851256, bug
1851845), and it's exposed to JS via `RustSuggest.sys.mjs`. Currently it only
supports AMP (sponsored, a.k.a. adM) and Wikipedia (non-sponsored) suggestions.

It's possible to configure the path of the Sqlite file created by the Rust
component. This patch uses `suggest.sqlite` in the user's local profile (cache)
directory.

This is only the initial integration. I can think of a few follow-ups:

* Handle icons. In this patch, results from `SuggestBackendRust` don't have
  icons at all. I have a WIP.
* Handle ingestion better. "Ingest" here means Firefox must tell the Rust
  component to re-fetch suggestions from remote settings and rebuild its Sqlite
  database. Unfortunately the Rust component doesn't keep the data updated by
  itself, so we'll need to periodically tell the component to ingest. This patch
  performs ingestion every time `SuggestBackendRust` is (re)enabled, which is a
  good enough start.
* Maybe handle tests better. For now I modified the main quick suggest unit
  test, test_quicksuggest.js, so it tests both backends. Other tests should
  maybe be updated too, I'm not sure yet.

Depends on D188681

Differential Revision: https://phabricator.services.mozilla.com/D188684
2023-09-26 05:15:30 +00:00