Fix default search logic for Tor window and guest window

Fixes brave/brave-browser#6476 Specifically, fixes the engine when a previously set engine is no longer available. For example, in DE the engine used to be DDG (501), but was changed in the prepopulated list to DDG_DE(516). The value in the preferences will not be able to be used to get a valid engine (and cause a crash). This change checks if the currently selected engine is available and if not resets the value to the currently preset default.
brave · Oct 17, 2019 · c91a50e · c91a50e
1 parent 7fa87d1
commit c91a50e
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 23 deletions.
diff --git a/browser/search_engines/guest_window_search_engine_provider_service.cc b/browser/search_engines/guest_window_search_engine_provider_service.cc
@@ -38,13 +38,18 @@ void GuestWindowSearchEngineProviderService::OnTemplateURLServiceChanged() {
   if (ignore_template_url_service_changing_)
     return;
 
-  // The purpose of below code is togging alternative prefs
+  // The purpose of below code is toggling alternative prefs
   // when user changes from ddg to different search engine provider
   // (or vice versa) from settings ui.
-  const bool is_ddg_is_set =
-      otr_template_url_service_->GetDefaultSearchProvider()->
-          data().prepopulate_id ==
-      TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO;
+  bool is_ddg_is_set = false;
+  switch (otr_template_url_service_->GetDefaultSearchProvider()
+              ->data()
+              .prepopulate_id) {
+    case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO:
+    case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO_DE:
+    case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO_AU_NZ_IE:
+      is_ddg_is_set = true;
+  }
 
   if (UseAlternativeSearchEngineProvider() || is_ddg_is_set)
     brave::ToggleUseAlternativeSearchEngineProvider(otr_profile_);

diff --git a/browser/search_engines/tor_window_search_engine_provider_service.cc b/browser/search_engines/tor_window_search_engine_provider_service.cc
@@ -25,9 +25,7 @@ TorWindowSearchEngineProviderService(Profile* otr_profile)
 
   // Configure previously used provider because effective tor profile is
   // off the recored profile.
-  auto provider_data =
-      TemplateURLPrepopulateData::GetPrepopulatedEngine(
-          otr_profile->GetPrefs(), GetInitialSearchEngineProvider());
+  auto provider_data = GetInitialSearchEngineProvider(otr_profile->GetPrefs());
   TemplateURL provider_url(*provider_data);
   otr_template_url_service_->SetUserSelectedDefaultSearchProvider(
       &provider_url);
@@ -48,23 +46,41 @@ void TorWindowSearchEngineProviderService::OnTemplateURLServiceChanged() {
          data().prepopulate_id);
 }
 
-int TorWindowSearchEngineProviderService::
-GetInitialSearchEngineProvider() const {
+std::unique_ptr<TemplateURLData>
+TorWindowSearchEngineProviderService::GetInitialSearchEngineProvider(
+    PrefService* prefs) const {
+  std::unique_ptr<TemplateURLData> provider_data = nullptr;
   int initial_id = alternative_search_engine_provider_in_tor_.GetValue();
-
-  bool region_for_qwant =
-      TemplateURLPrepopulateData::GetPrepopulatedDefaultSearch(
-          otr_profile_->GetPrefs())->prepopulate_id ==
-          TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_QWANT;
+  if (initial_id !=
+      TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_INVALID) {
+    provider_data = std::move(
+        TemplateURLPrepopulateData::GetPrepopulatedEngine(prefs, initial_id));
+  }
 
   // If this is first run, |initial_id| is invalid. Then, use qwant or ddg
-  // depends on default prepopulate data.
-  if (initial_id ==
-      TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_INVALID) {
-    initial_id = region_for_qwant ?
-        TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_QWANT :
-        TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO;
+  // depends on default prepopulate data. If not, check that the initial_id
+  // returned data.
+  if (!provider_data) {
+    initial_id = TemplateURLPrepopulateData::GetPrepopulatedDefaultSearch(
+                     otr_profile_->GetPrefs())
+                     ->prepopulate_id;
+    switch (initial_id) {
+      case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_QWANT:
+      case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO:
+      case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO_DE:
+      case TemplateURLPrepopulateData::
+          PREPOPULATED_ENGINE_ID_DUCKDUCKGO_AU_NZ_IE:
+        break;
+
+      default:
+        initial_id =
+            TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO;
+        break;
+    }
+    provider_data = std::move(
+        TemplateURLPrepopulateData::GetPrepopulatedEngine(prefs, initial_id));
   }
 
-  return initial_id;
+  DCHECK(provider_data);
+  return provider_data;
 }
diff --git a/browser/search_engines/tor_window_search_engine_provider_service.h b/browser/search_engines/tor_window_search_engine_provider_service.h
@@ -9,6 +9,9 @@
 #include "components/prefs/pref_member.h"
 #include "components/search_engines/template_url_service_observer.h"
 
+class PrefService;
+struct TemplateURLData;
+
 // The purpose of this service for tor is making user changed search engine
 // provider persist across the sessions.
 // Also, BraveProfileManager::SetNonPersonalProfilePrefs() overrides for it.
@@ -23,7 +26,8 @@ class TorWindowSearchEngineProviderService
   // TemplateURLServiceObserver overrides:
   void OnTemplateURLServiceChanged() override;
 
-  int GetInitialSearchEngineProvider() const;
+  std::unique_ptr<TemplateURLData> GetInitialSearchEngineProvider(
+      PrefService* prefs) const;
 
   IntegerPrefMember alternative_search_engine_provider_in_tor_;