fixed crash with search extension

browser/search_engines/private_window_search_engine_provider_service.cc

@@ -6,46 +6,67 @@
 #include "brave/browser/search_engines/private_window_search_engine_provider_service.h"
 
 #include "chrome/browser/profiles/profile.h"
-#include "components/search_engines/template_url_service.h"
+#include "components/prefs/pref_service.h"
+#include "components/search_engines/default_search_manager.h"
+#include "components/search_engines/search_engines_pref_names.h"
+#include "components/search_engines/template_url.h"
 
 PrivateWindowSearchEngineProviderService::
 PrivateWindowSearchEngineProviderService(Profile* otr_profile)
     : SearchEngineProviderService(otr_profile) {
   DCHECK(otr_profile->IsIncognitoProfile());
 
+  const bool use_extension_provider = ShouldUseExtensionSearchProvider();
+  otr_profile->GetPrefs()->SetBoolean(prefs::kDefaultSearchProviderByExtension,
+                                      use_extension_provider);
+
+  if (use_extension_provider) {
+    UseExtensionSearchProvider();
+  } else {
+    ConfigureSearchEngineProvider();
+  }
+
   // Monitor normal profile's search engine changing because private window
   // should that search engine provider when alternative search engine isn't
   // used.
-  original_template_url_service_->AddObserver(this);
-  ConfigureSearchEngineProvider();
+  observation_.Observe(original_template_url_service_);
 }
 
 PrivateWindowSearchEngineProviderService::
-~PrivateWindowSearchEngineProviderService() {
-  original_template_url_service_->RemoveObserver(this);
-}
+    ~PrivateWindowSearchEngineProviderService() = default;
 
 void PrivateWindowSearchEngineProviderService::
 OnUseAlternativeSearchEngineProviderChanged() {
+  // If extension search provider is used, user can't change DSE by toggling.
+  if (ShouldUseExtensionSearchProvider())
+    return;
+
   ConfigureSearchEngineProvider();
 }
 
 void PrivateWindowSearchEngineProviderService::
 ConfigureSearchEngineProvider() {
+  DCHECK(!ShouldUseExtensionSearchProvider());
+
   UseAlternativeSearchEngineProvider()
       ? ChangeToAlternativeSearchEngineProvider()
       : ChangeToNormalWindowSearchEngineProvider();
 }
 
-void
-PrivateWindowSearchEngineProviderService::OnTemplateURLServiceChanged() {
-  // If private window uses alternative, search provider changing of normal
-  // profile should not affect private window's provider.
-  if (UseAlternativeSearchEngineProvider())
+void PrivateWindowSearchEngineProviderService::OnTemplateURLServiceChanged() {
+  const bool use_extension_provider = ShouldUseExtensionSearchProvider();
+  otr_profile_->GetPrefs()->SetBoolean(prefs::kDefaultSearchProviderByExtension,
+                                       use_extension_provider);
+
+  if (use_extension_provider) {
+    UseExtensionSearchProvider();
     return;
+  }
 
-  // When normal profile's default search provider is changed, apply it to
-  // private window's provider.
-  ChangeToNormalWindowSearchEngineProvider();
+  ConfigureSearchEngineProvider();
 }
 
+void PrivateWindowSearchEngineProviderService::Shutdown() {
+  SearchEngineProviderService::Shutdown();
+  observation_.Reset();
+}

browser/search_engines/private_window_search_engine_provider_service.h

@@ -1,11 +1,14 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* Copyright (c) 2019 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef BRAVE_BROWSER_SEARCH_ENGINES_PRIVATE_WINDOW_SEARCH_ENGINE_PROVIDER_SERVICE_H_
 #define BRAVE_BROWSER_SEARCH_ENGINES_PRIVATE_WINDOW_SEARCH_ENGINE_PROVIDER_SERVICE_H_
 
+#include "base/scoped_observation.h"
 #include "brave/browser/search_engines/search_engine_provider_service.h"
+#include "components/search_engines/template_url_service.h"
 #include "components/search_engines/template_url_service_observer.h"
 
 class PrivateWindowSearchEngineProviderService
@@ -14,6 +17,10 @@ class PrivateWindowSearchEngineProviderService
  public:
   explicit PrivateWindowSearchEngineProviderService(Profile* otr_profile);
   ~PrivateWindowSearchEngineProviderService() override;
+  PrivateWindowSearchEngineProviderService(
+      const PrivateWindowSearchEngineProviderService&) = delete;
+  PrivateWindowSearchEngineProviderService& operator=(
+      const PrivateWindowSearchEngineProviderService&) = delete;
 
  private:
   // Configure appropriate provider according to prefs.
@@ -24,8 +31,10 @@ class PrivateWindowSearchEngineProviderService
 
   // SearchEngineProviderService overrides:
   void OnUseAlternativeSearchEngineProviderChanged() override;
+  void Shutdown() override;
 
-  DISALLOW_COPY_AND_ASSIGN(PrivateWindowSearchEngineProviderService);
+  base::ScopedObservation<TemplateURLService, TemplateURLServiceObserver>
+      observation_{this};
 };
 
 #endif  // BRAVE_BROWSER_SEARCH_ENGINES_PRIVATE_WINDOW_SEARCH_ENGINE_PROVIDER_SERVICE_H_

browser/search_engines/search_engine_provider_service.cc

@@ -1,18 +1,29 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* Copyright (c) 2019 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "brave/browser/search_engines/search_engine_provider_service.h"
 
+#include <utility>
+#include <vector>
+
 #include "brave/browser/search_engines/search_engine_provider_util.h"
 #include "brave/common/pref_names.h"
 #include "brave/components/search_engines/brave_prepopulated_engines.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "components/prefs/pref_service.h"
 #include "components/search_engines/prepopulated_engines.h"
+#include "components/search_engines/search_engines_pref_names.h"
+#include "components/search_engines/template_url_data_util.h"
 #include "components/search_engines/template_url_prepopulate_data.h"
 #include "components/search_engines/template_url_service.h"
+#include "extensions/buildflags/buildflags.h"
+
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+#include "extensions/browser/extension_prefs.h"
+#endif
 
 SearchEngineProviderService::SearchEngineProviderService(
     Profile* otr_profile)
@@ -49,8 +60,7 @@ SearchEngineProviderService::SearchEngineProviderService(
   alternative_search_engine_url_.reset(new TemplateURL(*data));
 }
 
-SearchEngineProviderService::~SearchEngineProviderService() {
-}
+SearchEngineProviderService::~SearchEngineProviderService() = default;
 
 void SearchEngineProviderService::OnPreferenceChanged(
     const std::string& pref_name) {
@@ -76,3 +86,77 @@ void SearchEngineProviderService::ChangeToNormalWindowSearchEngineProvider() {
   otr_template_url_service_->SetUserSelectedDefaultSearchProvider(
       &normal_url);
 }
+
+void SearchEngineProviderService::UseExtensionSearchProvider() {
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+  DCHECK(ShouldUseExtensionSearchProvider());
+
+  const auto* extension_provider_url =
+      original_template_url_service_->GetDefaultSearchProvider();
+  DCHECK(extension_provider_url);
+  auto data = extension_provider_url->data();
+  data.id = kInvalidTemplateURLID;
+
+  // Can't add same turl again to service.
+  if (CouldAddExtensionTemplateURL(extension_provider_url)) {
+    auto type = extension_provider_url->type();
+    auto extension_id = extension_provider_url->GetExtensionId();
+    extensions::ExtensionPrefs* prefs =
+        extensions::ExtensionPrefs::Get(otr_profile_->GetOriginalProfile());
+    auto time = prefs->GetInstallTime(extension_id);
+
+    auto turl =
+        std::make_unique<TemplateURL>(data, type, extension_id, time, true);
+
+    otr_template_url_service_->Add(std::move(turl));
+  }
+
+  // Clear default provider's guid to prevent unnecessary
+  // |kDefaultSearchProviderDataPrefName| update when search provider's favicon
+  // url is updated. If this is not cleared,  previous non-extension default
+  // search provider's guid is stored.
+  // For example, TemplateURLService::MaybeUpdateDSEViaPrefs() could update
+  // |kDefaultSearchProviderDataPrefName| if previous default search
+  // provider is qwant and current one is from extension and user searches
+  // with qwant keyword(:q) and it's favicon url is updated.
+  // Why we should prevent this? If not prevented, this user pref is updated.
+  // Then, this update pref's data is loaded again by
+  // DefaultSearchManager::LoadDefaultSearchEngineFromPrefs().
+  // Here, we have different behavior from upstream.
+  // In upstream, DSM still get extension's provider data from
+  // |pref_service_->GetDictionary(kDefaultSearchProviderDataPrefName)| because
+  // extension controlled prefs has more higher priority than user set pref.
+  // So, still DefaultSearchManager::extension_default_search_| stores extension
+  // search provider.
+  // Howeveer, we manually set |kDefaultSearchProviderDataPrefName|. So,
+  // It's replaced with qwant provider. It was extension search provider.
+  // This only happens when |kSyncedDefaultSearchProviderGUID| and favicon
+  // udpated search provider is same. (See the condition in
+  // TemplateURLService::MaybeUpdateDSEViaPrefs())
+  // And there is no side-effect when this prefs update is skipped because
+  // it can be updated again when qwant is default search provider.
+  otr_profile_->GetPrefs()->SetString(prefs::kSyncedDefaultSearchProviderGUID,
+                                      data.sync_guid);
+
+  otr_profile_->GetPrefs()->Set(
+      DefaultSearchManager::kDefaultSearchProviderDataPrefName,
+      *TemplateURLDataToDictionary(data));
+#endif
+}
+
+bool SearchEngineProviderService::ShouldUseExtensionSearchProvider() const {
+  return original_template_url_service_->IsExtensionControlledDefaultSearch();
+}
+
+bool SearchEngineProviderService::CouldAddExtensionTemplateURL(
+    const TemplateURL* url) {
+  DCHECK(url);
+  DCHECK_NE(TemplateURL::NORMAL, url->type());
+  for (const auto* turl : otr_template_url_service_->GetTemplateURLs()) {
+    DCHECK(turl);
+    if (url->type() == turl->type() &&
+        url->GetExtensionId() == turl->GetExtensionId())
+      return false;
+  }
+  return true;
+}

browser/search_engines/search_engine_provider_service.h

@@ -1,4 +1,5 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* Copyright (c) 2019 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
@@ -8,7 +9,6 @@
 #include <memory>
 #include <string>
 
-#include "base/macros.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/prefs/pref_member.h"
 
@@ -21,6 +21,10 @@ class SearchEngineProviderService : public KeyedService {
   explicit SearchEngineProviderService(Profile* otr_profile);
   ~SearchEngineProviderService() override;
 
+  SearchEngineProviderService(const SearchEngineProviderService&) = delete;
+  SearchEngineProviderService& operator=(const SearchEngineProviderService&) =
+      delete;
+
  protected:
   // If subclass want to know and configure according to prefs change, override
   // this.
@@ -30,21 +34,22 @@ class SearchEngineProviderService : public KeyedService {
   void ChangeToAlternativeSearchEngineProvider();
   void ChangeToNormalWindowSearchEngineProvider();
 
+  bool ShouldUseExtensionSearchProvider() const;
+  void UseExtensionSearchProvider();
+
   // Points off the record profile.
-  Profile* otr_profile_;
+  Profile* otr_profile_ = nullptr;
   // Service for original profile of |otr_profile_|.
-  TemplateURLService* original_template_url_service_;
+  TemplateURLService* original_template_url_service_ = nullptr;
   // Service for off the record profile.
-  TemplateURLService* otr_template_url_service_;
-
-  std::unique_ptr<TemplateURL> alternative_search_engine_url_;
+  TemplateURLService* otr_template_url_service_ = nullptr;
 
  private:
   void OnPreferenceChanged(const std::string& pref_name);
+  bool CouldAddExtensionTemplateURL(const TemplateURL* url);
 
+  std::unique_ptr<TemplateURL> alternative_search_engine_url_;
   BooleanPrefMember use_alternative_search_engine_provider_;
-
-  DISALLOW_COPY_AND_ASSIGN(SearchEngineProviderService);
 };
 
 #endif  // BRAVE_BROWSER_SEARCH_ENGINES_SEARCH_ENGINE_PROVIDER_SERVICE_H_