diff options
16 files changed, 348 insertions, 63 deletions
diff --git a/chrome/browser/media/android/router/media_router_android.cc b/chrome/browser/media/android/router/media_router_android.cc index 8bf0b29..2d3b584 100644 --- a/chrome/browser/media/android/router/media_router_android.cc +++ b/chrome/browser/media/android/router/media_router_android.cc @@ -132,7 +132,8 @@ void MediaRouterAndroid::ConnectRouteByRouteId( const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) { + base::TimeDelta timeout, + bool off_the_record) { NOTIMPLEMENTED(); } @@ -192,6 +193,7 @@ void MediaRouterAndroid::TerminateRoute(const MediaRoute::Id& route_id) { base::android::ConvertUTF8ToJavaString(env, route_id); Java_ChromeMediaRouter_closeRoute( env, java_media_router_.obj(), jroute_id.obj()); + OnRouteTerminated(route_id); } void MediaRouterAndroid::SendRouteMessage( @@ -382,6 +384,8 @@ void MediaRouterAndroid::OnRouteCreated( jis_local, std::string(), true); // TODO(avayvod): Populate for_display. + // TODO(crbug.com/588239): Call OnOffTheRecordRouteCreated() if |route| + // is OTR scoped_ptr<RouteRequestResult> result = RouteRequestResult::FromSuccess( make_scoped_ptr(new MediaRoute(route)), request->presentation_id); for (const MediaRouteResponseCallback& callback : request->callbacks) diff --git a/chrome/browser/media/android/router/media_router_android.h b/chrome/browser/media/android/router/media_router_android.h index 2258fa6..21d5086 100644 --- a/chrome/browser/media/android/router/media_router_android.h +++ b/chrome/browser/media/android/router/media_router_android.h @@ -52,7 +52,8 @@ class MediaRouterAndroid : public MediaRouterBase { const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) override; + base::TimeDelta timeout, + bool off_the_record) override; void DetachRoute(const MediaRoute::Id& route_id) override; void TerminateRoute(const MediaRoute::Id& route_id) override; void SendRouteMessage(const MediaRoute::Id& route_id, diff --git a/chrome/browser/media/router/media_router.h b/chrome/browser/media/router/media_router.h index f8d6f28..d932d93 100644 --- a/chrome/browser/media/router/media_router.h +++ b/chrome/browser/media/router/media_router.h @@ -22,6 +22,8 @@ #include "content/public/browser/presentation_service_delegate.h" #include "content/public/browser/presentation_session_message.h" +class Profile; + namespace content { class WebContents; } @@ -95,13 +97,16 @@ class MediaRouter : public KeyedService { // success or failure, in the order they are listed. // If |timeout| is positive, then any un-invoked |callbacks| will be invoked // with a timeout error after the timeout expires. + // If |off_the_record| is true, the request was made by an off the record + // (incognito) profile. virtual void ConnectRouteByRouteId( const MediaSource::Id& source_id, const MediaRoute::Id& route_id, const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) = 0; + base::TimeDelta timeout, + bool off_the_record) = 0; // Joins an existing route identified by |presentation_id|. // |source|: The source to route to the existing route. @@ -158,6 +163,10 @@ class MediaRouter : public KeyedService { const MediaRoute::Id& route_id, const content::PresentationConnectionStateChangedCallback& callback) = 0; + // Called when the off the record (incognito) profile for this instance is + // being shut down. This will terminate all off the record media routes. + virtual void OnOffTheRecordProfileShutdown() = 0; + private: friend class IssuesObserver; friend class MediaSinksObserver; diff --git a/chrome/browser/media/router/media_router.mojom b/chrome/browser/media/router/media_router.mojom index 6c28dd7..bb9beab 100644 --- a/chrome/browser/media/router/media_router.mojom +++ b/chrome/browser/media/router/media_router.mojom @@ -125,18 +125,22 @@ enum RouteRequestResultCode { // Modeled after the MediaRouter interface defined in // chrome/browser/media/router/media_router.h interface MediaRouteProvider { - // Initiates a media route from |media_source| to |sink_id|. + // Creates a media route from |media_source| to the sink given by |sink_id|. + // // The presentation ID of the route created will be |presentation_id|, but it // may be overridden by a provider implementation. The presentation ID will // be used by the presentation API to refer to the created route. + // // |origin| and |tab_id| may be passed in for enforcing same-origin and/or - // same-tab scopes. - // Since int types cannot be made optional, use -1 as |tab_id| in cases where - // it is not applicable. + // same-tab scopes. Use -1 as |tab_id| in cases where the request is not + // made on behalf of a tab. + // // If |timeout_millis| is positive, it will be used in place of the default // timeout defined by Media Route Provider Manager. + // // If |off_the_record| is true, the request was made by an off the record // (incognito) profile. + // // If the operation was successful, |route| will be defined and // |error_text| will be null. // If the operation failed, |route| will be null and |error_text| @@ -154,14 +158,18 @@ interface MediaRouteProvider { string? error_text, RouteRequestResultCode result_code); - // Joins an established route given by |presentation_id| - // with |media_source|. - // |origin| and |tab_id| are used for validating same-origin/tab scopes. - // (See CreateRoute for additional documentation) + // Requests a connection to an established route for |media_source| given + // by |presentation_id|. + // + // |origin| and |tab_id| are used for validating same-origin/tab scopes; + // see CreateRoute for additional documentation. + // // If |timeout_millis| is positive, it will be used in place of the default + // timeout defined by Media Route Provider Manager. + // // If the route request was created by an off the record (incognito) profile, // |off_the_record| must be true. - // timeout defined by Media Route Provider Manager. + // // If the operation was successful, |route| will be defined and // |error_text| will be null. // If the operation failed, |route| will be null and |error_text| @@ -178,18 +186,27 @@ interface MediaRouteProvider { string? error_text, RouteRequestResultCode result_code); - // Connects an established route given by |route_id| with - // |media_source|. The presentation ID of the route created will be - // |presentation_id|, but it may be overridden by a provider implementation. - // The presentation ID will be used by the presentation API to refer to the - // created route. |origin| and |tab_id| are used for validating - // same-origin/tab scopes. + // Creates a new route for |media_source| that connects to the established + // route given by |route_id|. + // + // The presentation ID of the new route will be |presentation_id|, but it may + // be overridden by a provider implementation. The presentation ID will be + // used by the presentation API to refer to the created route. + // + // |origin| and |tab_id| are used for validating same-origin/tab scopes; see + // CreateRoute for additional documentation. + // // If |timeout_millis| is positive, it will be used in place of the default - // timeout defined by Media Route Provider Manager. - // (See CreateRoute for additional documentation) + // timeout defined by Media Route Provider Manager; see CreateRoute for additional + // documentation. + // + // If the route request was created by an off the record (incognito) profile, + // |off_the_record| must be true. + // // If the operation was successful, |route| will be defined and // |error_text| will be null. If the operation failed, |route| will be null // and |error_text| will be set. + // // |result| will be set to OK if successful, or an error code if an error // occurred. ConnectRouteByRouteId(string media_source, @@ -197,7 +214,8 @@ interface MediaRouteProvider { string presentation_id, string origin, int32 tab_id, - int64 timeout_millis) => + int64 timeout_millis, + bool off_the_record) => (MediaRoute? route, string? error_text, RouteRequestResultCode result_code); diff --git a/chrome/browser/media/router/media_router_base.cc b/chrome/browser/media/router/media_router_base.cc index b95f5a4c..4387272 100644 --- a/chrome/browser/media/router/media_router_base.cc +++ b/chrome/browser/media/router/media_router_base.cc @@ -5,11 +5,9 @@ #include "chrome/browser/media/router/media_router_base.h" #include "base/bind.h" - -// TODO(mfoltz): Enforce/verify incognito policies: -// - CreateRoute/JoinRoute with OTR = true/false returns a route with -// OTR = true/false -// - Destroying an incognito profile terminates all OTR routes +#include "base/stl_util.h" +#include "chrome/browser/chrome_notification_types.h" +#include "chrome/browser/profiles/profile.h" namespace media_router { @@ -36,9 +34,27 @@ MediaRouterBase::AddPresentationConnectionStateChangedCallback( return callbacks->Add(callback); } +void MediaRouterBase::OnOffTheRecordProfileShutdown() { + // TODO(mfoltz): There is a race condition where off-the-record routes created + // by pending CreateRoute requests won't be terminated. Fixing this would + // extra bookeeping of route requests in progress, and a way to cancel them + // in-flight. + for (auto route_ids_it = off_the_record_route_ids_.begin(); + route_ids_it != off_the_record_route_ids_.end(); + /* no-op */) { + // TerminateRoute will erase |route_id| from |off_the_record_route_ids_|, + // make a copy as the iterator will be invalidated. + const MediaRoute::Id route_id = *route_ids_it++; + TerminateRoute(route_id); + } +} + void MediaRouterBase::NotifyPresentationConnectionStateChange( const MediaRoute::Id& route_id, content::PresentationConnectionState state) { + if (state == content::PRESENTATION_CONNECTION_STATE_TERMINATED) + OnRouteTerminated(route_id); + auto* callbacks = presentation_connection_state_callbacks_.get(route_id); if (!callbacks) return; @@ -61,6 +77,17 @@ void MediaRouterBase::NotifyPresentationConnectionClose( callbacks->Notify(info); } +void MediaRouterBase::OnOffTheRecordRouteCreated( + const MediaRoute::Id& route_id) { + DCHECK(!ContainsKey(off_the_record_route_ids_, route_id)); + off_the_record_route_ids_.insert(route_id); +} + +void MediaRouterBase::OnRouteTerminated(const MediaRoute::Id& route_id) { + // NOTE: This is called for all routes (off the record or not). + off_the_record_route_ids_.erase(route_id); +} + void MediaRouterBase::OnPresentationConnectionStateCallbackRemoved( const MediaRoute::Id& route_id) { auto* callbacks = presentation_connection_state_callbacks_.get(route_id); diff --git a/chrome/browser/media/router/media_router_base.h b/chrome/browser/media/router/media_router_base.h index d342d9b..41a7c43 100644 --- a/chrome/browser/media/router/media_router_base.h +++ b/chrome/browser/media/router/media_router_base.h @@ -5,6 +5,8 @@ #ifndef CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTER_BASE_H_ #define CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTER_BASE_H_ +#include <set> + #include "base/callback_list.h" #include "base/containers/scoped_ptr_hash_map.h" #include "base/gtest_prod_util.h" @@ -12,6 +14,8 @@ #include "base/threading/thread_checker.h" #include "chrome/browser/media/router/media_router.h" +class Profile; + namespace media_router { class MediaRouterBase : public MediaRouter { @@ -25,6 +29,10 @@ class MediaRouterBase : public MediaRouter { const content::PresentationConnectionStateChangedCallback& callback) override; + // Called when the off the record (incognito) profile for this instance is + // being shut down. This will terminate all off the record media routes. + void OnOffTheRecordProfileShutdown() override; + protected: FRIEND_TEST_ALL_PREFIXES(MediaRouterMojoImplTest, PresentationConnectionStateChangedCallback); @@ -39,6 +47,11 @@ class MediaRouterBase : public MediaRouter { content::PresentationConnectionCloseReason reason, const std::string& message); + // Called when off the record route |route_id| has been created. + void OnOffTheRecordRouteCreated(const MediaRoute::Id& route_id); + // Called when route |route_id| has been terminated. + void OnRouteTerminated(const MediaRoute::Id& route_id); + using PresentationConnectionStateChangedCallbacks = base::CallbackList<void( const content::PresentationConnectionStateChangeInfo&)>; base::ScopedPtrHashMap< @@ -54,6 +67,9 @@ class MediaRouterBase : public MediaRouter { void OnPresentationConnectionStateCallbackRemoved( const MediaRoute::Id& route_id); + // Ids of current off the record media routes. + std::set<MediaRoute::Id> off_the_record_route_ids_; + DISALLOW_COPY_AND_ASSIGN(MediaRouterBase); }; diff --git a/chrome/browser/media/router/media_router_factory.cc b/chrome/browser/media/router/media_router_factory.cc index 8cc918d..58d051f 100644 --- a/chrome/browser/media/router/media_router_factory.cc +++ b/chrome/browser/media/router/media_router_factory.cc @@ -6,6 +6,7 @@ #include "build/build_config.h" #include "chrome/browser/profiles/incognito_helpers.h" +#include "chrome/browser/profiles/profile.h" #include "components/keyed_service/content/browser_context_dependency_manager.h" #if defined(OS_ANDROID) @@ -37,6 +38,17 @@ MediaRouter* MediaRouterFactory::GetApiForBrowserContext( service_factory.Get().GetServiceForBrowserContext(context, true)); } +void MediaRouterFactory::BrowserContextShutdown( + content::BrowserContext* context) { + if (context->IsOffTheRecord()) { + MediaRouter* router = + static_cast<MediaRouter*>(GetServiceForBrowserContext(context, false)); + if (router) + router->OnOffTheRecordProfileShutdown(); + } + BrowserContextKeyedServiceFactory::BrowserContextShutdown(context); +} + MediaRouterFactory::MediaRouterFactory() : BrowserContextKeyedServiceFactory( "MediaRouter", diff --git a/chrome/browser/media/router/media_router_factory.h b/chrome/browser/media/router/media_router_factory.h index 627eb87..4bfac56 100644 --- a/chrome/browser/media/router/media_router_factory.h +++ b/chrome/browser/media/router/media_router_factory.h @@ -23,6 +23,11 @@ class MediaRouterFactory : public BrowserContextKeyedServiceFactory { public: static MediaRouter* GetApiForBrowserContext(content::BrowserContext* context); + protected: + // We override the shutdown method for the factory to give the Media Router a + // chance to remove off the record media routes. + void BrowserContextShutdown(content::BrowserContext* context) override; + private: friend struct base::DefaultLazyInstanceTraits<MediaRouterFactory>; diff --git a/chrome/browser/media/router/media_router_mojo_impl.cc b/chrome/browser/media/router/media_router_mojo_impl.cc index fe72fef..37ea11c 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.cc +++ b/chrome/browser/media/router/media_router_mojo_impl.cc @@ -238,6 +238,7 @@ void MediaRouterMojoImpl::OnRoutesUpdated( void MediaRouterMojoImpl::RouteResponseReceived( const std::string& presentation_id, + bool off_the_record, const std::vector<MediaRouteResponseCallback>& callbacks, interfaces::MediaRoutePtr media_route, const mojo::String& error_text, @@ -248,12 +249,19 @@ void MediaRouterMojoImpl::RouteResponseReceived( DCHECK(!error_text.is_null()); std::string error = !error_text.get().empty() ? error_text.get() : "Unknown error."; - result = RouteRequestResult::FromError( error, mojo::RouteRequestResultCodeFromMojo(result_code)); + } else if (media_route->off_the_record != off_the_record) { + std::string error = base::StringPrintf( + "Mismatch in off the record status: request = %d, response = %d", + off_the_record, media_route->off_the_record); + result = RouteRequestResult::FromError( + error, RouteRequestResult::OFF_THE_RECORD_MISMATCH); } else { result = RouteRequestResult::FromSuccess( media_route.To<scoped_ptr<MediaRoute>>(), presentation_id); + if (result->route()->off_the_record()) + OnOffTheRecordRouteCreated(result->route()->media_route_id()); } // TODO(imcheng): Add UMA histogram based on result code (crbug.com/583044). @@ -321,7 +329,8 @@ void MediaRouterMojoImpl::ConnectRouteByRouteId( const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) { + base::TimeDelta timeout, + bool off_the_record) { DCHECK(thread_checker_.CalledOnValidThread()); if (!origin.is_valid()) { @@ -338,12 +347,12 @@ void MediaRouterMojoImpl::ConnectRouteByRouteId( RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoConnectRouteByRouteId, base::Unretained(this), source_id, route_id, origin.is_empty() ? "" : origin.spec(), tab_id, - callbacks, timeout)); + callbacks, timeout, off_the_record)); } void MediaRouterMojoImpl::TerminateRoute(const MediaRoute::Id& route_id) { DCHECK(thread_checker_.CalledOnValidThread()); - + DVLOG(2) << "TerminateRoute " << route_id; SetWakeReason(MediaRouteProviderWakeReason::TERMINATE_ROUTE); RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoTerminateRoute, base::Unretained(this), route_id)); @@ -563,9 +572,9 @@ void MediaRouterMojoImpl::DoCreateRoute( media_route_provider_->CreateRoute( source_id, sink_id, presentation_id, origin, tab_id, timeout > base::TimeDelta() ? timeout.InMilliseconds() : 0, - off_the_record, - base::Bind(&MediaRouterMojoImpl::RouteResponseReceived, - base::Unretained(this), presentation_id, callbacks)); + off_the_record, base::Bind(&MediaRouterMojoImpl::RouteResponseReceived, + base::Unretained(this), presentation_id, + off_the_record, callbacks)); } void MediaRouterMojoImpl::DoJoinRoute( @@ -582,9 +591,9 @@ void MediaRouterMojoImpl::DoJoinRoute( media_route_provider_->JoinRoute( source_id, presentation_id, origin, tab_id, timeout > base::TimeDelta() ? timeout.InMilliseconds() : 0, - off_the_record, - base::Bind(&MediaRouterMojoImpl::RouteResponseReceived, - base::Unretained(this), presentation_id, callbacks)); + off_the_record, base::Bind(&MediaRouterMojoImpl::RouteResponseReceived, + base::Unretained(this), presentation_id, + off_the_record, callbacks)); } void MediaRouterMojoImpl::DoConnectRouteByRouteId( @@ -593,7 +602,8 @@ void MediaRouterMojoImpl::DoConnectRouteByRouteId( const std::string& origin, int tab_id, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) { + base::TimeDelta timeout, + bool off_the_record) { std::string presentation_id("mr_"); presentation_id += base::GenerateGUID(); DVLOG_WITH_INSTANCE(1) << "DoConnectRouteByRouteId " << source_id @@ -603,13 +613,15 @@ void MediaRouterMojoImpl::DoConnectRouteByRouteId( media_route_provider_->ConnectRouteByRouteId( source_id, route_id, presentation_id, origin, tab_id, timeout > base::TimeDelta() ? timeout.InMilliseconds() : 0, - base::Bind(&MediaRouterMojoImpl::RouteResponseReceived, - base::Unretained(this), presentation_id, callbacks)); + off_the_record, base::Bind(&MediaRouterMojoImpl::RouteResponseReceived, + base::Unretained(this), presentation_id, + off_the_record, callbacks)); } void MediaRouterMojoImpl::DoTerminateRoute(const MediaRoute::Id& route_id) { DVLOG_WITH_INSTANCE(1) << "DoTerminateRoute " << route_id; media_route_provider_->TerminateRoute(route_id); + OnRouteTerminated(route_id); } void MediaRouterMojoImpl::DoDetachRoute(const MediaRoute::Id& route_id) { diff --git a/chrome/browser/media/router/media_router_mojo_impl.h b/chrome/browser/media/router/media_router_mojo_impl.h index 0cc2eaf..ff088aa 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.h +++ b/chrome/browser/media/router/media_router_mojo_impl.h @@ -84,7 +84,8 @@ class MediaRouterMojoImpl : public MediaRouterBase, const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout) override; + base::TimeDelta timeout, + bool off_the_record) override; void TerminateRoute(const MediaRoute::Id& route_id) override; void DetachRoute(const MediaRoute::Id& route_id) override; void SendRouteMessage(const MediaRoute::Id& route_id, @@ -233,7 +234,8 @@ class MediaRouterMojoImpl : public MediaRouterBase, const std::string& origin, int tab_id, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout); + base::TimeDelta timeout, + bool off_the_record); void DoTerminateRoute(const MediaRoute::Id& route_id); void DoDetachRoute(const MediaRoute::Id& route_id); void DoSendSessionMessage(const MediaRoute::Id& route_id, @@ -287,6 +289,7 @@ class MediaRouterMojoImpl : public MediaRouterBase, // into a local callback. void RouteResponseReceived( const std::string& presentation_id, + bool off_the_record, const std::vector<MediaRouteResponseCallback>& callbacks, interfaces::MediaRoutePtr media_route, const mojo::String& error_text, diff --git a/chrome/browser/media/router/media_router_mojo_impl_unittest.cc b/chrome/browser/media/router/media_router_mojo_impl_unittest.cc index 8bb8ac9..0cd285a 100644 --- a/chrome/browser/media/router/media_router_mojo_impl_unittest.cc +++ b/chrome/browser/media/router/media_router_mojo_impl_unittest.cc @@ -256,6 +256,82 @@ TEST_F(MediaRouterMojoImplTest, CreateRouteFails) { run_loop.Run(); } +TEST_F(MediaRouterMojoImplTest, CreateRouteOffTheRecordMismatchFails) { + EXPECT_CALL(mock_media_route_provider_, + CreateRoute(mojo::String(kSource), mojo::String(kSinkId), _, + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, + true, _)) + .WillOnce(Invoke( + [](const mojo::String& source, const mojo::String& sink, + const mojo::String& presentation_id, const mojo::String& origin, + int tab_id, int64_t timeout_millis, bool off_the_record, + const interfaces::MediaRouteProvider::CreateRouteCallback& cb) { + interfaces::MediaRoutePtr route = interfaces::MediaRoute::New(); + route->media_source = kSource; + route->media_sink_id = kSinkId; + route->media_route_id = kRouteId; + route->description = kDescription; + route->is_local = true; + route->for_display = true; + route->off_the_record = false; + cb.Run(std::move(route), mojo::String(), + interfaces::RouteRequestResultCode::OK); + })); + + RouteResponseCallbackHandler handler; + base::RunLoop run_loop; + std::string error( + "Mismatch in off the record status: request = 1, response = 0"); + EXPECT_CALL(handler, DoInvoke(nullptr, "", error, + RouteRequestResult::OFF_THE_RECORD_MISMATCH)) + .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); + std::vector<MediaRouteResponseCallback> route_response_callbacks; + route_response_callbacks.push_back(base::Bind( + &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); + router()->CreateRoute( + kSource, kSinkId, GURL(kOrigin), nullptr, route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis), true); + run_loop.Run(); +} + +TEST_F(MediaRouterMojoImplTest, OffTheRecordRoutesTerminatedOnProfileShutdown) { + EXPECT_CALL(mock_media_route_provider_, + CreateRoute(mojo::String(kSource), mojo::String(kSinkId), _, + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, + true, _)) + .WillOnce(Invoke( + [](const mojo::String& source, const mojo::String& sink, + const mojo::String& presentation_id, const mojo::String& origin, + int tab_id, int64_t timeout_millis, bool off_the_record, + const interfaces::MediaRouteProvider::CreateRouteCallback& cb) { + interfaces::MediaRoutePtr route = interfaces::MediaRoute::New(); + route->media_source = kSource; + route->media_sink_id = kSinkId; + route->media_route_id = kRouteId; + route->description = kDescription; + route->is_local = true; + route->for_display = true; + route->off_the_record = true; + cb.Run(std::move(route), mojo::String(), + interfaces::RouteRequestResultCode::OK); + })); + base::RunLoop run_loop; + router()->CreateRoute(kSource, kSinkId, GURL(kOrigin), nullptr, + std::vector<MediaRouteResponseCallback>(), + base::TimeDelta::FromMilliseconds(kTimeoutMillis), + true); + + // TODO(mfoltz): Where possible, convert other tests to use RunUntilIdle + // instead of manually calling Run/Quit on the run loop. + run_loop.RunUntilIdle(); + + EXPECT_CALL(mock_media_route_provider_, + TerminateRoute(mojo::String(kRouteId))); + base::RunLoop run_loop2; + router()->OnOffTheRecordProfileShutdown(); + run_loop2.RunUntilIdle(); +} + TEST_F(MediaRouterMojoImplTest, JoinRoute) { MediaSource media_source(kSource); MediaRoute expected_route(kRouteId, media_source, kSinkId, "", false, "", @@ -327,11 +403,53 @@ TEST_F(MediaRouterMojoImplTest, JoinRouteFails) { run_loop.Run(); } +TEST_F(MediaRouterMojoImplTest, JoinRouteOffTheRecordMismatchFails) { + interfaces::MediaRoutePtr route = interfaces::MediaRoute::New(); + route->media_source = kSource; + route->media_sink_id = kSinkId; + route->media_route_id = kRouteId; + route->description = kDescription; + route->is_local = true; + route->for_display = true; + route->off_the_record = false; + + // Use a lambda function as an invocation target here to work around + // a limitation with GMock::Invoke that prevents it from using move-only types + // in runnable parameter lists. + EXPECT_CALL( + mock_media_route_provider_, + JoinRoute(mojo::String(kSource), mojo::String(kPresentationId), + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, true, _)) + .WillOnce(Invoke([&route]( + const mojo::String& source, const mojo::String& presentation_id, + const mojo::String& origin, int tab_id, int64_t timeout_millis, + bool off_the_record, + const interfaces::MediaRouteProvider::JoinRouteCallback& cb) { + cb.Run(std::move(route), mojo::String(), + interfaces::RouteRequestResultCode::OK); + })); + + RouteResponseCallbackHandler handler; + base::RunLoop run_loop; + std::string error( + "Mismatch in off the record status: request = 1, response = 0"); + EXPECT_CALL(handler, DoInvoke(nullptr, "", error, + RouteRequestResult::OFF_THE_RECORD_MISMATCH)) + .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); + std::vector<MediaRouteResponseCallback> route_response_callbacks; + route_response_callbacks.push_back(base::Bind( + &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); + router()->JoinRoute(kSource, kPresentationId, GURL(kOrigin), nullptr, + route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis), true); + run_loop.Run(); +} + TEST_F(MediaRouterMojoImplTest, ConnectRouteByRouteId) { MediaSource media_source(kSource); MediaRoute expected_route(kRouteId, media_source, kSinkId, "", false, "", false); - expected_route.set_off_the_record(true); + expected_route.set_off_the_record(false); interfaces::MediaRoutePtr route = interfaces::MediaRoute::New(); route->media_source = kSource; route->media_sink_id = kSinkId; @@ -339,19 +457,20 @@ TEST_F(MediaRouterMojoImplTest, ConnectRouteByRouteId) { route->description = kDescription; route->is_local = true; route->for_display = true; - route->off_the_record = true; + route->off_the_record = false; // Use a lambda function as an invocation target here to work around // a limitation with GMock::Invoke that prevents it from using move-only types // in runnable parameter lists. - EXPECT_CALL(mock_media_route_provider_, - ConnectRouteByRouteId( - mojo::String(kSource), mojo::String(kRouteId), _, - mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _)) + EXPECT_CALL( + mock_media_route_provider_, + ConnectRouteByRouteId(mojo::String(kSource), mojo::String(kRouteId), _, + mojo::String(kOrigin), kInvalidTabId, + kTimeoutMillis, false, _)) .WillOnce(Invoke([&route]( const mojo::String& source, const mojo::String& route_id, const mojo::String& presentation_id, const mojo::String& origin, - int tab_id, int64_t timeout_millis, + int tab_id, int64_t timeout_millis, bool off_the_record, const interfaces::MediaRouteProvider::JoinRouteCallback& cb) { cb.Run(std::move(route), mojo::String(), interfaces::RouteRequestResultCode::OK); @@ -367,19 +486,20 @@ TEST_F(MediaRouterMojoImplTest, ConnectRouteByRouteId) { &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); router()->ConnectRouteByRouteId( kSource, kRouteId, GURL(kOrigin), nullptr, route_response_callbacks, - base::TimeDelta::FromMilliseconds(kTimeoutMillis)); + base::TimeDelta::FromMilliseconds(kTimeoutMillis), false); run_loop.Run(); } TEST_F(MediaRouterMojoImplTest, ConnectRouteByRouteIdFails) { - EXPECT_CALL(mock_media_route_provider_, - ConnectRouteByRouteId( - mojo::String(kSource), mojo::String(kRouteId), _, - mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _)) + EXPECT_CALL( + mock_media_route_provider_, + ConnectRouteByRouteId(mojo::String(kSource), mojo::String(kRouteId), _, + mojo::String(kOrigin), kInvalidTabId, + kTimeoutMillis, true, _)) .WillOnce(Invoke( [](const mojo::String& source, const mojo::String& route_id, const mojo::String& presentation_id, const mojo::String& origin, - int tab_id, int64_t timeout_millis, + int tab_id, int64_t timeout_millis, bool off_the_record, const interfaces::MediaRouteProvider::JoinRouteCallback& cb) { cb.Run(interfaces::MediaRoutePtr(), mojo::String(kError), interfaces::RouteRequestResultCode::TIMED_OUT); @@ -395,7 +515,50 @@ TEST_F(MediaRouterMojoImplTest, ConnectRouteByRouteIdFails) { &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); router()->ConnectRouteByRouteId( kSource, kRouteId, GURL(kOrigin), nullptr, route_response_callbacks, - base::TimeDelta::FromMilliseconds(kTimeoutMillis)); + base::TimeDelta::FromMilliseconds(kTimeoutMillis), true); + run_loop.Run(); +} + +TEST_F(MediaRouterMojoImplTest, ConnectRouteByIdOffTheRecordMismatchFails) { + interfaces::MediaRoutePtr route = interfaces::MediaRoute::New(); + route->media_source = kSource; + route->media_sink_id = kSinkId; + route->media_route_id = kRouteId; + route->description = kDescription; + route->is_local = true; + route->for_display = true; + route->off_the_record = false; + + // Use a lambda function as an invocation target here to work around + // a limitation with GMock::Invoke that prevents it from using move-only types + // in runnable parameter lists. + EXPECT_CALL( + mock_media_route_provider_, + ConnectRouteByRouteId(mojo::String(kSource), mojo::String(kRouteId), _, + mojo::String(kOrigin), kInvalidTabId, + kTimeoutMillis, true, _)) + .WillOnce(Invoke([&route]( + const mojo::String& source, const mojo::String& route_id, + const mojo::String& presentation_id, const mojo::String& origin, + int tab_id, int64_t timeout_millis, bool off_the_record, + const interfaces::MediaRouteProvider::JoinRouteCallback& cb) { + cb.Run(std::move(route), mojo::String(), + interfaces::RouteRequestResultCode::OK); + })); + + RouteResponseCallbackHandler handler; + base::RunLoop run_loop; + std::string error( + "Mismatch in off the record status: request = 1, response = 0"); + EXPECT_CALL(handler, DoInvoke(nullptr, "", error, + RouteRequestResult::OFF_THE_RECORD_MISMATCH)) + .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); + std::vector<MediaRouteResponseCallback> route_response_callbacks; + route_response_callbacks.push_back(base::Bind( + &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); + router()->ConnectRouteByRouteId( + kSource, kRouteId, GURL(kOrigin), nullptr, route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis), true); run_loop.Run(); } diff --git a/chrome/browser/media/router/mock_media_router.h b/chrome/browser/media/router/mock_media_router.h index 193cba3..4ab2725 100644 --- a/chrome/browser/media/router/mock_media_router.h +++ b/chrome/browser/media/router/mock_media_router.h @@ -41,13 +41,14 @@ class MockMediaRouter : public MediaRouter { const std::vector<MediaRouteResponseCallback>& callbacks, base::TimeDelta timeout, bool off_the_record)); - MOCK_METHOD6(ConnectRouteByRouteId, + MOCK_METHOD7(ConnectRouteByRouteId, void(const MediaSource::Id& source, const MediaRoute::Id& route_id, const GURL& origin, content::WebContents* web_contents, const std::vector<MediaRouteResponseCallback>& callbacks, - base::TimeDelta timeout)); + base::TimeDelta timeout, + bool off_the_record)); MOCK_METHOD1(DetachRoute, void(const MediaRoute::Id& route_id)); MOCK_METHOD1(TerminateRoute, void(const MediaRoute::Id& route_id)); MOCK_METHOD3(SendRouteMessage, @@ -76,6 +77,8 @@ class MockMediaRouter : public MediaRouter { OnAddPresentationConnectionStateChangedCallbackInvoked(callback); return connection_state_callbacks_.Add(callback); } + + MOCK_METHOD0(OnOffTheRecordProfileShutdown, void()); MOCK_METHOD1(OnAddPresentationConnectionStateChangedCallbackInvoked, void(const content::PresentationConnectionStateChangedCallback& callback)); diff --git a/chrome/browser/media/router/route_request_result.h b/chrome/browser/media/router/route_request_result.h index 6fbfa60..475f240 100644 --- a/chrome/browser/media/router/route_request_result.h +++ b/chrome/browser/media/router/route_request_result.h @@ -30,7 +30,13 @@ class MediaRoute; // |result_code|: A value from RouteRequestResult describing the error. class RouteRequestResult { public: - enum ResultCode { UNKNOWN_ERROR, OK, TIMED_OUT, INVALID_ORIGIN }; + enum ResultCode { + UNKNOWN_ERROR, + OK, + TIMED_OUT, + INVALID_ORIGIN, + OFF_THE_RECORD_MISMATCH + }; static scoped_ptr<RouteRequestResult> FromSuccess( scoped_ptr<MediaRoute> route, diff --git a/chrome/browser/media/router/test_helper.h b/chrome/browser/media/router/test_helper.h index 40d1103..057eb39 100644 --- a/chrome/browser/media/router/test_helper.h +++ b/chrome/browser/media/router/test_helper.h @@ -114,13 +114,14 @@ class MockMediaRouteProvider : public interfaces::MediaRouteProvider { int64_t timeout_secs, bool off_the_record, const JoinRouteCallback& callback)); - MOCK_METHOD7(ConnectRouteByRouteId, + MOCK_METHOD8(ConnectRouteByRouteId, void(const mojo::String& source_urn, const mojo::String& route_id, const mojo::String& presentation_id, const mojo::String& origin, int tab_id, int64_t timeout_secs, + bool off_the_record, const JoinRouteCallback& callback)); MOCK_METHOD1(DetachRoute, void(const mojo::String& route_id)); MOCK_METHOD1(TerminateRoute, void(const mojo::String& route_id)); diff --git a/chrome/browser/ui/webui/media_router/media_router_ui.cc b/chrome/browser/ui/webui/media_router/media_router_ui.cc index c15f419..7bf21a1 100644 --- a/chrome/browser/ui/webui/media_router/media_router_ui.cc +++ b/chrome/browser/ui/webui/media_router/media_router_ui.cc @@ -409,13 +409,15 @@ bool MediaRouterUI::CreateOrConnectRoute(const MediaSink::Id& sink_id, } base::TimeDelta timeout = GetRouteRequestTimeout(cast_mode); + bool off_the_record = Profile::FromWebUI(web_ui())->IsOffTheRecord(); if (route_id.empty()) { router_->CreateRoute(source.id(), sink_id, origin, initiator_, route_response_callbacks, timeout, - Profile::FromWebUI(web_ui())->IsOffTheRecord()); + off_the_record); } else { router_->ConnectRouteByRouteId(source.id(), route_id, origin, initiator_, - route_response_callbacks, timeout); + route_response_callbacks, timeout, + off_the_record); } return true; } diff --git a/extensions/renderer/resources/media_router_bindings.js b/extensions/renderer/resources/media_router_bindings.js index cfb36f1..e7f32d8 100644 --- a/extensions/renderer/resources/media_router_bindings.js +++ b/extensions/renderer/resources/media_router_bindings.js @@ -624,15 +624,18 @@ define('media_router_bindings', [ * @param {!number} timeoutMillis If positive, the timeout duration for the * request, measured in seconds. Otherwise, the default duration will be * used. + * @param {!boolean} offTheRecord If true, the route is being requested by + * an off the record (incognito) profile. * @return {!Promise.<!Object>} A Promise resolving to an object describing * the newly created media route, or rejecting with an error message on * failure. */ MediaRouteProvider.prototype.connectRouteByRouteId = function(sourceUrn, routeId, presentationId, origin, tabId, - timeoutMillis) { + timeoutMillis, offTheRecord) { return this.handlers_.connectRouteByRouteId( - sourceUrn, routeId, presentationId, origin, tabId, timeoutMillis) + sourceUrn, routeId, presentationId, origin, tabId, timeoutMillis, + offTheRecord) .then(function(route) { return toSuccessRouteResponse_(route); }, |