Skip to content

Commit

Permalink
Add a versioning mechanism for default search engines and backfill pr…
Browse files Browse the repository at this point in the history
…ofiles missing this version.

Fixes brave/brave-browser#12231
  • Loading branch information
bsclifton committed Nov 16, 2020
1 parent 4b81ec7 commit bd6d0cf
Show file tree
Hide file tree
Showing 13 changed files with 187 additions and 41 deletions.
28 changes: 17 additions & 11 deletions browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,30 @@
#include "brave/browser/ui/omnibox/brave_omnibox_client_impl.h"
#include "brave/common/pref_names.h"
#include "brave/components/binance/browser/buildflags/buildflags.h"
#include "brave/components/gemini/browser/buildflags/buildflags.h"
#include "brave/components/brave_ads/browser/ads_p2a.h"
#include "brave/components/brave_perf_predictor/browser/buildflags.h"
#include "brave/components/brave_rewards/common/pref_names.h"
#include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h"
#include "brave/components/brave_sync/brave_sync_prefs.h"
#include "brave/components/brave_rewards/common/pref_names.h"
#include "brave/components/brave_wallet/buildflags/buildflags.h"
#include "brave/components/brave_wayback_machine/buildflags.h"
#include "brave/components/brave_webtorrent/browser/buildflags/buildflags.h"
#include "brave/components/moonpay/browser/buildflags/buildflags.h"
#include "brave/components/crypto_dot_com/browser/buildflags/buildflags.h"
#include "brave/components/gemini/browser/buildflags/buildflags.h"
#include "brave/components/ipfs/buildflags/buildflags.h"
#include "brave/components/l10n/browser/locale_helper.h"
#include "brave/components/l10n/common/locale_util.h"
#include "brave/components/moonpay/browser/buildflags/buildflags.h"
#include "brave/components/search_engines/brave_prepopulated_engines.h"
#include "brave/components/speedreader/buildflags.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "chrome/browser/net/prediction_options.h"
#include "chrome/browser/prefs/session_startup_pref.h"
#include "chrome/common/pref_names.h"
#include "components/autofill/core/common/autofill_prefs.h"
#include "components/content_settings/core/common/pref_names.h"
#include "components/embedder_support/pref_names.h"
#include "components/gcm_driver/gcm_buildflags.h"
#include "components/autofill/core/common/autofill_prefs.h"
#include "components/password_manager/core/common/password_manager_pref_names.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
Expand Down Expand Up @@ -77,8 +78,8 @@
#endif

#if BUILDFLAG(ENABLE_BRAVE_PERF_PREDICTOR)
#include "brave/components/brave_perf_predictor/browser/perf_predictor_tab_helper.h"
#include "brave/components/brave_perf_predictor/browser/p3a_bandwidth_savings_tracker.h"
#include "brave/components/brave_perf_predictor/browser/perf_predictor_tab_helper.h"
#endif

#if !BUILDFLAG(USE_GCM_FROM_PLATFORM)
Expand Down Expand Up @@ -134,9 +135,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {

// appearance
registry->RegisterBooleanPref(kLocationBarIsWide, false);
registry->RegisterBooleanPref(
brave_rewards::prefs::kHideButton,
false);
registry->RegisterBooleanPref(brave_rewards::prefs::kHideButton, false);
registry->RegisterBooleanPref(kMRUCyclingEnabled, false);

brave_sync::Prefs::RegisterProfilePrefs(registry);
Expand Down Expand Up @@ -185,7 +184,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterBooleanPref(kSafetynetCheckFailed, false);
// clear default popular sites
registry->SetDefaultPrefValue(ntp_tiles::prefs::kPopularSitesJsonPref,
base::Value(base::Value::Type::LIST));
base::Value(base::Value::Type::LIST));
// Disable NTP suggestions
registry->SetDefaultPrefValue(feed::prefs::kEnableSnippets,
base::Value(false));
Expand All @@ -207,7 +206,8 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
// 2. On upgrade users might have enabled Media Router and the pref should
// be set correctly, so we use feature switch to set the initial value
#if BUILDFLAG(ENABLE_EXTENSIONS)
registry->RegisterBooleanPref(kBraveEnabledMediaRouter,
registry->RegisterBooleanPref(
kBraveEnabledMediaRouter,
FeatureSwitch::load_media_router_component_extension()->IsEnabled());
#endif

Expand Down Expand Up @@ -280,7 +280,8 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterIntegerPref(kBraveWalletPrefVersion, 0);
registry->RegisterStringPref(kBraveWalletAES256GCMSivNonce, "");
registry->RegisterStringPref(kBraveWalletEncryptedSeed, "");
registry->RegisterIntegerPref(kBraveWalletWeb3Provider,
registry->RegisterIntegerPref(
kBraveWalletWeb3Provider,
static_cast<int>(BraveWalletWeb3ProviderTypes::ASK));
registry->RegisterBooleanPref(kLoadCryptoWalletsOnStartup, false);
registry->RegisterBooleanPref(kOptedIntoCryptoWallets, false);
Expand Down Expand Up @@ -310,6 +311,11 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
registry->SetDefaultPrefValue(autofill::prefs::kAutofillWalletImportEnabled,
base::Value(false));

// Default search engine version
registry->RegisterIntegerPref(
kBraveDefaultSearchVersion,
TemplateURLPrepopulateData::kBraveCurrentDataVersion);

#if BUILDFLAG(ENABLE_SPEEDREADER)
speedreader::SpeedreaderService::RegisterPrefs(registry);
#endif
Expand Down
1 change: 1 addition & 0 deletions browser/profiles/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ source_set("profiles") {
"//components/gcm_driver:gcm_buildflags",
"//components/prefs",
"//components/safe_browsing/core/common:safe_browsing_prefs",
"//components/search_engines",
"//components/translate/core/browser",
"//third_party/blink/public/common",
"//ui/base",
Expand Down
5 changes: 0 additions & 5 deletions browser/profiles/brave_profile_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ BraveProfileImpl::BraveProfileImpl(
scoped_refptr<base::SequencedTaskRunner> io_task_runner)
: ProfileImpl(path, delegate, create_mode, creation_time, io_task_runner),
weak_ptr_factory_(this) {
// Profile can be loaded sync or async; if async, there is a matching block
// in `browser/profiles/brave_profile_manager.cc` (OnProfileCreated)
if (create_mode == CREATE_MODE_SYNCHRONOUS) {
brave::RecordInitialP3AValues(this);
}
// In sessions profiles, prefs are created from the original profile like how
// incognito profile works. By the time chromium start to observe prefs
// initialization in ProfileImpl constructor for the async creation case,
Expand Down
4 changes: 2 additions & 2 deletions browser/profiles/brave_profile_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ void BraveProfileManager::InitProfileUserPrefs(Profile* profile) {
InitTorProfileUserPrefs(profile);
} else {
ProfileManager::InitProfileUserPrefs(profile);
brave::RecordInitialP3AValues(profile);
brave::SetDefaultSearchVersion(profile, profile->IsNewProfile());
}
}

Expand Down Expand Up @@ -211,8 +213,6 @@ void BraveProfileManager::OnProfileCreated(Profile* profile,
TorProfileServiceFactory::GetForContext(profile);
}
#endif

brave::RecordInitialP3AValues(profile);
}

// This overridden method doesn't clear |kDefaultSearchProviderDataPrefName|.
Expand Down
12 changes: 12 additions & 0 deletions browser/profiles/profile_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
#include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h"
#include "base/no_destructor.h"
#include "brave/common/pref_names.h"
#include "brave/components/ntp_background_images/common/pref_names.h"
#include "brave/components/search_engines/brave_prepopulated_engines.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "brave/components/tor/tor_constants.h"
#include "chrome/browser/browser_process.h"
Expand Down Expand Up @@ -241,6 +243,16 @@ void RecordInitialP3AValues(Profile* profile) {
RecordSponsoredImagesEnabledP3A(profile);
}

void SetDefaultSearchVersion(Profile* profile, bool is_new_profile) {
const PrefService::Preference* pref_default_search_version =
profile->GetPrefs()->FindPreference(kBraveDefaultSearchVersion);
if (!pref_default_search_version->HasUserSetting()) {
profile->GetPrefs()->SetInteger(kBraveDefaultSearchVersion, is_new_profile
? TemplateURLPrepopulateData::kBraveCurrentDataVersion
: TemplateURLPrepopulateData::kBraveFirstTrackedDataVersion);
}
}

} // namespace brave

namespace chrome {
Expand Down
6 changes: 6 additions & 0 deletions browser/profiles/profile_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ void RecordSponsoredImagesEnabledP3A(Profile* profile);
// browser/brave_browser_main_extra_parts.cc
void RecordInitialP3AValues(Profile* profile);

// Used for capturing the value of kBraveCurrentDataVersion so that the
// default search engine for that version can be determined. New profiles
// will get locked into newer versions when created. Existing profiles
// missing this value are backfilled to the first version introduced.
void SetDefaultSearchVersion(Profile* profile, bool is_new_profile);

} // namespace brave

namespace chrome {
Expand Down
86 changes: 86 additions & 0 deletions browser/profiles/profile_util_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// 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/profiles/profile_util.h"

#include "base/files/file_path.h"
#include "base/files/scoped_temp_dir.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/profiles/profile_manager.h"
#include "chrome/test/base/scoped_testing_local_state.h"
#include "chrome/test/base/test_browser_window.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
#include "chrome/test/base/testing_profile_manager.h"
#include "components/prefs/pref_service.h"
#include "content/public/test/browser_task_environment.h"
#include "content/public/test/test_utils.h"
#include "testing/gtest/include/gtest/gtest.h"

class BraveProfileUtilTest : public testing::Test {
public:
BraveProfileUtilTest()
: testing_profile_manager_(TestingBrowserProcess::GetGlobal()) {}
~BraveProfileUtilTest() override {}

protected:
void SetUp() override {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
ASSERT_TRUE(testing_profile_manager_.SetUp(temp_dir_.GetPath()));
}

Profile* GetProfile() { return ProfileManager::GetActiveUserProfile(); }

PrefService* GetPrefs() {
return ProfileManager::GetActiveUserProfile()->GetPrefs();
}

content::BrowserTaskEnvironment task_environment_;
TestingProfileManager testing_profile_manager_;
base::ScopedTempDir temp_dir_;
};

// No entry yet. Check initialized value
TEST_F(BraveProfileUtilTest, SetDefaultSearchVersionExistingProfileNoEntryYet) {
const PrefService::Preference* pref =
GetPrefs()->FindPreference(kBraveDefaultSearchVersion);
EXPECT_TRUE(pref->IsDefaultValue());
brave::SetDefaultSearchVersion(GetProfile(), false);
ASSERT_EQ(GetPrefs()->GetInteger(kBraveDefaultSearchVersion),
TemplateURLPrepopulateData::kBraveFirstTrackedDataVersion);
}

TEST_F(BraveProfileUtilTest, SetDefaultSearchVersionNewProfileNoEntryYet) {
const PrefService::Preference* pref =
GetPrefs()->FindPreference(kBraveDefaultSearchVersion);
EXPECT_TRUE(pref->IsDefaultValue());
brave::SetDefaultSearchVersion(GetProfile(), true);
ASSERT_EQ(GetPrefs()->GetInteger(kBraveDefaultSearchVersion),
TemplateURLPrepopulateData::kBraveCurrentDataVersion);
}

// Entry there; ensure value is kept
TEST_F(BraveProfileUtilTest,
SetDefaultSearchVersionExistingProfileHasEntryKeepsValue) {
GetPrefs()->SetInteger(kBraveDefaultSearchVersion, 1);
const PrefService::Preference* pref =
GetPrefs()->FindPreference(kBraveDefaultSearchVersion);
EXPECT_FALSE(pref->IsDefaultValue());
brave::SetDefaultSearchVersion(GetProfile(), false);
ASSERT_EQ(GetPrefs()->GetInteger(kBraveDefaultSearchVersion), 1);
}

TEST_F(BraveProfileUtilTest,
SetDefaultSearchVersionNewProfileHasEntryKeepsValue) {
// This is an anomaly case; new profile won't ever have a hard set value
GetPrefs()->SetInteger(kBraveDefaultSearchVersion, 1);
const PrefService::Preference* pref =
GetPrefs()->FindPreference(kBraveDefaultSearchVersion);
EXPECT_FALSE(pref->IsDefaultValue());
brave::SetDefaultSearchVersion(GetProfile(), true);
ASSERT_EQ(GetPrefs()->GetInteger(kBraveDefaultSearchVersion), 1);
}
Loading

0 comments on commit bd6d0cf

Please sign in to comment.