Commit 006c6020 authored by mark a. foltz's avatar mark a. foltz Committed by Chromium LUCI CQ

[Media Router] Ensure MediaRouterEnabled() does not change per-profile.

A runtime change to the kMediaRouterEnabled policy (managed preference) results
in undefined behavior and crashes.  This is a workaround to ensure that
MediaRouterEnabled() returns the same value for the lifetime of a profile.

Fixing the Media Router to be enabled/disabled dynamically is possible, but will
be blocked on extension removal (specifically the MediaRouteProvider mojo
interface).

Tested by manually enabling/disabling the policy while running Chrome and
ensuring no crashes or partially-enabled UI states happen.

Bug: 1076831
Change-Id: I966a58196f92be40aed8771282c27f762102da6e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2597752
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: default avatarTakumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838705}
parent 0b41df56
......@@ -250,7 +250,12 @@ source_set("unittests") {
"providers/extension/extension_media_route_provider_proxy_unittest.cc",
"providers/wired_display/wired_display_media_route_provider_unittest.cc",
]
deps += [ ":test_support" ]
deps += [
":test_support",
"//chrome/test:test_support",
"//components/sync_preferences:test_support",
"//content/test:test_support",
]
}
if (enable_openscreen) {
......
......@@ -4,8 +4,12 @@
#include "chrome/browser/media/router/media_router_feature.h"
#include <utility>
#include "base/base64.h"
#include "base/containers/flat_map.h"
#include "base/feature_list.h"
#include "base/no_destructor.h"
#include "base/strings/string_util.h"
#include "build/build_config.h"
#include "chrome/browser/profiles/profile.h"
......@@ -66,11 +70,22 @@ bool MediaRouterEnabled(content::BrowserContext* context) {
#endif // !defined(OFFICIAL_BUILD) && !defined(OS_ANDROID)
#if defined(OS_ANDROID) || BUILDFLAG(ENABLE_EXTENSIONS)
static base::NoDestructor<base::flat_map<content::BrowserContext*, bool>>
stored_pref_values;
// If the Media Router was already enabled or disabled for |context|, then it
// must remain so. The Media Router does not support dynamic
// enabling/disabling.
auto const it = stored_pref_values->find(context);
if (it != stored_pref_values->end())
return it->second;
// Check the enterprise policy.
const PrefService::Preference* pref = GetMediaRouterPref(context);
// Only use the pref value if it set from a mandatory policy.
if (pref->IsManaged() && !pref->IsDefaultValue()) {
bool allowed = false;
bool allowed;
CHECK(pref->GetValue()->GetAsBoolean(&allowed));
stored_pref_values->insert(std::make_pair(context, allowed));
return allowed;
}
......
......@@ -4,11 +4,23 @@
#include "chrome/browser/media/router/media_router_feature.h"
#include <memory>
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h"
#include "extensions/buildflags/buildflags.h"
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_ANDROID) || BUILDFLAG(ENABLE_EXTENSIONS)
#include "base/values.h"
#include "chrome/common/pref_names.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"
#endif // defined(OS_ANDROID) || BUILDFLAG(ENABLE_EXTENSIONS)
namespace media_router {
TEST(MediaRouterFeatureTest, GetCastAllowAllIPsPref) {
......@@ -42,4 +54,41 @@ TEST(MediaRouterFeatureTest, GetReceiverIdHashToken) {
EXPECT_EQ(token, GetReceiverIdHashToken(pref_service.get()));
}
#if defined(OS_ANDROID) || BUILDFLAG(ENABLE_EXTENSIONS)
class MediaRouterEnabledTest : public ::testing::Test {
public:
MediaRouterEnabledTest() = default;
MediaRouterEnabledTest(const MediaRouterEnabledTest&) = delete;
~MediaRouterEnabledTest() override = default;
MediaRouterEnabledTest& operator=(const MediaRouterEnabledTest&) = delete;
protected:
content::BrowserTaskEnvironment test_environment;
TestingProfile enabled_profile;
TestingProfile disabled_profile;
};
TEST_F(MediaRouterEnabledTest, TestEnabledByPolicy) {
enabled_profile.GetTestingPrefService()->SetManagedPref(
::prefs::kEnableMediaRouter, std::make_unique<base::Value>(true));
EXPECT_TRUE(MediaRouterEnabled(&enabled_profile));
enabled_profile.GetTestingPrefService()->SetManagedPref(
::prefs::kEnableMediaRouter, std::make_unique<base::Value>(false));
// Runtime changes are not supported.
EXPECT_TRUE(MediaRouterEnabled(&enabled_profile));
}
TEST_F(MediaRouterEnabledTest, TestDisabledByPolicy) {
disabled_profile.GetTestingPrefService()->SetManagedPref(
::prefs::kEnableMediaRouter, std::make_unique<base::Value>(false));
EXPECT_FALSE(MediaRouterEnabled(&disabled_profile));
disabled_profile.GetTestingPrefService()->SetManagedPref(
::prefs::kEnableMediaRouter, std::make_unique<base::Value>(true));
// Runtime changes are not supported.
EXPECT_FALSE(MediaRouterEnabled(&disabled_profile));
}
#endif // defined(OS_ANDROID) || BUILDFLAG(ENABLE_EXTENSIONS)
} // namespace media_router
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment