Commit 74e43623 authored by Takumi Fujimoto's avatar Takumi Fujimoto Committed by Chromium LUCI CQ

Give NOTREACHED() when trying to instantiate Media Router while disabled

The clients of Media Router must now check that MediaRouterEnabled() is
true before using Media Router.

Bug: 1156871
Change-Id: I4af13b7159072272835008b89ad550c3d2272eaf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612268Reviewed-by: default avatarAndrey Kosyakov <caseq@chromium.org>
Reviewed-by: default avatarmark a. foltz <mfoltz@chromium.org>
Reviewed-by: default avatarTommy Steimel <steimel@chromium.org>
Reviewed-by: default avatarMitsuru Oshima <oshima@chromium.org>
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842804}
parent 0ed7f3fe
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <utility> #include <utility>
#include "base/bind.h" #include "base/bind.h"
#include "chrome/browser/media/router/media_router_feature.h"
#include "chrome/browser/ui/media_router/media_router_ui_helper.h" #include "chrome/browser/ui/media_router/media_router_ui_helper.h"
#include "components/media_router/browser/media_router.h" #include "components/media_router/browser/media_router.h"
#include "components/media_router/browser/media_router_factory.h" #include "components/media_router/browser/media_router_factory.h"
...@@ -23,9 +24,15 @@ using protocol::Cast::Sink; ...@@ -23,9 +24,15 @@ using protocol::Cast::Sink;
namespace { namespace {
constexpr char kMediaRouterErrorMessage[] =
"You must enable the Media Router feature in order to use Cast.";
media_router::MediaRouter* GetMediaRouter(content::WebContents* web_contents) { media_router::MediaRouter* GetMediaRouter(content::WebContents* web_contents) {
return media_router::MediaRouterFactory::GetApiForBrowserContext( if (media_router::MediaRouterEnabled(web_contents->GetBrowserContext())) {
web_contents->GetBrowserContext()); return media_router::MediaRouterFactory::GetApiForBrowserContext(
web_contents->GetBrowserContext());
}
return nullptr;
} }
} // namespace } // namespace
...@@ -83,7 +90,9 @@ CastHandler::CastHandler(content::WebContents* web_contents, ...@@ -83,7 +90,9 @@ CastHandler::CastHandler(content::WebContents* web_contents,
CastHandler::~CastHandler() = default; CastHandler::~CastHandler() = default;
Response CastHandler::SetSinkToUse(const std::string& in_sink_name) { Response CastHandler::SetSinkToUse(const std::string& in_sink_name) {
EnsureInitialized(); Response init_response = EnsureInitialized();
if (!init_response.IsSuccess())
return init_response;
media_router::PresentationServiceDelegateImpl::GetOrCreateForWebContents( media_router::PresentationServiceDelegateImpl::GetOrCreateForWebContents(
web_contents_) web_contents_)
->set_start_presentation_cb( ->set_start_presentation_cb(
...@@ -95,7 +104,9 @@ Response CastHandler::SetSinkToUse(const std::string& in_sink_name) { ...@@ -95,7 +104,9 @@ Response CastHandler::SetSinkToUse(const std::string& in_sink_name) {
void CastHandler::StartTabMirroring( void CastHandler::StartTabMirroring(
const std::string& in_sink_name, const std::string& in_sink_name,
std::unique_ptr<StartTabMirroringCallback> callback) { std::unique_ptr<StartTabMirroringCallback> callback) {
EnsureInitialized(); Response init_response = EnsureInitialized();
if (!init_response.IsSuccess())
callback->sendFailure(init_response);
const media_router::MediaSink::Id& sink_id = GetSinkIdByName(in_sink_name); const media_router::MediaSink::Id& sink_id = GetSinkIdByName(in_sink_name);
if (sink_id.empty()) { if (sink_id.empty()) {
callback->sendFailure(Response::ServerError("Sink not found")); callback->sendFailure(Response::ServerError("Sink not found"));
...@@ -116,7 +127,9 @@ void CastHandler::StartTabMirroring( ...@@ -116,7 +127,9 @@ void CastHandler::StartTabMirroring(
} }
Response CastHandler::StopCasting(const std::string& in_sink_name) { Response CastHandler::StopCasting(const std::string& in_sink_name) {
EnsureInitialized(); Response init_response = EnsureInitialized();
if (!init_response.IsSuccess())
return init_response;
const media_router::MediaSink::Id& sink_id = GetSinkIdByName(in_sink_name); const media_router::MediaSink::Id& sink_id = GetSinkIdByName(in_sink_name);
if (sink_id.empty()) if (sink_id.empty())
return Response::ServerError("Sink not found"); return Response::ServerError("Sink not found");
...@@ -129,7 +142,9 @@ Response CastHandler::StopCasting(const std::string& in_sink_name) { ...@@ -129,7 +142,9 @@ Response CastHandler::StopCasting(const std::string& in_sink_name) {
} }
Response CastHandler::Enable(protocol::Maybe<std::string> in_presentation_url) { Response CastHandler::Enable(protocol::Maybe<std::string> in_presentation_url) {
EnsureInitialized(); Response init_response = EnsureInitialized();
if (!init_response.IsSuccess())
return init_response;
StartObservingForSinks(std::move(in_presentation_url)); StartObservingForSinks(std::move(in_presentation_url));
return Response::Success(); return Response::Success();
} }
...@@ -152,9 +167,11 @@ void CastHandler::OnResultsUpdated( ...@@ -152,9 +167,11 @@ void CastHandler::OnResultsUpdated(
CastHandler::CastHandler(content::WebContents* web_contents) CastHandler::CastHandler(content::WebContents* web_contents)
: web_contents_(web_contents), router_(GetMediaRouter(web_contents)) {} : web_contents_(web_contents), router_(GetMediaRouter(web_contents)) {}
void CastHandler::EnsureInitialized() { Response CastHandler::EnsureInitialized() {
if (!router_)
return Response::ServerError(kMediaRouterErrorMessage);
if (query_result_manager_) if (query_result_manager_)
return; return Response::Success();
query_result_manager_ = query_result_manager_ =
std::make_unique<media_router::QueryResultManager>(router_); std::make_unique<media_router::QueryResultManager>(router_);
...@@ -165,6 +182,7 @@ void CastHandler::EnsureInitialized() { ...@@ -165,6 +182,7 @@ void CastHandler::EnsureInitialized() {
issues_observer_ = std::make_unique<IssuesObserver>( issues_observer_ = std::make_unique<IssuesObserver>(
router_, router_,
base::BindRepeating(&CastHandler::OnIssue, base::Unretained(this))); base::BindRepeating(&CastHandler::OnIssue, base::Unretained(this)));
return Response::Success();
} }
void CastHandler::StartPresentation( void CastHandler::StartPresentation(
......
...@@ -55,8 +55,9 @@ class CastHandler : public protocol::Cast::Backend, ...@@ -55,8 +55,9 @@ class CastHandler : public protocol::Cast::Backend,
// Constructor that does not wire the handler to a dispatcher. Used in tests. // Constructor that does not wire the handler to a dispatcher. Used in tests.
explicit CastHandler(content::WebContents* web_contents); explicit CastHandler(content::WebContents* web_contents);
// Initializes the handler if it hasn't been initialized yet. // Initializes the handler if it hasn't been initialized yet. Returns
void EnsureInitialized(); // Response::Success() if initialization succeeds, otherwise an error.
protocol::Response EnsureInitialized();
void StartPresentation( void StartPresentation(
const std::string& sink_name, const std::string& sink_name,
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "chrome/browser/media/router/chrome_media_router_factory.h" #include "chrome/browser/media/router/chrome_media_router_factory.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/media/router/media_router_feature.h"
#include "chrome/browser/profiles/incognito_helpers.h" #include "chrome/browser/profiles/incognito_helpers.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/media_router/browser/media_router_dialog_controller.h" #include "components/media_router/browser/media_router_dialog_controller.h"
...@@ -70,6 +71,10 @@ content::BrowserContext* ChromeMediaRouterFactory::GetBrowserContextToUse( ...@@ -70,6 +71,10 @@ content::BrowserContext* ChromeMediaRouterFactory::GetBrowserContextToUse(
KeyedService* ChromeMediaRouterFactory::BuildServiceInstanceFor( KeyedService* ChromeMediaRouterFactory::BuildServiceInstanceFor(
BrowserContext* context) const { BrowserContext* context) const {
if (!MediaRouterEnabled(context)) {
NOTREACHED();
return nullptr;
}
MediaRouterBase* media_router = nullptr; MediaRouterBase* media_router = nullptr;
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
media_router = new MediaRouterAndroid(); media_router = new MediaRouterAndroid();
......
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/callback_helpers.h" #include "base/callback_helpers.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chrome_notification_types.h"
...@@ -32,12 +31,7 @@ namespace { ...@@ -32,12 +31,7 @@ namespace {
base::Optional<media_router::MediaRouter*> media_router_for_test_; base::Optional<media_router::MediaRouter*> media_router_for_test_;
// Returns the MediaRouter instance for the current primary profile, if there is Profile* GetProfile() {
// one.
media_router::MediaRouter* GetMediaRouter() {
if (media_router_for_test_)
return *media_router_for_test_;
if (!user_manager::UserManager::IsInitialized()) if (!user_manager::UserManager::IsInitialized())
return nullptr; return nullptr;
...@@ -45,7 +39,16 @@ media_router::MediaRouter* GetMediaRouter() { ...@@ -45,7 +39,16 @@ media_router::MediaRouter* GetMediaRouter() {
if (!user) if (!user)
return nullptr; return nullptr;
Profile* profile = chromeos::ProfileHelper::Get()->GetProfileByUser(user); return chromeos::ProfileHelper::Get()->GetProfileByUser(user);
}
// Returns the MediaRouter instance for the current primary profile, if there is
// one.
media_router::MediaRouter* GetMediaRouter() {
if (media_router_for_test_)
return *media_router_for_test_;
Profile* profile = GetProfile();
if (!profile) if (!profile)
return nullptr; return nullptr;
...@@ -144,11 +147,6 @@ void CastDeviceCache::OnRoutesUpdated( ...@@ -144,11 +147,6 @@ void CastDeviceCache::OnRoutesUpdated(
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// CastConfigControllerMediaRouter: // CastConfigControllerMediaRouter:
void CastConfigControllerMediaRouter::SetMediaRouterForTest(
media_router::MediaRouter* media_router) {
media_router_for_test_ = media_router;
}
CastConfigControllerMediaRouter::CastConfigControllerMediaRouter() { CastConfigControllerMediaRouter::CastConfigControllerMediaRouter() {
// TODO(jdufault): This should use a callback interface once there is an // TODO(jdufault): This should use a callback interface once there is an
// equivalent. See crbug.com/666005. // equivalent. See crbug.com/666005.
...@@ -158,6 +156,20 @@ CastConfigControllerMediaRouter::CastConfigControllerMediaRouter() { ...@@ -158,6 +156,20 @@ CastConfigControllerMediaRouter::CastConfigControllerMediaRouter() {
CastConfigControllerMediaRouter::~CastConfigControllerMediaRouter() = default; CastConfigControllerMediaRouter::~CastConfigControllerMediaRouter() = default;
// static
bool CastConfigControllerMediaRouter::MediaRouterEnabled() {
if (media_router_for_test_)
return true;
Profile* profile = GetProfile();
return profile ? media_router::MediaRouterEnabled(profile) : false;
}
// static
void CastConfigControllerMediaRouter::SetMediaRouterForTest(
media_router::MediaRouter* media_router) {
media_router_for_test_ = media_router;
}
CastDeviceCache* CastConfigControllerMediaRouter::device_cache() { CastDeviceCache* CastConfigControllerMediaRouter::device_cache() {
// The CastDeviceCache instance is lazily allocated because the MediaRouter // The CastDeviceCache instance is lazily allocated because the MediaRouter
// component is not ready when the constructor is invoked. // component is not ready when the constructor is invoked.
...@@ -243,15 +255,18 @@ CastConfigControllerMediaRouter::GetSinksAndRoutes() { ...@@ -243,15 +255,18 @@ CastConfigControllerMediaRouter::GetSinksAndRoutes() {
} }
void CastConfigControllerMediaRouter::CastToSink(const std::string& sink_id) { void CastConfigControllerMediaRouter::CastToSink(const std::string& sink_id) {
// TODO(imcheng): Pass in tab casting timeout. if (GetMediaRouter()) {
GetMediaRouter()->CreateRoute( // TODO(imcheng): Pass in tab casting timeout.
media_router::MediaSource::ForUnchosenDesktop().id(), sink_id, GetMediaRouter()->CreateRoute(
url::Origin::Create(GURL("http://cros-cast-origin/")), nullptr, media_router::MediaSource::ForUnchosenDesktop().id(), sink_id,
base::DoNothing(), base::TimeDelta(), false); url::Origin::Create(GURL("http://cros-cast-origin/")), nullptr,
base::DoNothing(), base::TimeDelta(), false);
}
} }
void CastConfigControllerMediaRouter::StopCasting(const std::string& route_id) { void CastConfigControllerMediaRouter::StopCasting(const std::string& route_id) {
GetMediaRouter()->TerminateRoute(route_id); if (GetMediaRouter())
GetMediaRouter()->TerminateRoute(route_id);
} }
void CastConfigControllerMediaRouter::Observe( void CastConfigControllerMediaRouter::Observe(
......
...@@ -26,6 +26,7 @@ class CastConfigControllerMediaRouter : public ash::CastConfigController, ...@@ -26,6 +26,7 @@ class CastConfigControllerMediaRouter : public ash::CastConfigController,
CastConfigControllerMediaRouter(); CastConfigControllerMediaRouter();
~CastConfigControllerMediaRouter() override; ~CastConfigControllerMediaRouter() override;
static bool MediaRouterEnabled();
static void SetMediaRouterForTest(media_router::MediaRouter* media_router); static void SetMediaRouterForTest(media_router::MediaRouter* media_router);
private: private:
......
...@@ -120,8 +120,10 @@ void ChromeBrowserMainExtraPartsAsh::PreProfileInit() { ...@@ -120,8 +120,10 @@ void ChromeBrowserMainExtraPartsAsh::PreProfileInit() {
std::make_unique<NetworkConnectDelegateChromeOS>(); std::make_unique<NetworkConnectDelegateChromeOS>();
chromeos::NetworkConnect::Initialize(network_connect_delegate_.get()); chromeos::NetworkConnect::Initialize(network_connect_delegate_.get());
cast_config_controller_media_router_ = if (CastConfigControllerMediaRouter::MediaRouterEnabled()) {
std::make_unique<CastConfigControllerMediaRouter>(); cast_config_controller_media_router_ =
std::make_unique<CastConfigControllerMediaRouter>();
}
// Needed by AmbientController in ash. // Needed by AmbientController in ash.
if (chromeos::features::IsAmbientModeEnabled()) if (chromeos::features::IsAmbientModeEnabled())
......
...@@ -86,9 +86,11 @@ bool IsWebContentsFocused(content::WebContents* web_contents) { ...@@ -86,9 +86,11 @@ bool IsWebContentsFocused(content::WebContents* web_contents) {
base::WeakPtr<media_router::WebContentsPresentationManager> base::WeakPtr<media_router::WebContentsPresentationManager>
GetPresentationManager(content::WebContents* web_contents) { GetPresentationManager(content::WebContents* web_contents) {
return web_contents if (!web_contents ||
? media_router::WebContentsPresentationManager::Get(web_contents) !media_router::MediaRouterEnabled(web_contents->GetBrowserContext())) {
: nullptr; return nullptr;
}
return media_router::WebContentsPresentationManager::Get(web_contents);
} }
} // anonymous namespace } // anonymous namespace
...@@ -316,18 +318,19 @@ void MediaNotificationService::Session::MarkActiveIfNecessary() { ...@@ -316,18 +318,19 @@ void MediaNotificationService::Session::MarkActiveIfNecessary() {
MediaNotificationService::MediaNotificationService(Profile* profile, MediaNotificationService::MediaNotificationService(Profile* profile,
bool show_from_all_profiles) bool show_from_all_profiles)
: overlay_media_notifications_manager_(this) { : overlay_media_notifications_manager_(this) {
if (base::FeatureList::IsEnabled(media::kGlobalMediaControlsForCast) && if (media_router::MediaRouterEnabled(profile)) {
media_router::MediaRouterEnabled(profile)) { if (base::FeatureList::IsEnabled(media::kGlobalMediaControlsForCast)) {
cast_notification_provider_ = cast_notification_provider_ =
std::make_unique<CastMediaNotificationProvider>( std::make_unique<CastMediaNotificationProvider>(
profile, this, profile, this,
base::BindRepeating( base::BindRepeating(
&MediaNotificationService::OnCastNotificationsChanged, &MediaNotificationService::OnCastNotificationsChanged,
base::Unretained(this))); base::Unretained(this)));
} }
if (media_router::GlobalMediaControlsCastStartStopEnabled()) { if (media_router::GlobalMediaControlsCastStartStopEnabled()) {
presentation_request_notification_provider_ = presentation_request_notification_provider_ =
std::make_unique<PresentationRequestNotificationProvider>(this); std::make_unique<PresentationRequestNotificationProvider>(this);
}
} }
// Connect to the controller manager so we can create media controllers for // Connect to the controller manager so we can create media controllers for
......
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