Skip to content

Commit

Permalink
Explicitly set the default view for shield settings.
Browse files Browse the repository at this point in the history
Defaults to:
- advanced for existing users
- simple for new users

Changing the default was not "locking in" the setting. This change
locks in that default.

Fixes brave/brave-browser#8533
  • Loading branch information
bsclifton committed Mar 10, 2020
1 parent 595e027 commit 44a6c3b
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 6 deletions.
9 changes: 5 additions & 4 deletions browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,21 +103,22 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterBooleanPref(kHTTPSEVerywhereControlType, true);
registry->RegisterBooleanPref(kNoScriptControlType, false);
registry->RegisterBooleanPref(kAdControlType, true);
// > advanced view is defaulted to true for EXISTING users; false for new
bool is_new_user = false;

// advanced view is defaulted to true for EXISTING users; false for new.
// also see brave_profile_impl.cc for where default is locked in.
bool is_new_user = false;
#if !defined(OS_ANDROID)
is_new_user = first_run::IsChromeFirstRun();
#endif
registry->RegisterBooleanPref(kShieldsAdvancedViewEnabled,
is_new_user == false);

#if !BUILDFLAG(USE_GCM_FROM_PLATFORM)
// PushMessaging
registry->SetDefaultPrefValue(gcm::prefs::kGCMChannelStatus,
base::Value(false));
#endif

registry->RegisterBooleanPref(kShieldsAdvancedViewEnabled,
is_new_user == false);
registry->RegisterBooleanPref(kShieldsStatsBadgeVisible, true);
registry->RegisterBooleanPref(kGoogleLoginControlType, true);
registry->RegisterBooleanPref(kFBEmbedControlType, true);
Expand Down
15 changes: 13 additions & 2 deletions browser/brave_profile_prefs_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,28 @@ typedef FirstRunMasterPrefsBrowserTestT<kFirstRunEmptyPrefs>
BraveProfilePrefsFirstRunBrowserTest;
IN_PROC_BROWSER_TEST_F(BraveProfilePrefsFirstRunBrowserTest,
AdvancedShieldsNewUserValue) {
EXPECT_FALSE(
browser()->profile()->GetPrefs()->GetBoolean(
// verify value of pref (default to simple view)
EXPECT_FALSE(browser()->profile()->GetPrefs()->GetBoolean(
kShieldsAdvancedViewEnabled));
// verify that pref was set (and is not default)
const PrefService::Preference* pref =
browser()->profile()->GetPrefs()->FindPreference(
kShieldsAdvancedViewEnabled);
EXPECT_TRUE(pref->HasUserSetting());
}

// Existing Brave users should default shields to Advanced view
IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest,
AdvancedShieldsExistingUserValue) {
// verify value of pref (default to advanced view)
EXPECT_TRUE(
browser()->profile()->GetPrefs()->GetBoolean(
kShieldsAdvancedViewEnabled));
// verify that pref was set (and is not default)
const PrefService::Preference* pref =
browser()->profile()->GetPrefs()->FindPreference(
kShieldsAdvancedViewEnabled);
EXPECT_TRUE(pref->HasUserSetting());
}
#endif

Expand Down
13 changes: 13 additions & 0 deletions browser/profiles/brave_profile_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "content/public/browser/notification_source.h"
#include "brave/common/pref_names.h"

BraveProfileImpl::BraveProfileImpl(
const base::FilePath& path,
Expand All @@ -20,6 +21,18 @@ BraveProfileImpl::BraveProfileImpl(
scoped_refptr<base::SequencedTaskRunner> io_task_runner)
: ProfileImpl(path, delegate, create_mode, creation_time, io_task_runner),
weak_ptr_factory_(this) {
#if !defined(OS_ANDROID)
const PrefService::Preference* pref =
GetPrefs()->FindPreference(kShieldsAdvancedViewEnabled);
if (!pref->HasUserSetting()) {
bool default_value = GetPrefs()->GetBoolean(kShieldsAdvancedViewEnabled);
// advanced view is defaulted to true for EXISTING users; false for new.
// preference needs to be explicitly set to hold its value
// see brave_profile_prefs.cc for where this default is determined
GetPrefs()->SetBoolean(kShieldsAdvancedViewEnabled, default_value);
}
#endif

// 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

0 comments on commit 44a6c3b

Please sign in to comment.