From 01403f0fa06118b9cf50df2616e8a473b3cafa89 Mon Sep 17 00:00:00 2001 From: Pranjal Jumde Date: Wed, 15 Sep 2021 23:29:56 -0500 Subject: [PATCH] Fix #18152: Cast should be disabled on toggling off media router switch in extension settings --- browser/brave_prefs_browsertest.cc | 7 ++ browser/brave_profile_prefs.cc | 2 + .../api/settings_private/brave_prefs_util.cc | 2 +- browser/profiles/brave_profile_manager.cc | 19 ++++ .../brave_profile_manager_browsertest.cc | 46 ++++++++ .../brave_profile_manager_unittest.cc | 102 ++++++++++++++++++ .../brave_default_extensions_browser_proxy.js | 5 + .../brave_default_extensions_page.html | 8 +- .../brave_default_extensions_page.js | 4 + .../media/router/media_router_feature.cc | 10 +- .../settings_localized_strings_provider.cc | 18 ++-- common/pref_names.cc | 2 + common/pref_names.h | 6 ++ test/BUILD.gn | 1 + 14 files changed, 219 insertions(+), 13 deletions(-) create mode 100644 browser/profiles/brave_profile_manager_unittest.cc diff --git a/browser/brave_prefs_browsertest.cc b/browser/brave_prefs_browsertest.cc index c331eb9d899c..989b246e7cef 100644 --- a/browser/brave_prefs_browsertest.cc +++ b/browser/brave_prefs_browsertest.cc @@ -161,6 +161,13 @@ IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest, prefs::kHideWebStoreIcon)); } +IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest, MediaRouterPrefTest) { + EXPECT_FALSE(chrome_test_utils::GetProfile(this)->GetPrefs()->GetBoolean( + ::prefs::kEnableMediaRouter)); + EXPECT_FALSE(chrome_test_utils::GetProfile(this)->GetPrefs()->GetBoolean( + kEnableMediaRouterOnRestart)); +} + IN_PROC_BROWSER_TEST_F(BraveLocalStatePrefsBrowserTest, DefaultLocalStateTest) { #if !defined(OS_ANDROID) EXPECT_TRUE(g_browser_process->local_state()->GetBoolean( diff --git a/browser/brave_profile_prefs.cc b/browser/brave_profile_prefs.cc index 86e6b2d4a0f9..3996a5b90773 100644 --- a/browser/brave_profile_prefs.cc +++ b/browser/brave_profile_prefs.cc @@ -417,6 +417,8 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { registry->SetDefaultPrefValue(prefs::kEnableMediaRouter, base::Value(false)); + registry->RegisterBooleanPref(kEnableMediaRouterOnRestart, false); + RegisterProfilePrefsForMigration(registry); } diff --git a/browser/extensions/api/settings_private/brave_prefs_util.cc b/browser/extensions/api/settings_private/brave_prefs_util.cc index 24f8ce42893e..5cf7c825666f 100644 --- a/browser/extensions/api/settings_private/brave_prefs_util.cc +++ b/browser/extensions/api/settings_private/brave_prefs_util.cc @@ -272,7 +272,7 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetAllowlistedKeys() { #endif // Media router pref - (*s_brave_allowlist)[prefs::kEnableMediaRouter] = + (*s_brave_allowlist)[kEnableMediaRouterOnRestart] = settings_api::PrefType::PREF_TYPE_BOOLEAN; return *s_brave_allowlist; diff --git a/browser/profiles/brave_profile_manager.cc b/browser/profiles/brave_profile_manager.cc index 58c148bbf3f5..88376729128b 100644 --- a/browser/profiles/brave_profile_manager.cc +++ b/browser/profiles/brave_profile_manager.cc @@ -86,6 +86,25 @@ void BraveProfileManager::InitProfileUserPrefs(Profile* profile) { content_settings::BravePrefProvider::CopyPluginSettingsForMigration( profile->GetPrefs()); +// Chromecast is enabled by default on Android. +#if !defined(OS_ANDROID) + auto* pref_service = profile->GetPrefs(); + // At start, the value of kEnableMediaRouterOnRestart is updated to match + // kEnableMediaRouter so users don't lose their current setting + if (pref_service->FindPreference(kEnableMediaRouterOnRestart) + ->IsDefaultValue()) { + auto enabled = pref_service->GetBoolean(::prefs::kEnableMediaRouter); + pref_service->SetBoolean(kEnableMediaRouterOnRestart, enabled); + } else { + // For Desktop, kEnableMediaRouterOnRestart is used to track the current + // state of the media router switch in brave://settings/extensions. The + // value of kEnableMediaRouter is only updated to match + // kEnableMediaRouterOnRestart on restart + auto enabled = pref_service->GetBoolean(kEnableMediaRouterOnRestart); + pref_service->SetBoolean(::prefs::kEnableMediaRouter, enabled); + } +#endif + ProfileManager::InitProfileUserPrefs(profile); brave::RecordInitialP3AValues(profile); brave::SetDefaultSearchVersion(profile, profile->IsNewProfile()); diff --git a/browser/profiles/brave_profile_manager_browsertest.cc b/browser/profiles/brave_profile_manager_browsertest.cc index cfd4b293d91f..0aca307c5769 100644 --- a/browser/profiles/brave_profile_manager_browsertest.cc +++ b/browser/profiles/brave_profile_manager_browsertest.cc @@ -8,6 +8,7 @@ #include "base/strings/utf_string_conversions.h" #include "brave/browser/brave_ads/ads_service_factory.h" #include "brave/browser/brave_rewards/rewards_service_factory.h" +#include "brave/common/pref_names.h" #include "brave/components/ipfs/buildflags/buildflags.h" #include "brave/components/tor/buildflags/buildflags.h" #include "brave/components/tor/tor_constants.h" @@ -187,6 +188,51 @@ IN_PROC_BROWSER_TEST_F(BraveProfileManagerTest, #endif } +#if !defined(OS_ANDROID) +IN_PROC_BROWSER_TEST_F(BraveProfileManagerTest, + PRE_MediaRouterDisabledRestartTest) { + Profile* profile = + g_browser_process->profile_manager()->GetPrimaryUserProfile(); + { + profile->GetPrefs()->SetBoolean(::prefs::kEnableMediaRouter, true); + profile->GetPrefs()->SetBoolean(kEnableMediaRouterOnRestart, false); + EXPECT_TRUE(profile->GetPrefs()->GetBoolean(::prefs::kEnableMediaRouter)); + EXPECT_FALSE(profile->GetPrefs()->GetBoolean(kEnableMediaRouterOnRestart)); + } +} + +IN_PROC_BROWSER_TEST_F(BraveProfileManagerTest, + MediaRouterDisabledRestartTest) { + Profile* profile = + g_browser_process->profile_manager()->GetPrimaryUserProfile(); + { + EXPECT_FALSE(profile->GetPrefs()->GetBoolean(::prefs::kEnableMediaRouter)); + EXPECT_FALSE(profile->GetPrefs()->GetBoolean(kEnableMediaRouterOnRestart)); + } +} + +IN_PROC_BROWSER_TEST_F(BraveProfileManagerTest, + PRE_MediaRouterEnabledRestartTest) { + Profile* profile = + g_browser_process->profile_manager()->GetPrimaryUserProfile(); + { + profile->GetPrefs()->SetBoolean(::prefs::kEnableMediaRouter, false); + profile->GetPrefs()->SetBoolean(kEnableMediaRouterOnRestart, true); + EXPECT_FALSE(profile->GetPrefs()->GetBoolean(::prefs::kEnableMediaRouter)); + EXPECT_TRUE(profile->GetPrefs()->GetBoolean(kEnableMediaRouterOnRestart)); + } +} + +IN_PROC_BROWSER_TEST_F(BraveProfileManagerTest, MediaRouterEnabledRestartTest) { + Profile* profile = + g_browser_process->profile_manager()->GetPrimaryUserProfile(); + { + EXPECT_TRUE(profile->GetPrefs()->GetBoolean(::prefs::kEnableMediaRouter)); + EXPECT_TRUE(profile->GetPrefs()->GetBoolean(kEnableMediaRouterOnRestart)); + } +} +#endif + #if BUILDFLAG(ENABLE_TOR) IN_PROC_BROWSER_TEST_F(BraveProfileManagerTest, GetLastUsedProfileName) { diff --git a/browser/profiles/brave_profile_manager_unittest.cc b/browser/profiles/brave_profile_manager_unittest.cc new file mode 100644 index 000000000000..10b57e90c039 --- /dev/null +++ b/browser/profiles/brave_profile_manager_unittest.cc @@ -0,0 +1,102 @@ +// Copyright 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/. + +#include + +#include "base/files/scoped_temp_dir.h" +#include "brave/browser/profiles/brave_profile_manager.h" +#include "brave/common/pref_names.h" +#include "chrome/common/pref_names.h" +#include "chrome/test/base/scoped_testing_local_state.h" +#include "chrome/test/base/testing_browser_process.h" +#include "chrome/test/base/testing_profile.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" +#include "content/public/test/browser_task_environment.h" +#include "content/public/test/test_utils.h" +#include "testing/gtest/include/gtest/gtest.h" + +class BraveProfileManagerTest : public testing::Test { + public: + BraveProfileManagerTest() + : local_state_(TestingBrowserProcess::GetGlobal()) {} + + void SetUp() override { + ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); + TestingBrowserProcess::GetGlobal()->SetProfileManager( + CreateProfileManagerForTest()); + } + + void TearDown() override { + TestingBrowserProcess::GetGlobal()->SetProfileManager(nullptr); + content::RunAllTasksUntilIdle(); + } + + protected: + std::unique_ptr CreateProfileManagerForTest() { + return std::make_unique( + temp_dir_.GetPath()); + } + + base::FilePath GetTempPath() { return temp_dir_.GetPath(); } + + private: + base::ScopedTempDir temp_dir_; + content::BrowserTaskEnvironment task_environment_; + ScopedTestingLocalState local_state_; +}; + +TEST_F(BraveProfileManagerTest, EnableMediaRouterOnRestartDefaultValue) { + ProfileManager* profile_manager = g_browser_process->profile_manager(); + ASSERT_TRUE(profile_manager); + + base::FilePath path = GetTempPath().AppendASCII("profile"); + TestingProfile::Builder builder; + builder.SetPath(path); + builder.SetIsNewProfile(true); + std::unique_ptr profile = builder.Build(); + + auto* pref_service = profile->GetTestingPrefService(); + + pref_service->RemoveUserPref(kEnableMediaRouterOnRestart); + EXPECT_TRUE(pref_service->FindPreference(kEnableMediaRouterOnRestart) + ->IsDefaultValue()); + pref_service->SetBoolean(::prefs::kEnableMediaRouter, true); + profile_manager->InitProfileUserPrefs(profile.get()); + EXPECT_TRUE(pref_service->GetBoolean(kEnableMediaRouterOnRestart)); + + pref_service->RemoveUserPref(kEnableMediaRouterOnRestart); + EXPECT_TRUE(pref_service->FindPreference(kEnableMediaRouterOnRestart) + ->IsDefaultValue()); + pref_service->SetBoolean(::prefs::kEnableMediaRouter, false); + profile_manager->InitProfileUserPrefs(profile.get()); + EXPECT_FALSE(pref_service->GetBoolean(kEnableMediaRouterOnRestart)); +} + +TEST_F(BraveProfileManagerTest, EnableMediaRouterOnRestartNonDefaultValue) { + ProfileManager* profile_manager = g_browser_process->profile_manager(); + ASSERT_TRUE(profile_manager); + + base::FilePath path = GetTempPath().AppendASCII("profile"); + TestingProfile::Builder builder; + builder.SetPath(path); + builder.SetIsNewProfile(true); + std::unique_ptr profile = builder.Build(); + + auto* pref_service = profile->GetTestingPrefService(); + + EXPECT_FALSE(pref_service->FindPreference(kEnableMediaRouterOnRestart) + ->IsDefaultValue()); + pref_service->SetBoolean(kEnableMediaRouterOnRestart, true); + pref_service->SetBoolean(::prefs::kEnableMediaRouter, false); + profile_manager->InitProfileUserPrefs(profile.get()); + EXPECT_TRUE(pref_service->GetBoolean(kEnableMediaRouterOnRestart)); + + EXPECT_FALSE(pref_service->FindPreference(kEnableMediaRouterOnRestart) + ->IsDefaultValue()); + pref_service->SetBoolean(kEnableMediaRouterOnRestart, false); + pref_service->SetBoolean(::prefs::kEnableMediaRouter, true); + profile_manager->InitProfileUserPrefs(profile.get()); + EXPECT_FALSE(pref_service->GetBoolean(kEnableMediaRouterOnRestart)); +} diff --git a/browser/resources/settings/brave_default_extensions_page/brave_default_extensions_browser_proxy.js b/browser/resources/settings/brave_default_extensions_page/brave_default_extensions_browser_proxy.js index 431bb583fe67..3c0bae3b28f1 100644 --- a/browser/resources/settings/brave_default_extensions_page/brave_default_extensions_browser_proxy.js +++ b/browser/resources/settings/brave_default_extensions_page/brave_default_extensions_browser_proxy.js @@ -21,6 +21,7 @@ cr.define('settings', function () { isWidevineEnabled() {} getRestartNeeded () {} wasSignInEnabledAtStartup () {} + isMediaRouterEnabled () {} isDecentralizedDnsEnabled() {} getDecentralizedDnsResolveMethodList(provider) {} } @@ -70,6 +71,10 @@ cr.define('settings', function () { return loadTimeData.getBoolean('signInAllowedOnNextStartupInitialValue') } + isMediaRouterEnabled () { + return loadTimeData.getBoolean('isMediaRouterEnabled') + } + isDecentralizedDnsEnabled () { return cr.sendWithPromise('isDecentralizedDnsEnabled') } diff --git a/browser/resources/settings/brave_default_extensions_page/brave_default_extensions_page.html b/browser/resources/settings/brave_default_extensions_page/brave_default_extensions_page.html index 6821cf5868f5..fad5650216d5 100644 --- a/browser/resources/settings/brave_default_extensions_page/brave_default_extensions_page.html +++ b/browser/resources/settings/brave_default_extensions_page/brave_default_extensions_page.html @@ -87,9 +87,15 @@ +