Skip to content

Commit

Permalink
Fixed crash from typing in tor window when SE extension is active
Browse files Browse the repository at this point in the history
fix brave/brave-browser#15140

When OnTemplateURLServiceChanged() is called, it tries to access default
search engine provider but it's null when search engine extension is installed
and that extension enabled to private window.
Those code is not valid now so just deleted because we can't change tor window's
search engine during the run time. It was possible ago but not now.
  • Loading branch information
simonhong committed Apr 8, 2021
1 parent 581f179 commit c01ea61
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 65 deletions.
5 changes: 5 additions & 0 deletions browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ void RegisterProfilePrefsForMigration(

// Restore "Other Bookmarks" migration
registry->RegisterBooleanPref(kOtherBookmarksMigrated, false);

// Added 04/2021
registry->RegisterIntegerPref(
kAlternativeSearchEngineProviderInTor,
TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_INVALID);
}

void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,4 @@ SearchEngineProviderServiceFactory::ServiceIsCreatedWithBrowserContext() const {
void SearchEngineProviderServiceFactory::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterBooleanPref(kUseAlternativeSearchEngineProvider, false);
registry->RegisterIntegerPref(
kAlternativeSearchEngineProviderInTor,
TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_INVALID);
}
74 changes: 21 additions & 53 deletions browser/search_engines/tor_window_search_engine_provider_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

#include "brave/browser/search_engines/tor_window_search_engine_provider_service.h"

#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 "components/search_engines/template_url_prepopulate_data.h"
Expand All @@ -16,69 +14,39 @@ TorWindowSearchEngineProviderService::
TorWindowSearchEngineProviderService(Profile* otr_profile)
: SearchEngineProviderService(otr_profile) {
DCHECK(otr_profile->IsTor());
DCHECK(otr_profile->IsOffTheRecord());

alternative_search_engine_provider_in_tor_.Init(
kAlternativeSearchEngineProviderInTor,
otr_profile->GetOriginalProfile()->GetPrefs());

// Configure previously used provider because effective tor profile is
// off the recored profile.
// Config default provider for tor window.
auto provider_data = GetInitialSearchEngineProvider(otr_profile->GetPrefs());
TemplateURL provider_url(*provider_data);
otr_template_url_service_->SetUserSelectedDefaultSearchProvider(
&provider_url);

// Monitor otr(off the record) profile's search engine changing to caching
// in original profile.
otr_template_url_service_->AddObserver(this);
}

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

void TorWindowSearchEngineProviderService::OnTemplateURLServiceChanged() {
alternative_search_engine_provider_in_tor_.SetValue(
otr_template_url_service_->GetDefaultSearchProvider()->
data().prepopulate_id);
}
TorWindowSearchEngineProviderService::~TorWindowSearchEngineProviderService() =
default;

std::unique_ptr<TemplateURLData>
TorWindowSearchEngineProviderService::GetInitialSearchEngineProvider(
PrefService* prefs) const {
std::unique_ptr<TemplateURLData> provider_data = nullptr;
int initial_id = alternative_search_engine_provider_in_tor_.GetValue();
if (initial_id !=
TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_INVALID) {
provider_data =
TemplateURLPrepopulateData::GetPrepopulatedEngine(prefs, initial_id);
}

// If this is first run, |initial_id| is invalid. Then, use qwant or ddg
// depends on default prepopulate data. If not, check that the initial_id
// returned data.
if (!provider_data) {
initial_id = TemplateURLPrepopulateData::GetPrepopulatedDefaultSearch(
otr_profile_->GetPrefs())
->prepopulate_id;
switch (initial_id) {
case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_QWANT:
case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO:
case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO_DE:
case TemplateURLPrepopulateData::
PREPOPULATED_ENGINE_ID_DUCKDUCKGO_AU_NZ_IE:
break;

default:
initial_id =
TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO;
break;
}
provider_data =
TemplateURLPrepopulateData::GetPrepopulatedEngine(prefs, initial_id);
std::unique_ptr<TemplateURLData> provider_data;

int initial_id = TemplateURLPrepopulateData::GetPrepopulatedDefaultSearch(
otr_profile_->GetPrefs())
->prepopulate_id;
switch (initial_id) {
case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_QWANT:
case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO:
case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO_DE:
case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO_AU_NZ_IE:
break;

default:
initial_id =
TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO;
break;
}
provider_data =
TemplateURLPrepopulateData::GetPrepopulatedEngine(prefs, initial_id);

DCHECK(provider_data);
return provider_data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
#define BRAVE_BROWSER_SEARCH_ENGINES_TOR_WINDOW_SEARCH_ENGINE_PROVIDER_SERVICE_H_

#include <memory>

#include "brave/browser/search_engines/search_engine_provider_service.h"
#include "components/prefs/pref_member.h"
#include "components/search_engines/template_url_service_observer.h"

class PrefService;
struct TemplateURLData;
Expand All @@ -18,21 +17,15 @@ struct TemplateURLData;
// provider persist across the sessions.
// Also, BraveProfileManager::SetNonPersonalProfilePrefs() overrides for it.
class TorWindowSearchEngineProviderService
: public SearchEngineProviderService,
public TemplateURLServiceObserver {
: public SearchEngineProviderService {
public:
explicit TorWindowSearchEngineProviderService(Profile* otr_profile);
~TorWindowSearchEngineProviderService() override;

private:
// TemplateURLServiceObserver overrides:
void OnTemplateURLServiceChanged() override;

std::unique_ptr<TemplateURLData> GetInitialSearchEngineProvider(
PrefService* prefs) const;

IntegerPrefMember alternative_search_engine_provider_in_tor_;

DISALLOW_COPY_AND_ASSIGN(TorWindowSearchEngineProviderService);
};

Expand Down
2 changes: 2 additions & 0 deletions browser/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import("//brave/browser/new_tab/sources.gni")
import("//brave/browser/permissions/sources.gni")
import("//brave/browser/signin/sources.gni")
import("//brave/browser/themes/sources.gni")
import("//brave/chromium_src/chrome/browser/prefs/sources.gni")

brave_chrome_browser_sources = []
brave_chrome_browser_sources += brave_browser_themes_sources
Expand All @@ -27,6 +28,7 @@ brave_chrome_browser_deps += brave_browser_new_tab_deps
brave_chrome_browser_deps += brave_browser_permissions_deps
brave_chrome_browser_deps += brave_browser_signin_deps
brave_chrome_browser_deps += brave_browser_brave_rewards_deps
brave_chrome_browser_deps += brave_chromium_src_chrome_browser_prefs_deps

brave_chrome_browser_public_deps = [
"//brave/components/brave_sync:constants",
Expand Down
4 changes: 4 additions & 0 deletions chromium_src/chrome/browser/prefs/browser_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "brave/browser/brave_profile_prefs.h"
#include "brave/browser/search/ntp_utils.h"
#include "brave/browser/themes/brave_dark_mode_utils.h"
#include "brave/common/pref_names.h"
#include "brave/components/brave_sync/brave_sync_prefs.h"
#include "chrome/browser/profiles/profile.h"
#include "components/gcm_driver/gcm_buildflags.h"
Expand Down Expand Up @@ -51,6 +52,9 @@ void MigrateObsoleteProfilePrefs(Profile* profile) {
#if !defined(OS_ANDROID)
new_tab_page::MigrateNewTabPagePrefs(profile);
#endif

// Added 04/2021
profile->GetPrefs()->ClearPref(kAlternativeSearchEngineProviderInTor);
}

// This method should be periodically pruned of year+ old migrations.
Expand Down
28 changes: 28 additions & 0 deletions chromium_src/chrome/browser/prefs/sources.gni
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright (c) 2021 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/.

import("//components/gcm_driver/config.gni")

# Fix: Added import loop. Need this import for using |enable_widevine| vars.
# import("//third_party/widevine/cdm/widevine.gni")

brave_chromium_src_chrome_browser_prefs_deps = [
"//brave/common:pref_names",
"//brave/components/brave_sync",
"//chrome/browser/profiles:profile",
"//components/gcm_driver:gcm_buildflags",
"//third_party/widevine/cdm:buildflags",
]

# |is_android| is used instead of |enable_widevine|. See above comment.
# We enable widevine except android.
if (!is_android) {
brave_chromium_src_chrome_browser_prefs_deps += [ "//brave/browser/widevine" ]
}

if (!use_gcm_from_platform) {
brave_chromium_src_chrome_browser_prefs_deps +=
[ "//brave/browser/gcm_driver" ]
}

0 comments on commit c01ea61

Please sign in to comment.