WIP: Fixed crash with search provider extension

fix: brave/brave-browser#15224
fix: brave/brave-browser#10601
fix: brave/brave-browser#14218

Changed to use same TemplateURLService for normal & private profile.

browser/BUILD.gn

@@ -74,10 +74,6 @@ source_set("browser_process") {
     "geolocation/brave_geolocation_permission_context_delegate.h",
     "metrics/metrics_reporting_util.cc",
     "metrics/metrics_reporting_util.h",
-    "search_engines/guest_window_search_engine_provider_service.cc",
-    "search_engines/guest_window_search_engine_provider_service.h",
-    "search_engines/private_window_search_engine_provider_service.cc",
-    "search_engines/private_window_search_engine_provider_service.h",
     "search_engines/search_engine_provider_service.cc",
     "search_engines/search_engine_provider_service.h",
     "search_engines/search_engine_provider_service_factory.cc",
@@ -86,8 +82,6 @@ source_set("browser_process") {
     "search_engines/search_engine_provider_util.h",
     "search_engines/search_engine_tracker.cc",
     "search_engines/search_engine_tracker.h",
-    "search_engines/tor_window_search_engine_provider_service.cc",
-    "search_engines/tor_window_search_engine_provider_service.h",
     "update_util.cc",
     "update_util.h",
   ]

browser/search_engines/search_engine_provider_service.cc

@@ -12,15 +12,11 @@
 #include "components/prefs/pref_service.h"
 #include "components/search_engines/prepopulated_engines.h"
 #include "components/search_engines/template_url_prepopulate_data.h"
-#include "components/search_engines/template_url_service.h"
 
 SearchEngineProviderService::SearchEngineProviderService(
     Profile* otr_profile)
     : otr_profile_(otr_profile),
-      original_template_url_service_(
-          TemplateURLServiceFactory::GetForProfile(
-              otr_profile_->GetOriginalProfile())),
-      otr_template_url_service_(
+      template_url_service_(
           TemplateURLServiceFactory::GetForProfile(otr_profile_)) {
   use_alternative_search_engine_provider_.Init(
       kUseAlternativeSearchEngineProvider,
@@ -47,17 +43,49 @@ SearchEngineProviderService::SearchEngineProviderService(
   // There should ALWAYS be one entry
   DCHECK(data);
   alternative_search_engine_url_.reset(new TemplateURL(*data));
+  observation_.Observe(template_url_service_);
 }
 
-SearchEngineProviderService::~SearchEngineProviderService() {
+SearchEngineProviderService::~SearchEngineProviderService() = default;
+
+void SearchEngineProviderService::OnTemplateURLServiceChanged() {
+  // When this change is came from pref changing, we don't need to control
+  // prefs again.
+  if (ignore_template_url_service_changing_)
+    return;
+
+  // 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.
+  bool is_ddg_is_set = false;
+  switch (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;
+      break;
+    default:
+      break;
+  }
+
+  if (UseAlternativeSearchEngineProvider() || is_ddg_is_set)
+    brave::ToggleUseAlternativeSearchEngineProvider(otr_profile_);
 }
 
 void SearchEngineProviderService::OnPreferenceChanged(
     const std::string& pref_name) {
   DCHECK(pref_name == kUseAlternativeSearchEngineProvider);
-  DCHECK(!brave::IsRegionForQwant(otr_profile_));
 
-  OnUseAlternativeSearchEngineProviderChanged();
+  // When this call is from setting's change, we don't need to set provider
+  // again.
+  if (ignore_template_url_service_changing_)
+    return;
+
+  base::AutoReset<bool> reset(&ignore_template_url_service_changing_, true);
+  UseAlternativeSearchEngineProvider()
+      ? ChangeToAlternativeSearchEngineProvider()
+      : ChangeToNormalWindowSearchEngineProvider();
 }
 
 bool
@@ -66,13 +94,10 @@ SearchEngineProviderService::UseAlternativeSearchEngineProvider() const {
 }
 
 void SearchEngineProviderService::ChangeToAlternativeSearchEngineProvider() {
-  otr_template_url_service_->SetUserSelectedDefaultSearchProvider(
+  template_url_service_->SetUserSelectedDefaultSearchProvider(
       alternative_search_engine_url_.get());
 }
 
 void SearchEngineProviderService::ChangeToNormalWindowSearchEngineProvider() {
-  TemplateURL normal_url(
-      original_template_url_service_->GetDefaultSearchProvider()->data());
-  otr_template_url_service_->SetUserSelectedDefaultSearchProvider(
-      &normal_url);
+  template_url_service_->SetUserSelectedDefaultSearchProvider(nullptr);
 }

browser/search_engines/search_engine_provider_service.h

@@ -8,42 +8,41 @@
 #include <memory>
 #include <string>
 
-#include "base/macros.h"
+#include "base/scoped_observation.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/prefs/pref_member.h"
+#include "components/search_engines/template_url_service.h"
+#include "components/search_engines/template_url_service_observer.h"
 
 class Profile;
 class TemplateURL;
 class TemplateURLService;
 
-class SearchEngineProviderService : public KeyedService {
+// TODO(simonhong): Rename this to GuestWindowSearchEngineProviderService.
+class SearchEngineProviderService : public KeyedService,
+                                    public TemplateURLServiceObserver {
  public:
   explicit SearchEngineProviderService(Profile* otr_profile);
   ~SearchEngineProviderService() override;
 
- protected:
-  // If subclass want to know and configure according to prefs change, override
-  // this.
-  virtual void OnUseAlternativeSearchEngineProviderChanged() {}
-
-  bool UseAlternativeSearchEngineProvider() const;
-  void ChangeToAlternativeSearchEngineProvider();
-  void ChangeToNormalWindowSearchEngineProvider();
+  // TemplateURLServiceObserver overrides:
+  void OnTemplateURLServiceChanged() override;
 
+ private:
   // Points off the record profile.
   Profile* otr_profile_;
-  // Service for original profile of |otr_profile_|.
-  TemplateURLService* original_template_url_service_;
-  // Service for off the record profile.
-  TemplateURLService* otr_template_url_service_;
-
+  TemplateURLService* template_url_service_;
   std::unique_ptr<TemplateURL> alternative_search_engine_url_;
+  BooleanPrefMember use_alternative_search_engine_provider_;
+  base::ScopedObservation<TemplateURLService, TemplateURLServiceObserver>
+      observation_{this};
+  bool ignore_template_url_service_changing_ = false;
 
- private:
+  bool UseAlternativeSearchEngineProvider() const;
+  void ChangeToAlternativeSearchEngineProvider();
+  void ChangeToNormalWindowSearchEngineProvider();
   void OnPreferenceChanged(const std::string& pref_name);
 
-  BooleanPrefMember use_alternative_search_engine_provider_;
-
   DISALLOW_COPY_AND_ASSIGN(SearchEngineProviderService);
 };