Skip to content

Commit

Permalink
Merge pull request #5407 from brave/simon_issue_9293
Browse files Browse the repository at this point in the history
Use temporal AutocompleteClassifier to get proper selection navigation url
  • Loading branch information
simonhong authored May 28, 2020
2 parents 345b685 + 49ea4ef commit 07ab4af
Show file tree
Hide file tree
Showing 15 changed files with 97 additions and 159 deletions.
2 changes: 0 additions & 2 deletions browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

11 changes: 4 additions & 7 deletions browser/autocomplete/brave_autocomplete_scheme_classifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,18 @@

#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)
#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) {
#if BUILDFLAG(ENABLE_BRAVE_WEBTORRENT)
profile_ = profile;
#endif
}

BraveAutocompleteSchemeClassifier::~BraveAutocompleteSchemeClassifier() {
Expand Down
13 changes: 10 additions & 3 deletions browser/autocomplete/brave_autocomplete_scheme_classifier.h
Original file line number Diff line number Diff line change
@@ -1,13 +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 <string>

#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;
Expand All @@ -16,7 +21,9 @@ class BraveAutocompleteSchemeClassifier : public ChromeAutocompleteSchemeClassif
const std::string& scheme) const override;

private:
Profile* profile_;
#if BUILDFLAG(ENABLE_BRAVE_WEBTORRENT)
Profile* profile_ = nullptr;
#endif

DISALLOW_COPY_AND_ASSIGN(BraveAutocompleteSchemeClassifier);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -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/. */

Expand All @@ -21,6 +22,9 @@ class GuestWindowSearchEngineProviderService
~GuestWindowSearchEngineProviderService() override;

private:
FRIEND_TEST_ALL_PREFIXES(SearchEngineProviderServiceTest,
GuestWindowControllerTest);

// TemplateURLServiceObserver overrides:
void OnTemplateURLServiceChanged() override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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));
Expand All @@ -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<GuestWindowSearchEngineProviderService*>(
SearchEngineProviderServiceFactory::GetForProfile(guest_profile));
search_engine_provider_service->OnTemplateURLServiceChanged();

// Check alternative pref is turned off.
EXPECT_FALSE(brave::UseAlternativeSearchEngineProviderEnabled(guest_profile));

Expand All @@ -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));
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ KeyedService* InitializeSearchEngineProviderServiceIfNeeded(Profile* profile) {

} // namespace

// static
SearchEngineProviderService* SearchEngineProviderServiceFactory::GetForProfile(
Profile* profile) {
return static_cast<SearchEngineProviderService*>(
GetInstance()->GetServiceForBrowserContext(profile, true));
}

// static
SearchEngineProviderServiceFactory*
SearchEngineProviderServiceFactory::GetInstance() {
Expand Down
17 changes: 13 additions & 4 deletions browser/search_engines/search_engine_provider_service_factory.h
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
/* 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"

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
Expand All @@ -18,6 +22,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>;
SearchEngineProviderServiceFactory();
Expand All @@ -36,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_
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 "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"
Expand All @@ -18,6 +22,31 @@
#undef RenderViewContextMenu
#define RenderViewContextMenu RenderViewContextMenu_Chromium

namespace {

GURL GetSelectionNavigationURL(Profile* profile, const base::string16& text) {
AutocompleteMatch match;
AutocompleteClassifier classifier(
std::make_unique<AutocompleteController>(
std::make_unique<BraveAutocompleteProviderClient>(profile),
AutocompleteClassifier::DefaultOmniboxProviders()),
std::make_unique<BraveAutocompleteSchemeClassifier>(profile));
classifier.Classify(text, false, false,
metrics::OmniboxEventProto::INVALID_SPEC, &match, NULL);
classifier.Shutdown();
return match.destination_url;
}

} // namespace

#define BRAVE_APPEND_SEARCH_PROVIDER \
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

Expand All @@ -27,6 +56,7 @@

// Make it clear which class we mean here.
#undef RenderViewContextMenu
#undef BRAVE_APPEND_SEARCH_PROVIDER

BraveRenderViewContextMenu::BraveRenderViewContextMenu(
content::RenderFrameHost* render_frame_host,
Expand Down

This file was deleted.

Loading

0 comments on commit 07ab4af

Please sign in to comment.