From ea10d2a8725fccc9b7ba76e001f43fdfe4dd1fe8 Mon Sep 17 00:00:00 2001
From: Mugurell <Mugurell@users.noreply.github.com>
Date: Mon, 6 Jul 2020 20:06:21 +0300
Subject: [PATCH] For #11875 - Prevent mixup of region / locale based search
 engines

We have two search engine types:
- one based on MLS reported region,
- one based only on Locale.

There are multiple steps involved in returning the default search engine for
example and though at each step we could verify if a certain operation is
completed we are still exposed to concurrency issues.
Simplest and most effective way to make sure the MLS engines do not mix with
Locale based engines is to use the same type of engines for the entire duration
of the app. At the next cold start we'll verify again which engines to use.

Using the Locale based engines (fallbacks) is expected to only happen once, at
the first run of the application after being installed.
---
 .../searchengine/FenixSearchEngineProvider.kt       | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProvider.kt b/app/src/main/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProvider.kt
index 6d61f2440..8e1c28ed8 100644
--- a/app/src/main/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProvider.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProvider.kt
@@ -43,6 +43,13 @@ open class FenixSearchEngineProvider(
         )
     }
 
+    // We have two search engine types: one based on MLS reported region, one based only on Locale.
+    // There are multiple steps involved in returning the default search engine for example.
+    // Simplest and most effective way to make sure the MLS engines do not mix with Locale based engines
+    // is to use the same type of engines for the entire duration of the app's run.
+    // See fenix/issues/11875
+    private val isRegionCachedByLocationService = locationService.hasRegionCached()
+
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     open val localizationProvider: SearchLocalizationProvider =
         RegionSearchLocalizationProvider(locationService)
@@ -91,7 +98,7 @@ open class FenixSearchEngineProvider(
     // the main one hasn't completed yet
     private val searchEngines: Deferred<SearchEngineList>
         get() =
-            if (loadedSearchEngines.isCompleted) {
+            if (isRegionCachedByLocationService) {
                 loadedSearchEngines
             } else {
                 fallbackEngines
@@ -200,7 +207,7 @@ open class FenixSearchEngineProvider(
 
         if (!prefs.contains(installedEnginesKey)) {
             val searchEngines =
-                if (baseSearchEngines.isCompleted) baseSearchEngines
+                if (isRegionCachedByLocationService) baseSearchEngines
                 else fallbackEngines
 
             val defaultSet = searchEngines.await()
@@ -219,7 +226,7 @@ open class FenixSearchEngineProvider(
 
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     suspend fun localeAwareInstalledEnginesKey(): String {
-        val tag = if (loadedRegion.isCompleted) {
+        val tag = if (isRegionCachedByLocationService) {
             val localization = loadedRegion.await()
             val region = localization.region?.let {
                 if (it.isEmpty()) "" else "-$it"