Fenix: FNX2-13658 ⁃ [Bug]Wrong default search engine displayed on startup

[AndiAJ]

Prerequisites

Have a fresh install or a clean profile
Have the Language and input setting set to English - US

Steps to reproduce

1. Launch Fenix
2. Check the default search engine icon displayed in Nav bar

Expected behavior

Google's icon should be displayed in the Nav bar

Actual behavior

Amazon's icon is displayed
After you tap "Start browsing" and dismiss the search view the correct default search engine is displayed

Device information

• Android device:
  • Google Pixel 3a (Android 10)
  • OnePlus A3 (Android 6.0.1)
• Fenix version: Nightly Build 200624 from 6/24

Notes

► Video

[AndiAJ]

The Amazon engine is set after force close + resume on 79.0.0-beta.3 using the following devices:
• Google Pixel 3a (Android 10)
• OnePlus A3 (Android 6.0.1)

► Video

[sv-ohorvath]

I didn't only see the amazon icon in the search bar but also had it set as default and Google still missing from the engines list. Once I've done a search using amazon, it stays as default even after another restart. Not 100% reproducible, maybe 1/6.
So I'd say this is still the same as https://github.com/mozilla-mobile/fenix/issues/12081
device: Pixel 4 (Android 10)

[liuche]

(adding these labels for tracking, but if this is an edge case, we may skip it for a release blocker, and only do this in the fast-follow release that is this current sprint)

[liuche]

Hmm, seems like it could be a timing issue. Does this still happen if the user doesn't restart immediately after? I think @boek investigated it briefly.

Is this something that we could retest on the 79-beta.4 build?

What do you think, @vesta0 ?

[liuche]

@Mugurell is this something you and your team could look at during your day?

[AndiAJ]

Hi @liuche I've just re-checked this on 79.0.0-beta.4 from 7/1.

✔️ Can't reproduce on: Huawei Mate 20 Lite (Android 9)
❌ Reproducible on :
• Google Pixel 3a (Android 10)
• OnePlus A3 (Android 6.0.1)

Each time the user opens Fenix after previously closing it Amazon will get displayed.
► Video

Deleting Amazon from the Search engine will display Bing (and so on) even if the default search engine is set to Google
► Video

[pocmo]

I also see this in Nightly for every fresh app start.

[Mugurell]

This is really interesting.
Indeed, it happens on Nightly, but we couldn't get it to reproduce on the same device(and configuration) while using a debug build.
@boek Seeing you worked on this recently, can you help tracking down the issue?

[sv-ohorvath]

So it looks like there is a difference between the Firefox Preview Beta (PS build) and Fenix.fennec-beta (migration) builds:

Scenario 1:

• on the PlayStore build, the Amazon icon is just displayed at every fresh start, then it goes away (what happens on Nightly as well).
• this doesn't happen on the migration build.

Scenario 2:

• on the fennec-beta migration build, for a first-time install (no fennec app installed previously): if you kill the app while you are on the Fenix onboarding page and then restart, Google is not in the default engines list and Amazon is set as default. => so it's an edge case for first-time users only.
• this happens on the PS Firefox Preview Beta build, but only after you press Start browsing after the restart...

*If you had Fennec installed and migrate to Fenix, this doesn't happen.

[Mugurell]

This is really interesting.
Indeed, it happens on Nightly, but we couldn't get it to reproduce on the same device(and configuration) while using a debug build.

I can reproduce the same behavior as for Nightly if configuring the debug build to use the same MLS here so the issue may be related to the region/country/language MLS returns or how we use that data.

[Mugurell]

The issue is that refreshAsync() takes too much while based on MLS we get the location aware identifiers early.
-> we get to filter fallback engines by location identifiers.
-> google-b-1-m which is only available for US is filtered out for other regions

We could come up with a simple patch to also use the fallback identifiers..
but I'm be more interested in reducing the time refreshAsync() takes to complete or somehow synchronize the code/calls/ui updates so that we won't be showing the fallback search engines too often.

[liuche]

We talked this over and decided that we can ship this to Beta (so that we can get the perf experiment #7795 running), but want it to be fixed for Release, so we can take it as an uplift.

[Mugurell]

One important detail is that this issue is possible due to Google having it's identifier overridden for US to be "google-b-1-m"
If using the fallback engine identifiers (on a device using English-US) but with another region (based on MLS) based engines which for Google probably use "google-b-m" the Google search engine will be filtered out.

Thought I'd be able to sync the various calls but this leads to a longer startup time, something that we wanted to avoid in the first place - (see #9935)
To avoid the issue from #9935 we are trying to be as fast as possible and use many various conditions to decide whether to use the fallback engines / identifiers or not.
"Synchronizing" the behavior on more such checks leads many times to improper combinations of engines/identifiers and even allows for the fallbackEngines to be saved in the proper localeAwareInstalledEnginesKey and so perpetually have the wrong set of engines (if loadedRegion.isCompleted and baseSearchEngines.isCompleted.not()) for the app. (Users can of course still update the engines)

What I suggest (based on Sebastian's comment - https://github.com/mozilla-mobile/fenix/issues/9935#issuecomment-617633354) is to:

• accept that at the first app start (ever) we will have to probably show the fallback engines.
• refactor our code to choose whether to show the fallback or not engines based on a single check (used for the above) and so avoid having mix-matched engines and identifiers.
• even if showing the fallback engines, continue in the background to compute the region based list and persist that for the entire Fenix's lifetime.
• refactor our code so that at the next app start MLS is not used anymore. We already have persisted the region based engines, no need to redo this long time operation. Avoiding having to wait for MLS again and reading the already persisted engines means a faster startup and no need for other complex synchronizations.

Curious about your thoughts @boek

[ghost]

Do you think this bug is somehow related to this:
https://github.com/mozilla-mobile/fenix/issues/12117

[vesta0]

@SS1113 that could be related, thanks for pointing that out.

[Mugurell]

Also did some light profiling with tracing methods and saw that to return the default engine on first app start (no cached values):

• using MLS I get it after 1.3 seconds
• using fallbacks I get it after 0.45 seconds
  (The above values are not indicative of the actual performance since tracing method calls introduces an important overhead)

For getting the default engine we are calling multiple methods which check for other previous calls being completed.
With the big execution time disparity from above it's easy to see how fallback values could alter the end result.

To keep things simple I plan to "ask" MLS if it has a cached region and if so wait for the engines/default engine result based on MLS, and if not - use the fallback values.

[liuche]

When we get to QA, we need to make sure that this also fixes #11906 ! and if not, we'll need to find and uplift a separate fix for that.

[Mugurell]

This should be ready to test in the next Nightly.

[Augures01]

Now, when I add custom DuckDuckGo search, it won't appears until browser restart

Steps:

1. Clean install
2. Add DDG with russian locale
3. "DDG added" hint appears, but no new engine added
4. Restart Fenix - DDG appears in list

Hmmm... but

1. Clean install
2. Restart Fenix
3. Add DDG and it's appears in list

[Mugurell]

Thank you. Looking into this.

[sv-ohorvath]

@Mugurell There is also this issue on 79.0-beta6:

1. The device language is Turkish or Russian.
2. Clean install.
3. On the onboarding screen & first run, until restart, Yandex is the default search engine.
4. Restart the app and then Google is the default engine.

The difference between beta 5 & 6 is that before, the Yandex logo was only shown on the onboarding screen and disappeared when you tapped the search bar.

On EN-US I didn't see any issues.

[sv-ohorvath]

Talked with @Mugurell and decided to create a new issue for the remaining bugs since this was already merged: https://github.com/mozilla-mobile/fenix/issues/12544
So I'll close this one.

[Mugurell]

Closing as per https://github.com/mozilla-mobile/fenix/issues/11875#event-3543896873