From 8c790aee5e87d058f0a3e75d02b54b7dc10f91c5 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Tue, 28 Apr 2020 19:26:56 +0900 Subject: [PATCH 1/5] Use temporal AutocompleteClassifier to get proper navigation url To get proper selection navigation url for profile, use tempral AutocompleteClassfier. We can't use AutocompleteClassifierFactory for this because AutocompleteClassifierFactory doesn't create service for otr profile. But we need different navigation url for OTR profile. --- browser/BUILD.gn | 2 - ...utocomplete_provider_client_browsertest.cc | 44 ------------------- ...complete_provider_client_for_classifier.cc | 30 ------------- ...ocomplete_provider_client_for_classifier.h | 44 ------------------- .../brave_autocomplete_scheme_classifier.cc | 8 +--- .../autocomplete_classifier_factory.cc | 4 +- .../render_view_context_menu.cc | 28 ++++++++++++ ...e-autocomplete_classifier_factory.cc.patch | 17 ------- ...ext_menu-render_view_context_menu.cc.patch | 12 +++++ test/BUILD.gn | 1 - 10 files changed, 44 insertions(+), 146 deletions(-) delete mode 100644 browser/autocomplete/brave_autocomplete_provider_client_browsertest.cc delete mode 100644 browser/autocomplete/brave_autocomplete_provider_client_for_classifier.cc delete mode 100644 browser/autocomplete/brave_autocomplete_provider_client_for_classifier.h delete mode 100644 patches/chrome-browser-autocomplete-autocomplete_classifier_factory.cc.patch create mode 100644 patches/chrome-browser-renderer_context_menu-render_view_context_menu.cc.patch diff --git a/browser/BUILD.gn b/browser/BUILD.gn index 0d0b860ed556..982b7b3fe2be 100644 --- a/browser/BUILD.gn +++ b/browser/BUILD.gn @@ -31,8 +31,6 @@ source_set("browser_process") { sources = [ "autocomplete/brave_autocomplete_provider_client.cc", "autocomplete/brave_autocomplete_provider_client.h", - "autocomplete/brave_autocomplete_provider_client_for_classifier.cc", - "autocomplete/brave_autocomplete_provider_client_for_classifier.h", "autocomplete/brave_autocomplete_scheme_classifier.cc", "autocomplete/brave_autocomplete_scheme_classifier.h", "brave_shields/ad_block_pref_service_factory.cc", diff --git a/browser/autocomplete/brave_autocomplete_provider_client_browsertest.cc b/browser/autocomplete/brave_autocomplete_provider_client_browsertest.cc deleted file mode 100644 index 3225f7b49f81..000000000000 --- a/browser/autocomplete/brave_autocomplete_provider_client_browsertest.cc +++ /dev/null @@ -1,44 +0,0 @@ -/* Copyright 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/autocomplete/brave_autocomplete_provider_client_for_classifier.h" - -#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/ui/browser.h" -#include "chrome/test/base/in_process_browser_test.h" -#include "chrome/test/base/testing_browser_process.h" - -using BraveAutocompleteProviderClientTest = InProcessBrowserTest; - -// BraveAutocompleteProviderClient should only use different TemplateURLService. -// And Other service should be same for normal/incognito profile. -IN_PROC_BROWSER_TEST_F(BraveAutocompleteProviderClientTest, - DependentServiceCheckTest) { - Profile* profile = browser()->profile(); - Profile* otr_profile = profile->GetOffTheRecordProfile(); - - // Brave initiates different AutocompleteClassifier service for - // normal/incognito profile. - EXPECT_NE(AutocompleteClassifierFactory::GetForProfile(profile), - AutocompleteClassifierFactory::GetForProfile(otr_profile)); - - BraveAutocompleteProviderClientForClassifier normal_client(profile); - BraveAutocompleteProviderClientForClassifier incognito_client(otr_profile); - - // Check different TemplateURLService is used. - EXPECT_NE(normal_client.GetTemplateURLService(), - incognito_client.GetTemplateURLService()); - - // Check same services are used except TemplateURLService. - EXPECT_EQ(normal_client.GetAutocompleteClassifier(), - incognito_client.GetAutocompleteClassifier()); - EXPECT_EQ(normal_client.GetHistoryService(), - incognito_client.GetHistoryService()); - EXPECT_EQ(normal_client.GetRemoteSuggestionsService(true), - incognito_client.GetRemoteSuggestionsService(true)); - EXPECT_EQ(normal_client.GetDocumentSuggestionsService(true), - incognito_client.GetDocumentSuggestionsService(true)); -} diff --git a/browser/autocomplete/brave_autocomplete_provider_client_for_classifier.cc b/browser/autocomplete/brave_autocomplete_provider_client_for_classifier.cc deleted file mode 100644 index a8a9790bf5ed..000000000000 --- a/browser/autocomplete/brave_autocomplete_provider_client_for_classifier.cc +++ /dev/null @@ -1,30 +0,0 @@ -/* 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/autocomplete/brave_autocomplete_provider_client_for_classifier.h" - -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/search_engines/template_url_service_factory.h" - -BraveAutocompleteProviderClientForClassifier:: -BraveAutocompleteProviderClientForClassifier( - Profile* profile) - : BraveAutocompleteProviderClient(profile->GetOriginalProfile()), - profile_(profile) { -} - -BraveAutocompleteProviderClientForClassifier:: -~BraveAutocompleteProviderClientForClassifier() { -} - -TemplateURLService* - BraveAutocompleteProviderClientForClassifier::GetTemplateURLService() { - return TemplateURLServiceFactory::GetForProfile(profile_); -} - -const TemplateURLService* -BraveAutocompleteProviderClientForClassifier::GetTemplateURLService() const { - return TemplateURLServiceFactory::GetForProfile(profile_); -} diff --git a/browser/autocomplete/brave_autocomplete_provider_client_for_classifier.h b/browser/autocomplete/brave_autocomplete_provider_client_for_classifier.h deleted file mode 100644 index 11e528ae4624..000000000000 --- a/browser/autocomplete/brave_autocomplete_provider_client_for_classifier.h +++ /dev/null @@ -1,44 +0,0 @@ -/* Copyright (c) 2020 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_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_PROVIDER_CLIENT_FOR_CLASSIFIER_H_ -#define BRAVE_BROWSER_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_PROVIDER_CLIENT_FOR_CLASSIFIER_H_ - -#include "brave/browser/autocomplete/brave_autocomplete_provider_client.h" - -// In brave, different AutocompleteClassifiers are created for normal and -// incognito profile by changing -// AutocompleteClassifierFactory::GetBrowserContextToUse(). -// This changes are needed to use different search engine used by web search in -// web page context menu. -// When context menu handles web search it gets search engine url from -// ChromeAutocompleteProviderClient via AutocompleteClassifiers. -// Because of this, private window will use same search engine url of normal -// window if same AutocompleteClassifiers are used on normal and incognito. -// So, we made this change. -// However, ChromeAutocompleteProviderClient exposes many other services based -// on profiles. -// We don't want to change other services. Only wanted to get proper -// TemplateURLService. To achieve this, BraveAutocompleteProviderClient is -// introduced. It initializes ChromeAutocompleteProviderClient with original -// profile and only overrided TemplateURLService getter. -// BraveAutocompleteSchemeClassifier also initialize -// ChromeAutocompleteSchemeClassifier with original profile for same reason. -class BraveAutocompleteProviderClientForClassifier - : public BraveAutocompleteProviderClient { - public: - explicit BraveAutocompleteProviderClientForClassifier(Profile* profile); - ~BraveAutocompleteProviderClientForClassifier() override; - - TemplateURLService* GetTemplateURLService() override; - const TemplateURLService* GetTemplateURLService() const override; - - private: - Profile* profile_; - - DISALLOW_COPY_AND_ASSIGN(BraveAutocompleteProviderClientForClassifier); -}; - -#endif // BRAVE_BROWSER_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_PROVIDER_CLIENT_FOR_CLASSIFIER_H_ diff --git a/browser/autocomplete/brave_autocomplete_scheme_classifier.cc b/browser/autocomplete/brave_autocomplete_scheme_classifier.cc index d04ba21a8bcb..8f7200cac082 100644 --- a/browser/autocomplete/brave_autocomplete_scheme_classifier.cc +++ b/browser/autocomplete/brave_autocomplete_scheme_classifier.cc @@ -16,14 +16,10 @@ #include "brave/components/brave_webtorrent/browser/webtorrent_util.h" #endif -// See the BraveAutocompleteProviderClient why GetOriginalProfile() is fetched. -// All services except TemplateURLService exposed from AutocompleteClassifier -// uses original profile. So, |profile_| should be original profile same as -// base class does. BraveAutocompleteSchemeClassifier::BraveAutocompleteSchemeClassifier( Profile* profile) - : ChromeAutocompleteSchemeClassifier(profile->GetOriginalProfile()), - profile_(profile->GetOriginalProfile()) { + : ChromeAutocompleteSchemeClassifier(profile), + profile_(profile) { } BraveAutocompleteSchemeClassifier::~BraveAutocompleteSchemeClassifier() { diff --git a/chromium_src/chrome/browser/autocomplete/autocomplete_classifier_factory.cc b/chromium_src/chrome/browser/autocomplete/autocomplete_classifier_factory.cc index c35c05e4559a..25b589e00e6a 100644 --- a/chromium_src/chrome/browser/autocomplete/autocomplete_classifier_factory.cc +++ b/chromium_src/chrome/browser/autocomplete/autocomplete_classifier_factory.cc @@ -3,12 +3,12 @@ * 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/autocomplete/brave_autocomplete_provider_client_for_classifier.h" +#include "brave/browser/autocomplete/brave_autocomplete_provider_client.h" #include "brave/browser/autocomplete/brave_autocomplete_scheme_classifier.h" #include "components/omnibox/browser/autocomplete_classifier.h" #include "components/omnibox/browser/autocomplete_controller.h" -#define ChromeAutocompleteProviderClient BraveAutocompleteProviderClientForClassifier // NOLINT +#define ChromeAutocompleteProviderClient BraveAutocompleteProviderClient #define ChromeAutocompleteSchemeClassifier BraveAutocompleteSchemeClassifier #include "../../../../../chrome/browser/autocomplete/autocomplete_classifier_factory.cc" #undef ChromeAutocompleteSchemeClassifier diff --git a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc index a828069406c9..02ebbfd857c1 100644 --- a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc +++ b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc @@ -4,10 +4,14 @@ #include "chrome/browser/renderer_context_menu/render_view_context_menu.h" +#include "brave/browser/autocomplete/brave_autocomplete_provider_client.h" +#include "brave/browser/autocomplete/brave_autocomplete_scheme_classifier.h" #include "brave/browser/profiles/profile_util.h" #include "brave/browser/tor/buildflags.h" #include "brave/browser/translate/buildflags/buildflags.h" #include "brave/browser/renderer_context_menu/brave_spelling_options_submenu_observer.h" +#include "brave/components/omnibox/browser/brave_autocomplete_controller.h" +#include "components/omnibox/browser/autocomplete_classifier.h" #if BUILDFLAG(ENABLE_TOR) #include "brave/browser/tor/tor_profile_service.h" @@ -18,6 +22,29 @@ #undef RenderViewContextMenu #define RenderViewContextMenu RenderViewContextMenu_Chromium +namespace { + +GURL GetSelectionNavigationURL(Profile* profile, const base::string16& text) { + AutocompleteMatch match; + AutocompleteClassifier classifier( + std::make_unique( + std::make_unique(profile), + nullptr, AutocompleteClassifier::DefaultOmniboxProviders()), + std::make_unique(profile)); + classifier.Classify(text, false, false, + metrics::OmniboxEventProto::INVALID_SPEC, &match, NULL); + classifier.Shutdown(); + return match.destination_url; +} + +} // namespace + +#define BRAVE_APPEND_SEARCH_PROVIDER \ + selection_navigation_url_ = \ + GetSelectionNavigationURL(GetProfile(), params_.selection_text); \ + if (!selection_navigation_url_.is_valid()) \ + return; + // Use our subclass to initialize SpellingOptionsSubMenuObserver. #define SpellingOptionsSubMenuObserver BraveSpellingOptionsSubMenuObserver @@ -27,6 +54,7 @@ // Make it clear which class we mean here. #undef RenderViewContextMenu +#undef BRAVE_APPEND_SEARCH_PROVIDER BraveRenderViewContextMenu::BraveRenderViewContextMenu( content::RenderFrameHost* render_frame_host, diff --git a/patches/chrome-browser-autocomplete-autocomplete_classifier_factory.cc.patch b/patches/chrome-browser-autocomplete-autocomplete_classifier_factory.cc.patch deleted file mode 100644 index 264507f11ce2..000000000000 --- a/patches/chrome-browser-autocomplete-autocomplete_classifier_factory.cc.patch +++ /dev/null @@ -1,17 +0,0 @@ -diff --git a/chrome/browser/autocomplete/autocomplete_classifier_factory.cc b/chrome/browser/autocomplete/autocomplete_classifier_factory.cc -index c106fe59a239c026852617d842f2f2c1a789cb01..34b0207a815fdf71530b48a2a8664afd88ebcb00 100644 ---- a/chrome/browser/autocomplete/autocomplete_classifier_factory.cc -+++ b/chrome/browser/autocomplete/autocomplete_classifier_factory.cc -@@ -68,7 +68,12 @@ AutocompleteClassifierFactory::~AutocompleteClassifierFactory() { - - content::BrowserContext* AutocompleteClassifierFactory::GetBrowserContextToUse( - content::BrowserContext* context) const { -+#if defined(BRAVE_CHROMIUM_BUILD) -+ // See BraveAutocompleteProviderClient about why separate services are used. -+ return chrome::GetBrowserContextOwnInstanceInIncognito(context); -+#else - return chrome::GetBrowserContextRedirectedInIncognito(context); -+#endif - } - - bool AutocompleteClassifierFactory::ServiceIsNULLWhileTesting() const { diff --git a/patches/chrome-browser-renderer_context_menu-render_view_context_menu.cc.patch b/patches/chrome-browser-renderer_context_menu-render_view_context_menu.cc.patch new file mode 100644 index 000000000000..2963e625c7dd --- /dev/null +++ b/patches/chrome-browser-renderer_context_menu-render_view_context_menu.cc.patch @@ -0,0 +1,12 @@ +diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chrome/browser/renderer_context_menu/render_view_context_menu.cc +index bc682e44df3b6f38e00ace7eb535a7ab569469be..39b8ff5632afe0597cc0827030e3c45c22def4d5 100644 +--- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc ++++ b/chrome/browser/renderer_context_menu/render_view_context_menu.cc +@@ -1631,6 +1631,7 @@ void RenderViewContextMenu::AppendSearchProvider() { + selection_navigation_url_ = match.destination_url; + if (!selection_navigation_url_.is_valid()) + return; ++ BRAVE_APPEND_SEARCH_PROVIDER + + base::string16 printable_selection_text = PrintableSelectionText(); + EscapeAmpersands(&printable_selection_text); diff --git a/test/BUILD.gn b/test/BUILD.gn index 64337431dde4..1a0133eaf428 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -478,7 +478,6 @@ test("brave_browser_tests") { sources = [ "//brave/app/brave_main_delegate_browsertest.cc", "//brave/app/brave_main_delegate_runtime_flags_browsertest.cc", - "//brave/browser/autocomplete/brave_autocomplete_provider_client_browsertest.cc", "//brave/browser/brave_scheme_load_browsertest.cc", "//brave/browser/autoplay/autoplay_permission_context_browsertest.cc", "//brave/browser/brave_content_browser_client_browsertest.cc", From 04deb2955ba4922aff54052853ffabb8ddd062da Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 29 Apr 2020 11:35:07 +0900 Subject: [PATCH 2/5] Fix failed test cases and build error GuestWindowControllerTest was failed because OTR's TemplateURLService is not in loaded state while test case is running. If it's not in loaded state, it doesn't call observer. So, explicitly call OnTemplateURLServiceChanged(). --- .../brave_autocomplete_scheme_classifier.cc | 1 - .../brave_autocomplete_scheme_classifier.h | 3 +++ ...est_window_search_engine_provider_service.h | 3 +++ ...arch_engine_provider_service_browsertest.cc | 18 ++++++++++++++---- .../search_engine_provider_service_factory.cc | 7 +++++++ .../search_engine_provider_service_factory.h | 8 ++++++++ 6 files changed, 35 insertions(+), 5 deletions(-) diff --git a/browser/autocomplete/brave_autocomplete_scheme_classifier.cc b/browser/autocomplete/brave_autocomplete_scheme_classifier.cc index 8f7200cac082..e03dbc61fb85 100644 --- a/browser/autocomplete/brave_autocomplete_scheme_classifier.cc +++ b/browser/autocomplete/brave_autocomplete_scheme_classifier.cc @@ -9,7 +9,6 @@ #include "base/strings/string_util.h" #include "brave/common/url_constants.h" -#include "brave/components/brave_webtorrent/browser/buildflags/buildflags.h" #include "chrome/browser/profiles/profile.h" #if BUILDFLAG(ENABLE_BRAVE_WEBTORRENT) diff --git a/browser/autocomplete/brave_autocomplete_scheme_classifier.h b/browser/autocomplete/brave_autocomplete_scheme_classifier.h index 098b2824d633..cc2c6f6eab56 100644 --- a/browser/autocomplete/brave_autocomplete_scheme_classifier.h +++ b/browser/autocomplete/brave_autocomplete_scheme_classifier.h @@ -5,6 +5,7 @@ #ifndef BRAVE_BROWSER_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_SCHEME_CLASSIFIER_H_ #define BRAVE_BROWSER_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_SCHEME_CLASSIFIER_H_ +#include "brave/components/brave_webtorrent/browser/buildflags/buildflags.h" #include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h" class BraveAutocompleteSchemeClassifier : public ChromeAutocompleteSchemeClassifier { @@ -16,7 +17,9 @@ class BraveAutocompleteSchemeClassifier : public ChromeAutocompleteSchemeClassif const std::string& scheme) const override; private: +#if BUILDFLAG(ENABLE_BRAVE_WEBTORRENT) Profile* profile_; +#endif DISALLOW_COPY_AND_ASSIGN(BraveAutocompleteSchemeClassifier); }; diff --git a/browser/search_engines/guest_window_search_engine_provider_service.h b/browser/search_engines/guest_window_search_engine_provider_service.h index d5878f16e880..c71d17c68c64 100644 --- a/browser/search_engines/guest_window_search_engine_provider_service.h +++ b/browser/search_engines/guest_window_search_engine_provider_service.h @@ -21,6 +21,9 @@ class GuestWindowSearchEngineProviderService ~GuestWindowSearchEngineProviderService() override; private: + FRIEND_TEST_ALL_PREFIXES(SearchEngineProviderServiceTest, + GuestWindowControllerTest); + // TemplateURLServiceObserver overrides: void OnTemplateURLServiceChanged() override; diff --git a/browser/search_engines/search_engine_provider_service_browsertest.cc b/browser/search_engines/search_engine_provider_service_browsertest.cc index 495f33fe3a3a..44ddee859414 100644 --- a/browser/search_engines/search_engine_provider_service_browsertest.cc +++ b/browser/search_engines/search_engine_provider_service_browsertest.cc @@ -7,6 +7,8 @@ #include "brave/browser/brave_browser_process_impl.h" #include "brave/browser/profiles/brave_profile_manager.h" #include "brave/browser/profiles/profile_util.h" +#include "brave/browser/search_engines/guest_window_search_engine_provider_service.h" +#include "brave/browser/search_engines/search_engine_provider_service_factory.h" #include "brave/browser/search_engines/search_engine_provider_util.h" #include "brave/browser/tor/buildflags.h" #include "brave/browser/ui/browser_commands.h" @@ -209,7 +211,8 @@ IN_PROC_BROWSER_TEST_F(SearchEngineProviderServiceTest, if (brave::IsRegionForQwant(guest_profile)) return; - auto* service = TemplateURLServiceFactory::GetForProfile(guest_profile); + auto* template_service = + TemplateURLServiceFactory::GetForProfile(guest_profile); // alternative pref is initially disabled. EXPECT_FALSE(brave::UseAlternativeSearchEngineProviderEnabled(guest_profile)); @@ -220,14 +223,20 @@ IN_PROC_BROWSER_TEST_F(SearchEngineProviderServiceTest, TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO; // Check guest profile's search provider is set to ddg. - EXPECT_EQ(service->GetDefaultSearchProvider()->data().prepopulate_id, + EXPECT_EQ(template_service->GetDefaultSearchProvider()->data().prepopulate_id, provider_id); auto bing_data = TemplateURLPrepopulateData::GetPrepopulatedEngine( guest_profile->GetPrefs(), TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_BING); TemplateURL bing_url(*bing_data); - service->SetUserSelectedDefaultSearchProvider(&bing_url); + template_service->SetUserSelectedDefaultSearchProvider(&bing_url); + + auto* search_engine_provider_service = + static_cast( + SearchEngineProviderServiceFactory::GetForProfile(guest_profile)); + search_engine_provider_service->OnTemplateURLServiceChanged(); + // Check alternative pref is turned off. EXPECT_FALSE(brave::UseAlternativeSearchEngineProviderEnabled(guest_profile)); @@ -236,6 +245,7 @@ IN_PROC_BROWSER_TEST_F(SearchEngineProviderServiceTest, TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO); TemplateURL ddg_url(*ddg_data); - service->SetUserSelectedDefaultSearchProvider(&ddg_url); + template_service->SetUserSelectedDefaultSearchProvider(&ddg_url); + search_engine_provider_service->OnTemplateURLServiceChanged(); EXPECT_TRUE(brave::UseAlternativeSearchEngineProviderEnabled(guest_profile)); } diff --git a/browser/search_engines/search_engine_provider_service_factory.cc b/browser/search_engines/search_engine_provider_service_factory.cc index 9aaf3e5926f7..3dc7468a1e10 100644 --- a/browser/search_engines/search_engine_provider_service_factory.cc +++ b/browser/search_engines/search_engine_provider_service_factory.cc @@ -52,6 +52,13 @@ KeyedService* InitializeSearchEngineProviderServiceIfNeeded(Profile* profile) { } // namespace +// static +SearchEngineProviderService* SearchEngineProviderServiceFactory::GetForProfile( + Profile* profile) { + return static_cast( + GetInstance()->GetServiceForBrowserContext(profile, true)); +} + // static SearchEngineProviderServiceFactory* SearchEngineProviderServiceFactory::GetInstance() { diff --git a/browser/search_engines/search_engine_provider_service_factory.h b/browser/search_engines/search_engine_provider_service_factory.h index 7261991fa0e2..a524aae4f714 100644 --- a/browser/search_engines/search_engine_provider_service_factory.h +++ b/browser/search_engines/search_engine_provider_service_factory.h @@ -8,6 +8,9 @@ #include "base/memory/singleton.h" #include "components/keyed_service/content/browser_context_keyed_service_factory.h" +class Profile; +class SearchEngineProviderService; + // The purpose of this factory is to configure proper search engine provider to // private/guest/tor profile before it is referenced. // Also, this factory doesn't have public api. Instead, service is instantiated @@ -18,6 +21,11 @@ class SearchEngineProviderServiceFactory static SearchEngineProviderServiceFactory* GetInstance(); private: + FRIEND_TEST_ALL_PREFIXES(SearchEngineProviderServiceTest, + GuestWindowControllerTest); + // Only for test. + static SearchEngineProviderService* GetForProfile(Profile* profile); + friend struct base::DefaultSingletonTraits; SearchEngineProviderServiceFactory(); From 9908b06e0a4fcebaf63bb5994f6fc71823d5b591 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 29 Apr 2020 13:50:57 +0900 Subject: [PATCH 3/5] Fix lint error --- .../autocomplete/brave_autocomplete_scheme_classifier.h | 8 ++++++-- .../guest_window_search_engine_provider_service.h | 3 ++- .../search_engine_provider_service_factory.h | 9 +++++---- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/browser/autocomplete/brave_autocomplete_scheme_classifier.h b/browser/autocomplete/brave_autocomplete_scheme_classifier.h index cc2c6f6eab56..318da58c38f6 100644 --- a/browser/autocomplete/brave_autocomplete_scheme_classifier.h +++ b/browser/autocomplete/brave_autocomplete_scheme_classifier.h @@ -1,14 +1,18 @@ -/* 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_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_SCHEME_CLASSIFIER_H_ #define BRAVE_BROWSER_AUTOCOMPLETE_BRAVE_AUTOCOMPLETE_SCHEME_CLASSIFIER_H_ +#include + #include "brave/components/brave_webtorrent/browser/buildflags/buildflags.h" #include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h" -class BraveAutocompleteSchemeClassifier : public ChromeAutocompleteSchemeClassifier { +class BraveAutocompleteSchemeClassifier + : public ChromeAutocompleteSchemeClassifier { public: explicit BraveAutocompleteSchemeClassifier(Profile* profile); ~BraveAutocompleteSchemeClassifier() override; diff --git a/browser/search_engines/guest_window_search_engine_provider_service.h b/browser/search_engines/guest_window_search_engine_provider_service.h index c71d17c68c64..0311a99d924d 100644 --- a/browser/search_engines/guest_window_search_engine_provider_service.h +++ b/browser/search_engines/guest_window_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/. */ diff --git a/browser/search_engines/search_engine_provider_service_factory.h b/browser/search_engines/search_engine_provider_service_factory.h index a524aae4f714..a236b2e1396f 100644 --- a/browser/search_engines/search_engine_provider_service_factory.h +++ b/browser/search_engines/search_engine_provider_service_factory.h @@ -1,9 +1,10 @@ -/* 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_ENGINE_SEARCH_ENGINE_PROVIDER_SERVICE_FACTORY_H_ -#define BRAVE_BROWSER_SEARCH_ENGINE_SEARCH_ENGINE_PROVIDER_SERVICE_FACTORY_H_ +#ifndef BRAVE_BROWSER_SEARCH_ENGINES_SEARCH_ENGINE_PROVIDER_SERVICE_FACTORY_H_ +#define BRAVE_BROWSER_SEARCH_ENGINES_SEARCH_ENGINE_PROVIDER_SERVICE_FACTORY_H_ #include "base/memory/singleton.h" #include "components/keyed_service/content/browser_context_keyed_service_factory.h" @@ -44,4 +45,4 @@ class SearchEngineProviderServiceFactory DISALLOW_COPY_AND_ASSIGN(SearchEngineProviderServiceFactory); }; -#endif // BRAVE_BROWSER_SEARCH_ENGINE_SEARCH_ENGINE_PROVIDER_SERVICE_FACTORY_H_ +#endif // BRAVE_BROWSER_SEARCH_ENGINES_SEARCH_ENGINE_PROVIDER_SERVICE_FACTORY_H_ From 9cb6427b07123fa473692081addd95a625e8d793 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 29 Apr 2020 16:59:04 +0900 Subject: [PATCH 4/5] Fix build failure on android --- .../autocomplete/brave_autocomplete_scheme_classifier.cc | 6 ++++-- browser/autocomplete/brave_autocomplete_scheme_classifier.h | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/browser/autocomplete/brave_autocomplete_scheme_classifier.cc b/browser/autocomplete/brave_autocomplete_scheme_classifier.cc index e03dbc61fb85..c7f30f6e9290 100644 --- a/browser/autocomplete/brave_autocomplete_scheme_classifier.cc +++ b/browser/autocomplete/brave_autocomplete_scheme_classifier.cc @@ -17,8 +17,10 @@ BraveAutocompleteSchemeClassifier::BraveAutocompleteSchemeClassifier( Profile* profile) - : ChromeAutocompleteSchemeClassifier(profile), - profile_(profile) { + : ChromeAutocompleteSchemeClassifier(profile) { +#if BUILDFLAG(ENABLE_BRAVE_WEBTORRENT) + profile_ = profile; +#endif } BraveAutocompleteSchemeClassifier::~BraveAutocompleteSchemeClassifier() { diff --git a/browser/autocomplete/brave_autocomplete_scheme_classifier.h b/browser/autocomplete/brave_autocomplete_scheme_classifier.h index 318da58c38f6..6d1cb90f1d9b 100644 --- a/browser/autocomplete/brave_autocomplete_scheme_classifier.h +++ b/browser/autocomplete/brave_autocomplete_scheme_classifier.h @@ -22,7 +22,7 @@ class BraveAutocompleteSchemeClassifier private: #if BUILDFLAG(ENABLE_BRAVE_WEBTORRENT) - Profile* profile_; + Profile* profile_ = nullptr; #endif DISALLOW_COPY_AND_ASSIGN(BraveAutocompleteSchemeClassifier); From 49ea4ef62cbf104a2b5e07d6c334b1bbe55f7d09 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Thu, 30 Apr 2020 09:01:00 +0900 Subject: [PATCH 5/5] Don't need to get selection navigation url for normal profile again --- .../render_view_context_menu.cc | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc index 02ebbfd857c1..58063bc00024 100644 --- a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc +++ b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc @@ -10,8 +10,8 @@ #include "brave/browser/tor/buildflags.h" #include "brave/browser/translate/buildflags/buildflags.h" #include "brave/browser/renderer_context_menu/brave_spelling_options_submenu_observer.h" -#include "brave/components/omnibox/browser/brave_autocomplete_controller.h" #include "components/omnibox/browser/autocomplete_classifier.h" +#include "components/omnibox/browser/autocomplete_controller.h" #if BUILDFLAG(ENABLE_TOR) #include "brave/browser/tor/tor_profile_service.h" @@ -27,9 +27,9 @@ namespace { GURL GetSelectionNavigationURL(Profile* profile, const base::string16& text) { AutocompleteMatch match; AutocompleteClassifier classifier( - std::make_unique( + std::make_unique( std::make_unique(profile), - nullptr, AutocompleteClassifier::DefaultOmniboxProviders()), + AutocompleteClassifier::DefaultOmniboxProviders()), std::make_unique(profile)); classifier.Classify(text, false, false, metrics::OmniboxEventProto::INVALID_SPEC, &match, NULL); @@ -40,10 +40,12 @@ GURL GetSelectionNavigationURL(Profile* profile, const base::string16& text) { } // namespace #define BRAVE_APPEND_SEARCH_PROVIDER \ - selection_navigation_url_ = \ - GetSelectionNavigationURL(GetProfile(), params_.selection_text); \ - if (!selection_navigation_url_.is_valid()) \ - return; + if (GetProfile()->IsOffTheRecord()) { \ + selection_navigation_url_ = \ + GetSelectionNavigationURL(GetProfile(), params_.selection_text); \ + if (!selection_navigation_url_.is_valid()) \ + return; \ + } // Use our subclass to initialize SpellingOptionsSubMenuObserver. #define SpellingOptionsSubMenuObserver BraveSpellingOptionsSubMenuObserver