Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix alternative search engine provider feature bugs #529

Merged
merged 7 commits into from
Oct 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ import("//build/config/features.gni")

source_set("browser_process") {
sources = [
"alternate_private_search_engine_controller.cc",
"alternate_private_search_engine_controller.h",
"alternate_private_search_engine_util.cc",
"alternate_private_search_engine_util.h",
"autocomplete/brave_autocomplete_scheme_classifier.cc",
"autocomplete/brave_autocomplete_scheme_classifier.h",
"brave_browser_main_extra_parts.cc",
Expand All @@ -31,6 +27,8 @@ source_set("browser_process") {
"component_updater/brave_component_installer.h",
"component_updater/brave_component_updater_configurator.cc",
"component_updater/brave_component_updater_configurator.h",
"guest_window_search_engine_provider_controller.cc",
"guest_window_search_engine_provider_controller.h",
"importer/brave_external_process_importer_client.cc",
"importer/brave_external_process_importer_client.h",
"importer/brave_external_process_importer_host.cc",
Expand All @@ -48,8 +46,14 @@ source_set("browser_process") {
"mac/sparkle_glue.mm",
"mac/sparkle_glue.h",
"mac/su_updater.h",
"private_window_search_engine_provider_controller.cc",
"private_window_search_engine_provider_controller.h",
"profile_creation_monitor.cc",
"profile_creation_monitor.h",
"search_engine_provider_controller_base.cc",
"search_engine_provider_controller_base.h",
"search_engine_provider_util.cc",
"search_engine_provider_util.h",
"update_util.cc",
"update_util.h",

Expand Down
49 changes: 0 additions & 49 deletions browser/alternate_private_search_engine_browsertest.cc

This file was deleted.

102 changes: 0 additions & 102 deletions browser/alternate_private_search_engine_controller.cc

This file was deleted.

49 changes: 0 additions & 49 deletions browser/alternate_private_search_engine_controller.h

This file was deleted.

30 changes: 0 additions & 30 deletions browser/alternate_private_search_engine_util.cc

This file was deleted.

25 changes: 0 additions & 25 deletions browser/alternate_private_search_engine_util.h

This file was deleted.

4 changes: 2 additions & 2 deletions browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#include "brave/browser/brave_profile_prefs.h"

#include "brave/browser/alternate_private_search_engine_util.h"
#include "brave/browser/search_engine_provider_util.h"
#include "brave/browser/themes/brave_theme_service.h"
#include "brave/common/pref_names.h"
#include "brave/browser/tor/tor_profile_service.h"
Expand All @@ -24,7 +24,7 @@ namespace brave {
void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
brave_shields::BraveShieldsWebContentsObserver::RegisterProfilePrefs(registry);

RegisterAlternatePrivateSearchEngineProfilePrefs(registry);
RegisterAlternativeSearchEngineProviderProfilePrefs(registry);

// appearance
BraveThemeService::RegisterProfilePrefs(registry);
Expand Down
59 changes: 59 additions & 0 deletions browser/guest_window_search_engine_provider_controller.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/* 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/guest_window_search_engine_provider_controller.h"

#include "base/auto_reset.h"
#include "brave/browser/search_engine_provider_util.h"
#include "chrome/browser/profiles/profile.h"
#include "components/search_engines/template_url_service.h"

GuestWindowSearchEngineProviderController::
GuestWindowSearchEngineProviderController(Profile* profile)
: SearchEngineProviderControllerBase(profile) {
DCHECK_EQ(profile->GetProfileType(), Profile::GUEST_PROFILE);

// Monitor otr(off the record) profile's search engine changing to tracking
// user's default search engine provider.
// OTR profile's service is used for that.
otr_template_url_service_->AddObserver(this);
ConfigureSearchEngineProvider();
}

GuestWindowSearchEngineProviderController::
~GuestWindowSearchEngineProviderController() {
otr_template_url_service_->RemoveObserver(this);
}

void GuestWindowSearchEngineProviderController::OnTemplateURLServiceChanged() {
if (ignore_template_url_service_changing_)
return;

// Prevent search engine changing from settings page for tor profile.
// TODO(simonhong): Revisit when related ux is determined.
if (otr_profile_->IsTorProfile()) {
base::AutoReset<bool> reset(&ignore_template_url_service_changing_, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't allow users to change search engine in tor window?
I thought we only set it to DDG by default for tor and then if users have their own preference, we respect their decisions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, making this optional is next task.

ChangeToAlternativeSearchEngineProvider();
return;
}


// The purpose of below code is turn off alternative prefs
// when user changes to different search engine provider from settings.
// However, this callback is also called during the TemplateURLService
// initialization phase. Because of this, guest view always starts with
// this prefs off state when browser restarted(persisted during the runtime).
// Currently I don't know how to determine who is caller of this callback.
// TODO(simonhong): Revisit here when brave's related ux is determined.
if (UseAlternativeSearchEngineProvider())
brave::ToggleUseAlternativeSearchEngineProvider(otr_profile_);
}

void
GuestWindowSearchEngineProviderController::ConfigureSearchEngineProvider() {
base::AutoReset<bool> reset(&ignore_template_url_service_changing_, true);
UseAlternativeSearchEngineProvider()
? ChangeToAlternativeSearchEngineProvider()
: ChangeToNormalWindowSearchEngineProvider();
}
Loading