Skip to content

Commit

Permalink
Merge pull request #10350 from brave/cast_disable_1.31.x
Browse files Browse the repository at this point in the history
Fix #18152: Cast should be disabled on toggling off media router switch in extension settings (uplift to 1.31.x)
  • Loading branch information
kjozwiak authored Oct 8, 2021
2 parents 7bd0d0e + 37af820 commit 592a365
Show file tree
Hide file tree
Showing 14 changed files with 219 additions and 13 deletions.
7 changes: 7 additions & 0 deletions browser/brave_prefs_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,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(
Expand Down
2 changes: 2 additions & 0 deletions browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,8 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {

registry->SetDefaultPrefValue(prefs::kEnableMediaRouter, base::Value(false));

registry->RegisterBooleanPref(kEnableMediaRouterOnRestart, false);

RegisterProfilePrefsForMigration(registry);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,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;
Expand Down
19 changes: 19 additions & 0 deletions browser/profiles/brave_profile_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
46 changes: 46 additions & 0 deletions browser/profiles/brave_profile_manager_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
102 changes: 102 additions & 0 deletions browser/profiles/brave_profile_manager_unittest.cc
Original file line number Diff line number Diff line change
@@ -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 <memory>

#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<BraveProfileManager> CreateProfileManagerForTest() {
return std::make_unique<BraveProfileManagerWithoutInit>(
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<TestingProfile> 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<TestingProfile> 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));
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ cr.define('settings', function () {
isWidevineEnabled() {}
getRestartNeeded () {}
wasSignInEnabledAtStartup () {}
isMediaRouterEnabled () {}
isDecentralizedDnsEnabled() {}
getDecentralizedDnsResolveMethodList(provider) {}
}
Expand Down Expand Up @@ -70,6 +71,10 @@ cr.define('settings', function () {
return loadTimeData.getBoolean('signInAllowedOnNextStartupInitialValue')
}

isMediaRouterEnabled () {
return loadTimeData.getBoolean('isMediaRouterEnabled')
}

isDecentralizedDnsEnabled () {
return cr.sendWithPromise('isDecentralizedDnsEnabled')
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,15 @@
</settings-toggle-button>
<settings-toggle-button id="mediaRouterEnabled"
class="cr-row"
pref="{{prefs.media_router.enable_media_router}}"
pref="{{prefs.brave.enable_media_router_on_restart}}"
label="Media Router"
sub-label="$i18n{mediaRouterEnabledDesc}">
<template is="dom-if" if="[[shouldShowRestartForMediaRouter_(
prefs.brave.enable_media_router_on_restart.value)]]">
<cr-button on-click="restartBrowser_" slot="more-actions">
$i18n{restart}
</cr-button>
</template>
</settings-toggle-button>
<template is="dom-if" if="{{ decentralizedDnsEnabled_ }}">
<div class="settings-box">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ Polymer({

shouldShowRestartForGoogleLogin_: function(value) {
return this.browserProxy_.wasSignInEnabledAtStartup() != value;
},

shouldShowRestartForMediaRouter_: function(value) {
return this.browserProxy_.isMediaRouterEnabled() != value;
}

});
Expand Down
10 changes: 7 additions & 3 deletions chromium_src/chrome/browser/media/router/media_router_feature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,16 @@
namespace media_router {

bool MediaRouterEnabled(content::BrowserContext* context) {
#if !defined(OS_ANDROID)
if (GetMediaRouterPref(context)->IsDefaultValue()) {
#if defined(OS_ANDROID)
return MediaRouterEnabled_ChromiumImpl(context);
#else
if (!base::FeatureList::IsEnabled(kMediaRouter)) {
return false;
}
const PrefService::Preference* pref = GetMediaRouterPref(context);
CHECK(pref->GetValue()->is_bool());
return pref->GetValue()->GetBool();
#endif
return MediaRouterEnabled_ChromiumImpl(context);
}

} // namespace media_router
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@
#include "base/stl_util.h"
#include "base/strings/utf_string_conversions.h"
#include "brave/browser/ui/webui/settings/brave_privacy_handler.h"
#include "brave/common/pref_names.h"
#include "brave/common/url_constants.h"
#include "brave/components/brave_vpn/buildflags/buildflags.h"
#include "brave/components/ipfs/ipfs_constants.h"
#include "brave/components/ipfs/pref_names.h"
#include "brave/components/sidebar/buildflags/buildflags.h"
#include "brave/components/version_info/version_info.h"
#include "chrome/browser/media/router/media_router_feature.h"
#include "chrome/browser/ui/webui/webui_util.h"
#include "chrome/common/pref_names.h"
#include "components/grit/brave_components_strings.h"
Expand Down Expand Up @@ -371,20 +373,20 @@ void BraveAddAboutStrings(content::WebUIDataSource* html_source,
html_source->AddString("aboutProductLicense", license);
}

void BraveAddSocialBlockingLoadTimeData(content::WebUIDataSource* html_source,
Profile* profile) {
html_source->AddBoolean(
"signInAllowedOnNextStartupInitialValue",
profile->GetPrefs()->GetBoolean(prefs::kSigninAllowedOnNextStartup));
}

void BraveAddLocalizedStrings(content::WebUIDataSource* html_source,
Profile* profile) {
BraveAddCommonStrings(html_source, profile);
BraveAddResources(html_source, profile);
BraveAddAboutStrings(html_source, profile);
BravePrivacyHandler::AddLoadTimeData(html_source, profile);
BraveAddSocialBlockingLoadTimeData(html_source, profile);

// Load time data for brave://settings/extensions
html_source->AddBoolean(
"signInAllowedOnNextStartupInitialValue",
profile->GetPrefs()->GetBoolean(prefs::kSigninAllowedOnNextStartup));

html_source->AddBoolean("isMediaRouterEnabled",
media_router::MediaRouterEnabled(profile));
}

} // namespace settings
2 changes: 2 additions & 0 deletions common/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ const char kImportDialogPayments[] = "import_dialog_payments";
const char kMRUCyclingEnabled[] = "brave.mru_cycling_enabled";
const char kTabsSearchShow[] = "brave.tabs_search_show";
const char kDontAskForCrashReporting[] = "brave.dont_ask_for_crash_reporting";
const char kEnableMediaRouterOnRestart[] =
"brave.enable_media_router_on_restart";

#if BUILDFLAG(ENABLE_BRAVE_VPN)
const char kBraveVPNShowButton[] = "brave.brave_vpn.show_button";
Expand Down
6 changes: 6 additions & 0 deletions common/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,10 @@ extern const char kDontAskForCrashReporting[];
extern const char kBraveVPNShowButton[];
#endif

// Cast extension requires a browser restart once the setting is toggled.
// kEnableMediaRouterOnRestart is used as a proxy to identify the current
// state of the switch and prefs::kEnableMediaRouter is updated to
// kEnableMediaRouterOnRestart on restart.
extern const char kEnableMediaRouterOnRestart[];

#endif // BRAVE_COMMON_PREF_NAMES_H_
1 change: 1 addition & 0 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ test("brave_unit_tests") {
sources += [
"../utility/importer/chrome_importer_unittest.cc",
"//brave/app/brave_command_line_helper_unittest.cc",
"//brave/browser/profiles/brave_profile_manager_unittest.cc",
"//brave/browser/resources/settings/brandcode_config_fetcher_unittest.cc",
"//brave/browser/resources/settings/reset_report_uploader_unittest.cc",
"//brave/browser/themes/brave_theme_service_unittest.cc",
Expand Down

0 comments on commit 592a365

Please sign in to comment.