diff --git a/chrome/browser/media/android/router/media_router_android.cc b/chrome/browser/media/android/router/media_router_android.cc index 8a8385997fba18..bca53b1949fa13 100644 --- a/chrome/browser/media/android/router/media_router_android.cc +++ b/chrome/browser/media/android/router/media_router_android.cc @@ -18,6 +18,7 @@ #include "chrome/browser/media/router/media_routes_observer.h" #include "chrome/browser/media/router/media_sinks_observer.h" #include "chrome/browser/media/router/presentation_session_messages_observer.h" +#include "chrome/browser/media/router/route_request_result.h" #include "jni/ChromeMediaRouter_jni.h" #include "url/gurl.h" @@ -71,10 +72,14 @@ void MediaRouterAndroid::CreateRoute( const MediaSink::Id& sink_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks) { + const std::vector& callbacks, + base::TimeDelta timeout) { + // TODO(avayvod): Implement timeouts (crbug.com/583036). if (!origin.is_valid()) { + scoped_ptr result = RouteRequestResult::FromError( + "Invalid origin", RouteRequestResult::INVALID_ORIGIN); for (const MediaRouteResponseCallback& callback : callbacks) - callback.Run(nullptr, "", "Invalid origin"); + callback.Run(*result); return; } @@ -121,7 +126,8 @@ void MediaRouterAndroid::ConnectRouteByRouteId( const MediaRoute::Id& route_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks) { + const std::vector& callbacks, + base::TimeDelta timeout) { NOTIMPLEMENTED(); } @@ -130,10 +136,14 @@ void MediaRouterAndroid::JoinRoute( const std::string& presentation_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks) { + const std::vector& callbacks, + base::TimeDelta timeout) { + // TODO(avayvod): Implement timeouts (crbug.com/583036). if (!origin.is_valid()) { + scoped_ptr result = RouteRequestResult::FromError( + "Invalid origin", RouteRequestResult::INVALID_ORIGIN); for (const MediaRouteResponseCallback& callback : callbacks) - callback.Run(nullptr, "", "Invalid origin"); + callback.Run(*result); return; } @@ -365,8 +375,10 @@ void MediaRouterAndroid::OnRouteCreated( jis_local, std::string(), true); // TODO(avayvod): Populate for_display. + scoped_ptr result = RouteRequestResult::FromSuccess( + make_scoped_ptr(new MediaRoute(route)), request->presentation_id); for (const MediaRouteResponseCallback& callback : request->callbacks) - callback.Run(&route, request->presentation_id, std::string()); + callback.Run(*result); route_requests_.Remove(jroute_request_id); @@ -384,10 +396,12 @@ void MediaRouterAndroid::OnRouteRequestError( if (!request) return; - std::string error_text = ConvertJavaStringToUTF8(env, jerror_text); - + // TODO(imcheng): Provide a more specific result code. + scoped_ptr result = + RouteRequestResult::FromError(ConvertJavaStringToUTF8(env, jerror_text), + RouteRequestResult::UNKNOWN_ERROR); for (const MediaRouteResponseCallback& callback : request->callbacks) - callback.Run(nullptr, std::string(), error_text); + callback.Run(*result); route_requests_.Remove(jroute_request_id); } diff --git a/chrome/browser/media/android/router/media_router_android.h b/chrome/browser/media/android/router/media_router_android.h index e25e134b9cef2e..bae085b18ba807 100644 --- a/chrome/browser/media/android/router/media_router_android.h +++ b/chrome/browser/media/android/router/media_router_android.h @@ -32,24 +32,25 @@ class MediaRouterAndroid : public MediaRouterBase { const MediaRoute* FindRouteBySource(const MediaSource::Id& source_id) const; // MediaRouter implementation. - void CreateRoute( - const MediaSource::Id& source_id, - const MediaSink::Id& sink_id, - const GURL& origin, - content::WebContents* web_contents, - const std::vector& callbacks) override; - void JoinRoute( - const MediaSource::Id& source, - const std::string& presentation_id, - const GURL& origin, - content::WebContents* web_contents, - const std::vector& callbacks) override; + void CreateRoute(const MediaSource::Id& source_id, + const MediaSink::Id& sink_id, + const GURL& origin, + content::WebContents* web_contents, + const std::vector& callbacks, + base::TimeDelta timeout) override; + void JoinRoute(const MediaSource::Id& source, + const std::string& presentation_id, + const GURL& origin, + content::WebContents* web_contents, + const std::vector& callbacks, + base::TimeDelta timeout) override; void ConnectRouteByRouteId( const MediaSource::Id& source, const MediaRoute::Id& route_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks) override; + const std::vector& callbacks, + base::TimeDelta timeout) 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/android/router/media_router_dialog_controller_android.cc b/chrome/browser/media/android/router/media_router_dialog_controller_android.cc index cd153f6181d903..399c609e9cce8e 100644 --- a/chrome/browser/media/android/router/media_router_dialog_controller_android.cc +++ b/chrome/browser/media/android/router/media_router_dialog_controller_android.cc @@ -53,12 +53,8 @@ void MediaRouterDialogControllerAndroid::OnSinkSelected( MediaRouter* router = MediaRouterFactory::GetApiForBrowserContext( initiator()->GetBrowserContext()); - router->CreateRoute( - source_id, - ConvertJavaStringToUTF8(env, jsink_id), - origin, - initiator(), - route_response_callbacks); + router->CreateRoute(source_id, ConvertJavaStringToUTF8(env, jsink_id), origin, + initiator(), route_response_callbacks, base::TimeDelta()); } void MediaRouterDialogControllerAndroid::OnRouteClosed( diff --git a/chrome/browser/media/router/create_presentation_connection_request.cc b/chrome/browser/media/router/create_presentation_connection_request.cc index 984f676bdc2c5c..e9d728933f916b 100644 --- a/chrome/browser/media/router/create_presentation_connection_request.cc +++ b/chrome/browser/media/router/create_presentation_connection_request.cc @@ -5,6 +5,7 @@ #include "chrome/browser/media/router/create_presentation_connection_request.h" #include "chrome/browser/media/router/media_source_helper.h" +#include "chrome/browser/media/router/route_request_result.h" using content::PresentationSessionInfo; using content::PresentationError; @@ -57,15 +58,13 @@ void CreatePresentationConnectionRequest::InvokeErrorCallback( // static void CreatePresentationConnectionRequest::HandleRouteResponse( scoped_ptr presentation_request, - const MediaRoute* route, - const std::string& presentation_id, - const std::string& error) { - if (!route) { - presentation_request->InvokeErrorCallback( - content::PresentationError(content::PRESENTATION_ERROR_UNKNOWN, error)); + const RouteRequestResult& result) { + if (!result.route()) { + presentation_request->InvokeErrorCallback(content::PresentationError( + content::PRESENTATION_ERROR_UNKNOWN, result.error())); } else { - presentation_request->InvokeSuccessCallback(presentation_id, - route->media_route_id()); + presentation_request->InvokeSuccessCallback( + result.presentation_id(), result.route()->media_route_id()); } } diff --git a/chrome/browser/media/router/create_presentation_connection_request.h b/chrome/browser/media/router/create_presentation_connection_request.h index 024dd1bcfdd60b..e85f0c005add07 100644 --- a/chrome/browser/media/router/create_presentation_connection_request.h +++ b/chrome/browser/media/router/create_presentation_connection_request.h @@ -22,6 +22,8 @@ struct PresentationSessionInfo; namespace media_router { +class RouteRequestResult; + // Holds parameters for creating a presentation session. // A request object is created by presentation_service_delegate_impl when it // gets create-session request. The object is then passed to and owned by the @@ -62,9 +64,7 @@ class CreatePresentationConnectionRequest { // Handle route creation/joining response by invoking the right callback. static void HandleRouteResponse( scoped_ptr presentation_request, - const MediaRoute* route, - const std::string& presentation_id, - const std::string& error); + const RouteRequestResult& result); private: const PresentationRequest presentation_request_; diff --git a/chrome/browser/media/router/media_router.gypi b/chrome/browser/media/router/media_router.gypi index c66b9f62f93fce..7556a6c7106ec2 100644 --- a/chrome/browser/media/router/media_router.gypi +++ b/chrome/browser/media/router/media_router.gypi @@ -44,6 +44,8 @@ 'presentation_session_messages_observer.cc', 'presentation_session_messages_observer.h', 'render_frame_host_id.h', + 'route_request_result.cc', + 'route_request_result.h', ], # Files that are only needed on desktop builds 'media_router_non_android_sources': [ diff --git a/chrome/browser/media/router/media_router.h b/chrome/browser/media/router/media_router.h index 12ae3290a4764e..349c8b221a0c48 100644 --- a/chrome/browser/media/router/media_router.h +++ b/chrome/browser/media/router/media_router.h @@ -13,6 +13,7 @@ #include "base/callback.h" #include "base/callback_list.h" #include "base/memory/scoped_vector.h" +#include "base/time/time.h" #include "chrome/browser/media/router/issue.h" #include "chrome/browser/media/router/media_route.h" #include "chrome/browser/media/router/media_sink.h" @@ -32,25 +33,13 @@ class MediaRoutesObserver; class MediaSinksObserver; class PresentationConnectionStateObserver; class PresentationSessionMessagesObserver; +class RouteRequestResult; // Type of callback used in |CreateRoute()|, |JoinRoute()|, and -// |ConnectRouteByRouteId()|. Callback is -// invoked when the route request either succeeded or failed. -// On success: -// |route|: The route created or joined. -// |presentation_id|: -// The presentation ID of the route created or joined. In the case of -// |CreateRoute()|, the ID is generated by MediaRouter and is guaranteed to -// be unique. -// |error|: Empty string. -// On failure: -// |route|: nullptr -// |presentation_id|: Empty string. -// |error|: Non-empty string describing the error. +// |ConnectRouteByRouteId()|. Callback is invoked when the route request either +// succeeded or failed. using MediaRouteResponseCallback = - base::Callback; + base::Callback; // Subscription object returned by calling // |AddPresentationConnectionStateChangedCallback|. See the method comments for @@ -62,6 +51,7 @@ using PresentationConnectionStateSubscription = base::CallbackList& callbacks) = 0; + const std::vector& callbacks, + base::TimeDelta timeout) = 0; // Creates a route and connects it to an existing route identified by // |route_id|. |route_id| must refer to a non-local route, unnassociated with @@ -97,12 +90,15 @@ class MediaRouter : public KeyedService { // (See CreateRoute documentation). // Each callback in |callbacks| is invoked with a response indicating // 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. virtual void ConnectRouteByRouteId( const MediaSource::Id& source_id, const MediaRoute::Id& route_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks) = 0; + const std::vector& callbacks, + base::TimeDelta timeout) = 0; // Joins an existing route identified by |presentation_id|. // |source|: The source to route to the existing route. @@ -112,12 +108,15 @@ class MediaRouter : public KeyedService { // (See CreateRoute documentation). // Each callback in |callbacks| is invoked with a response indicating // 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. virtual void JoinRoute( const MediaSource::Id& source, const std::string& presentation_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks) = 0; + const std::vector& callbacks, + base::TimeDelta timeout) = 0; // Terminates the media route specified by |route_id|. virtual void TerminateRoute(const MediaRoute::Id& route_id) = 0; diff --git a/chrome/browser/media/router/media_router.mojom b/chrome/browser/media/router/media_router.mojom index af4b90cece83dc..fe45555b335277 100644 --- a/chrome/browser/media/router/media_router.mojom +++ b/chrome/browser/media/router/media_router.mojom @@ -111,6 +111,14 @@ struct RouteMessage { array? data; }; +// Maps to a ResultCode value in route_request_result.h +// The enum defined here is a subset of those defined in route_request_result.h. +enum RouteRequestResultCode { + UNKNOWN_ERROR, + OK, + TIMED_OUT +}; + // Modeled after the MediaRouter interface defined in // chrome/browser/media/router/media_router.h interface MediaRouteProvider { @@ -122,28 +130,42 @@ interface MediaRouteProvider { // same-tab scopes. // Since int types cannot be made optional, use -1 as |tab_id| in cases where // it is not applicable. + // If |timeout_millis| is positive, it will be used in place of the default + // 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| // will be set. + // |result| will be set to OK if successful, or an error code if an error + // occurred. CreateRoute(string media_source, string sink_id, string original_presentation_id, string origin, - int32 tab_id) => (MediaRoute? route, string? error_text); + int32 tab_id, + int64 timeout_millis) => (MediaRoute? route, + 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) + // If |timeout_millis| is positive, it will be used in place of the default + // 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| // will be set. + // |result| will be set to OK if successful, or an error code if an error + // occurred. JoinRoute(string media_source, string presentation_id, string origin, - int32 tab_id) => (MediaRoute? route, string? error_text); + int32 tab_id, + int64 timeout_millis) => (MediaRoute? route, + 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 @@ -151,16 +173,23 @@ interface MediaRouteProvider { // 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. + // 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) // 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, string route_id, string presentation_id, string origin, - int32 tab_id) => - (MediaRoute? route, string? error_text); + int32 tab_id, + int64 timeout_millis) => + (MediaRoute? route, + string? error_text, + RouteRequestResultCode result_code); // Terminates the route specified by |route_id|. TerminateRoute(string route_id); diff --git a/chrome/browser/media/router/media_router_mojo_impl.cc b/chrome/browser/media/router/media_router_mojo_impl.cc index 3a6ee713a00abb..d8fa96320112f4 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.cc +++ b/chrome/browser/media/router/media_router_mojo_impl.cc @@ -140,7 +140,8 @@ void MediaRouterMojoImpl::RegisterMediaRouteProvider( if (event_page_tracker_->IsEventPageSuspended( media_route_provider_extension_id_)) { DVLOG_WITH_INSTANCE(1) - << "ExecutePendingRequests was called while extension is suspended."; + << "RegisterMediaRouteProvider was called while extension is " + "suspended."; media_route_provider_.reset(); SetWakeReason(MediaRouteProviderWakeReason::REGISTER_MEDIA_ROUTE_PROVIDER); AttemptWakeEventPage(); @@ -223,21 +224,25 @@ void MediaRouterMojoImpl::RouteResponseReceived( const std::string& presentation_id, const std::vector& callbacks, interfaces::MediaRoutePtr media_route, - const mojo::String& error_text) { - scoped_ptr route; - std::string actual_presentation_id; - std::string error; + const mojo::String& error_text, + interfaces::RouteRequestResultCode result_code) { + scoped_ptr result; if (media_route.is_null()) { // An error occurred. DCHECK(!error_text.is_null()); - error = !error_text.get().empty() ? error_text.get() : "Unknown error."; + std::string error = + !error_text.get().empty() ? error_text.get() : "Unknown error."; + + result = RouteRequestResult::FromError( + error, mojo::RouteRequestResultCodeFromMojo(result_code)); } else { - route = media_route.To>(); - actual_presentation_id = presentation_id; + result = RouteRequestResult::FromSuccess( + media_route.To>(), presentation_id); } + // TODO(imcheng): Add UMA histogram based on result code (crbug.com/583044). for (const MediaRouteResponseCallback& callback : callbacks) - callback.Run(route.get(), actual_presentation_id, error); + callback.Run(*result); } void MediaRouterMojoImpl::CreateRoute( @@ -245,21 +250,25 @@ void MediaRouterMojoImpl::CreateRoute( const MediaSink::Id& sink_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks) { + const std::vector& callbacks, + base::TimeDelta timeout) { DCHECK(thread_checker_.CalledOnValidThread()); if (!origin.is_valid()) { DVLOG_WITH_INSTANCE(1) << "Invalid origin: " << origin; + scoped_ptr result = RouteRequestResult::FromError( + "Invalid origin", RouteRequestResult::INVALID_ORIGIN); for (const MediaRouteResponseCallback& callback : callbacks) - callback.Run(nullptr, "", "Invalid origin"); + callback.Run(*result); return; } SetWakeReason(MediaRouteProviderWakeReason::CREATE_ROUTE); int tab_id = SessionTabHelper::IdForTab(web_contents); - RunOrDefer(base::Bind( - &MediaRouterMojoImpl::DoCreateRoute, base::Unretained(this), source_id, - sink_id, origin.is_empty() ? "" : origin.spec(), tab_id, callbacks)); + RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoCreateRoute, + base::Unretained(this), source_id, sink_id, + origin.is_empty() ? "" : origin.spec(), tab_id, + callbacks, timeout)); } void MediaRouterMojoImpl::JoinRoute( @@ -267,13 +276,16 @@ void MediaRouterMojoImpl::JoinRoute( const std::string& presentation_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks) { + const std::vector& callbacks, + base::TimeDelta timeout) { DCHECK(thread_checker_.CalledOnValidThread()); if (!origin.is_valid()) { DVLOG_WITH_INSTANCE(1) << "Invalid origin: " << origin; + scoped_ptr result = RouteRequestResult::FromError( + "Invalid origin", RouteRequestResult::INVALID_ORIGIN); for (const MediaRouteResponseCallback& callback : callbacks) - callback.Run(nullptr, "", "Invalid origin"); + callback.Run(*result); return; } @@ -282,7 +294,7 @@ void MediaRouterMojoImpl::JoinRoute( RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoJoinRoute, base::Unretained(this), source_id, presentation_id, origin.is_empty() ? "" : origin.spec(), tab_id, - callbacks)); + callbacks, timeout)); } void MediaRouterMojoImpl::ConnectRouteByRouteId( @@ -290,13 +302,16 @@ void MediaRouterMojoImpl::ConnectRouteByRouteId( const MediaRoute::Id& route_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks) { + const std::vector& callbacks, + base::TimeDelta timeout) { DCHECK(thread_checker_.CalledOnValidThread()); if (!origin.is_valid()) { DVLOG_WITH_INSTANCE(1) << "Invalid origin: " << origin; + scoped_ptr result = RouteRequestResult::FromError( + "Invalid origin", RouteRequestResult::INVALID_ORIGIN); for (const MediaRouteResponseCallback& callback : callbacks) - callback.Run(nullptr, "", "Invalid origin"); + callback.Run(*result); return; } @@ -305,7 +320,7 @@ void MediaRouterMojoImpl::ConnectRouteByRouteId( RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoConnectRouteByRouteId, base::Unretained(this), source_id, route_id, origin.is_empty() ? "" : origin.spec(), tab_id, - callbacks)); + callbacks, timeout)); } void MediaRouterMojoImpl::TerminateRoute(const MediaRoute::Id& route_id) { @@ -518,15 +533,18 @@ void MediaRouterMojoImpl::DoCreateRoute( const MediaSink::Id& sink_id, const std::string& origin, int tab_id, - const std::vector& callbacks) { + const std::vector& callbacks, + base::TimeDelta timeout) { std::string presentation_id("mr_"); presentation_id += base::GenerateGUID(); DVLOG_WITH_INSTANCE(1) << "DoCreateRoute " << source_id << "=>" << sink_id << ", presentation ID: " << presentation_id; + media_route_provider_->CreateRoute( source_id, sink_id, presentation_id, origin, tab_id, + timeout > base::TimeDelta() ? timeout.InMilliseconds() : 0, base::Bind(&MediaRouterMojoImpl::RouteResponseReceived, - base::Unretained(this), presentation_id, callbacks)); + base::Unretained(this), presentation_id, callbacks)); } void MediaRouterMojoImpl::DoJoinRoute( @@ -534,13 +552,16 @@ void MediaRouterMojoImpl::DoJoinRoute( const std::string& presentation_id, const std::string& origin, int tab_id, - const std::vector& callbacks) { + const std::vector& callbacks, + base::TimeDelta timeout) { DVLOG_WITH_INSTANCE(1) << "DoJoinRoute " << source_id << ", presentation ID: " << presentation_id; + media_route_provider_->JoinRoute( source_id, presentation_id, origin, tab_id, + timeout > base::TimeDelta() ? timeout.InMilliseconds() : 0, base::Bind(&MediaRouterMojoImpl::RouteResponseReceived, - base::Unretained(this), presentation_id, callbacks)); + base::Unretained(this), presentation_id, callbacks)); } void MediaRouterMojoImpl::DoConnectRouteByRouteId( @@ -548,16 +569,19 @@ void MediaRouterMojoImpl::DoConnectRouteByRouteId( const MediaRoute::Id& route_id, const std::string& origin, int tab_id, - const std::vector& callbacks) { + const std::vector& callbacks, + base::TimeDelta timeout) { std::string presentation_id("mr_"); presentation_id += base::GenerateGUID(); DVLOG_WITH_INSTANCE(1) << "DoConnectRouteByRouteId " << source_id << ", route ID: " << route_id << ", presentation ID: " << presentation_id; + 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)); + base::Unretained(this), presentation_id, callbacks)); } void MediaRouterMojoImpl::DoTerminateRoute(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 a8d94926eed85f..cce100ccf640d8 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.h +++ b/chrome/browser/media/router/media_router_mojo_impl.h @@ -64,24 +64,25 @@ class MediaRouterMojoImpl : public MediaRouterBase, // MediaRouter implementation. // Execution of the requests is delegated to the Do* methods, which can be // enqueued for later use if the extension is temporarily suspended. - void CreateRoute( - const MediaSource::Id& source_id, - const MediaSink::Id& sink_id, - const GURL& origin, - content::WebContents* web_contents, - const std::vector& callbacks) override; - void JoinRoute( - const MediaSource::Id& source_id, - const std::string& presentation_id, - const GURL& origin, - content::WebContents* web_contents, - const std::vector& callbacks) override; + void CreateRoute(const MediaSource::Id& source_id, + const MediaSink::Id& sink_id, + const GURL& origin, + content::WebContents* web_contents, + const std::vector& callbacks, + base::TimeDelta timeout) override; + void JoinRoute(const MediaSource::Id& source_id, + const std::string& presentation_id, + const GURL& origin, + content::WebContents* web_contents, + const std::vector& callbacks, + base::TimeDelta timeout) override; void ConnectRouteByRouteId( const MediaSource::Id& source, const MediaRoute::Id& route_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks) override; + const std::vector& callbacks, + base::TimeDelta timeout) override; void TerminateRoute(const MediaRoute::Id& route_id) override; void DetachRoute(const MediaRoute::Id& route_id) override; void SendRouteMessage(const MediaRoute::Id& route_id, @@ -210,17 +211,21 @@ class MediaRouterMojoImpl : public MediaRouterBase, const MediaSink::Id& sink_id, const std::string& origin, int tab_id, - const std::vector& callbacks); + const std::vector& callbacks, + base::TimeDelta timeout); void DoJoinRoute(const MediaSource::Id& source_id, const std::string& presentation_id, const std::string& origin, int tab_id, - const std::vector& callbacks); - void DoConnectRouteByRouteId(const MediaSource::Id& source_id, - const MediaRoute::Id& route_id, - const std::string& origin, - int tab_id, - const std::vector& callbacks); + const std::vector& callbacks, + base::TimeDelta timeout); + void DoConnectRouteByRouteId( + const MediaSource::Id& source_id, + const MediaRoute::Id& route_id, + const std::string& origin, + int tab_id, + const std::vector& callbacks, + base::TimeDelta timeout); void DoTerminateRoute(const MediaRoute::Id& route_id); void DoDetachRoute(const MediaRoute::Id& route_id); void DoSendSessionMessage(const MediaRoute::Id& route_id, @@ -268,10 +273,11 @@ class MediaRouterMojoImpl : public MediaRouterBase, // Converts the callback result of calling Mojo CreateRoute()/JoinRoute() // into a local callback. void RouteResponseReceived( - const std::string& presentation_id, - const std::vector& callbacks, - interfaces::MediaRoutePtr media_route, - const mojo::String& error_text); + const std::string& presentation_id, + const std::vector& callbacks, + interfaces::MediaRoutePtr media_route, + const mojo::String& error_text, + interfaces::RouteRequestResultCode result_code); // Callback invoked by |event_page_tracker_| after an attempt to wake the // component extension. If |success| is false, the pending request queue is 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 a79b5f89e06c1f..e9ff83dc5baab4 100644 --- a/chrome/browser/media/router/media_router_mojo_impl_unittest.cc +++ b/chrome/browser/media/router/media_router_mojo_impl_unittest.cc @@ -67,6 +67,7 @@ const char kPresentationId[] = "presentationId"; const char kOrigin[] = "http://origin/"; const int kInvalidTabId = -1; const uint8_t kBinaryMessage[] = {0x01, 0x02, 0x03, 0x04}; +const int kTimeoutMillis = 5 * 1000; bool ArePresentationSessionMessagesEqual( const content::PresentationSessionMessage* expected, @@ -96,10 +97,15 @@ interfaces::IssuePtr CreateMojoIssue(const std::string& title) { class RouteResponseCallbackHandler { public: - MOCK_METHOD3(Invoke, + void Invoke(const RouteRequestResult& result) { + DoInvoke(result.route(), result.presentation_id(), result.error(), + result.result_code()); + } + MOCK_METHOD4(DoInvoke, void(const MediaRoute* route, const std::string& presentation_id, - const std::string& error_text)); + const std::string& error_text, + RouteRequestResult::ResultCode result_code)); }; class SendMessageCallbackHandler { @@ -179,63 +185,68 @@ TEST_F(MediaRouterMojoImplTest, CreateRoute) { MediaSource media_source(kSource); MediaRoute expected_route(kRouteId, media_source, kSinkId, "", false, "", false); - 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; // 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_, CreateRoute(mojo::String(kSource), mojo::String(kSinkId), _, - mojo::String(kOrigin), kInvalidTabId, _)) - .WillOnce(Invoke([&route]( - const mojo::String& source, const mojo::String& sink, - const mojo::String& presentation_id, const mojo::String& origin, - int tab_id, - const interfaces::MediaRouteProvider::CreateRouteCallback& cb) { - cb.Run(std::move(route), mojo::String()); - })); + mojo::String(kOrigin), kInvalidTabId, _, _)) + .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, + 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; + cb.Run(std::move(route), mojo::String(), + interfaces::RouteRequestResultCode::OK); + })); base::RunLoop run_loop; RouteResponseCallbackHandler handler; - EXPECT_CALL(handler, Invoke(Pointee(Equals(expected_route)), Not(""), "")) + EXPECT_CALL(handler, DoInvoke(Pointee(Equals(expected_route)), Not(""), "", + RouteRequestResult::OK)) .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); std::vector 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); + route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis)); run_loop.Run(); } TEST_F(MediaRouterMojoImplTest, CreateRouteFails) { - EXPECT_CALL(mock_media_route_provider_, - CreateRoute(mojo::String(kSource), mojo::String(kSinkId), _, - mojo::String(kOrigin), kInvalidTabId, _)) + EXPECT_CALL( + mock_media_route_provider_, + CreateRoute(mojo::String(kSource), mojo::String(kSinkId), _, + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _)) .WillOnce(Invoke( [](const mojo::String& source, const mojo::String& sink, const mojo::String& presentation_id, const mojo::String& origin, - int tab_id, + int tab_id, int64_t timeout_millis, const interfaces::MediaRouteProvider::CreateRouteCallback& cb) { - cb.Run(interfaces::MediaRoutePtr(), mojo::String(kError)); + cb.Run(interfaces::MediaRoutePtr(), mojo::String(kError), + interfaces::RouteRequestResultCode::TIMED_OUT); })); RouteResponseCallbackHandler handler; base::RunLoop run_loop; - EXPECT_CALL(handler, Invoke(nullptr, "", kError)) - .WillOnce(InvokeWithoutArgs([&run_loop]() { - run_loop.Quit(); - })); + EXPECT_CALL(handler, + DoInvoke(nullptr, "", kError, RouteRequestResult::TIMED_OUT)) + .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); std::vector 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); + route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis)); run_loop.Run(); } @@ -254,50 +265,56 @@ TEST_F(MediaRouterMojoImplTest, JoinRoute) { // 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, _)) + EXPECT_CALL( + mock_media_route_provider_, + JoinRoute(mojo::String(kSource), mojo::String(kPresentationId), + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _)) .WillOnce(Invoke([&route]( const mojo::String& source, const mojo::String& presentation_id, - const mojo::String& origin, int tab_id, + const mojo::String& origin, int tab_id, int64_t timeout_millis, const interfaces::MediaRouteProvider::JoinRouteCallback& cb) { - cb.Run(std::move(route), mojo::String()); + cb.Run(std::move(route), mojo::String(), + interfaces::RouteRequestResultCode::OK); })); RouteResponseCallbackHandler handler; base::RunLoop run_loop; - EXPECT_CALL(handler, Invoke(Pointee(Equals(expected_route)), Not(""), "")) + EXPECT_CALL(handler, DoInvoke(Pointee(Equals(expected_route)), Not(""), "", + RouteRequestResult::OK)) .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); std::vector 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); + route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis)); run_loop.Run(); } TEST_F(MediaRouterMojoImplTest, JoinRouteFails) { - EXPECT_CALL(mock_media_route_provider_, - JoinRoute(mojo::String(kSource), mojo::String(kPresentationId), - mojo::String(kOrigin), kInvalidTabId, _)) + EXPECT_CALL( + mock_media_route_provider_, + JoinRoute(mojo::String(kSource), mojo::String(kPresentationId), + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _)) .WillOnce(Invoke( [](const mojo::String& source, const mojo::String& presentation_id, - const mojo::String& origin, int tab_id, + const mojo::String& origin, int tab_id, int64_t timeout_millis, const interfaces::MediaRouteProvider::JoinRouteCallback& cb) { - cb.Run(interfaces::MediaRoutePtr(), mojo::String(kError)); + cb.Run(interfaces::MediaRoutePtr(), mojo::String(kError), + interfaces::RouteRequestResultCode::TIMED_OUT); })); RouteResponseCallbackHandler handler; base::RunLoop run_loop; - EXPECT_CALL(handler, Invoke(nullptr, "", kError)) - .WillOnce(InvokeWithoutArgs([&run_loop]() { - run_loop.Quit(); - })); + EXPECT_CALL(handler, + DoInvoke(nullptr, "", kError, RouteRequestResult::TIMED_OUT)) + .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); std::vector 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); + route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis)); run_loop.Run(); } @@ -317,54 +334,57 @@ TEST_F(MediaRouterMojoImplTest, ConnectRouteByRouteId) { // 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, _)) + ConnectRouteByRouteId( + mojo::String(kSource), mojo::String(kRouteId), _, + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _)) .WillOnce(Invoke([&route]( const mojo::String& source, const mojo::String& route_id, - const mojo::String& presentation_id, - const mojo::String& origin, int tab_id, + const mojo::String& presentation_id, const mojo::String& origin, + int tab_id, int64_t timeout_millis, const interfaces::MediaRouteProvider::JoinRouteCallback& cb) { - cb.Run(std::move(route), mojo::String()); + cb.Run(std::move(route), mojo::String(), + interfaces::RouteRequestResultCode::OK); })); RouteResponseCallbackHandler handler; base::RunLoop run_loop; - EXPECT_CALL(handler, Invoke(Pointee(Equals(expected_route)), Not(""), "")) + EXPECT_CALL(handler, DoInvoke(Pointee(Equals(expected_route)), Not(""), "", + RouteRequestResult::OK)) .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); std::vector 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); + router()->ConnectRouteByRouteId( + kSource, kRouteId, GURL(kOrigin), nullptr, route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis)); run_loop.Run(); } TEST_F(MediaRouterMojoImplTest, ConnectRouteByRouteIdFails) { EXPECT_CALL(mock_media_route_provider_, - ConnectRouteByRouteId(mojo::String(kSource), - mojo::String(kRouteId), _, - mojo::String(kOrigin), kInvalidTabId, _)) + ConnectRouteByRouteId( + mojo::String(kSource), mojo::String(kRouteId), _, + mojo::String(kOrigin), kInvalidTabId, kTimeoutMillis, _)) .WillOnce(Invoke( - [](const mojo::String& source, - const mojo::String& route_id, - const mojo::String& presentation_id, - const mojo::String& origin, int tab_id, + [](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, const interfaces::MediaRouteProvider::JoinRouteCallback& cb) { - cb.Run(interfaces::MediaRoutePtr(), mojo::String(kError)); + cb.Run(interfaces::MediaRoutePtr(), mojo::String(kError), + interfaces::RouteRequestResultCode::TIMED_OUT); })); RouteResponseCallbackHandler handler; base::RunLoop run_loop; - EXPECT_CALL(handler, Invoke(nullptr, "", kError)) - .WillOnce(InvokeWithoutArgs([&run_loop]() { - run_loop.Quit(); - })); + EXPECT_CALL(handler, + DoInvoke(nullptr, "", kError, RouteRequestResult::TIMED_OUT)) + .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); std::vector 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); + router()->ConnectRouteByRouteId( + kSource, kRouteId, GURL(kOrigin), nullptr, route_response_callbacks, + base::TimeDelta::FromMilliseconds(kTimeoutMillis)); run_loop.Run(); } diff --git a/chrome/browser/media/router/media_router_type_converters.cc b/chrome/browser/media/router/media_router_type_converters.cc index 7542f24aace745..82b6062128ca46 100644 --- a/chrome/browser/media/router/media_router_type_converters.cc +++ b/chrome/browser/media/router/media_router_type_converters.cc @@ -12,6 +12,7 @@ using media_router::interfaces::MediaSinkPtr; using PresentationConnectionState = media_router::interfaces::MediaRouter::PresentationConnectionState; +using RouteRequestResultCode = media_router::interfaces::RouteRequestResultCode; namespace mojo { @@ -150,4 +151,19 @@ content::PresentationConnectionState PresentationConnectionStateFromMojo( } } +media_router::RouteRequestResult::ResultCode RouteRequestResultCodeFromMojo( + RouteRequestResultCode result_code) { + switch (result_code) { + case RouteRequestResultCode::UNKNOWN_ERROR: + return media_router::RouteRequestResult::UNKNOWN_ERROR; + case RouteRequestResultCode::OK: + return media_router::RouteRequestResult::OK; + case RouteRequestResultCode::TIMED_OUT: + return media_router::RouteRequestResult::TIMED_OUT; + default: + NOTREACHED() << "Unknown RouteRequestResultCode " << result_code; + return media_router::RouteRequestResult::UNKNOWN_ERROR; + } +} + } // namespace mojo diff --git a/chrome/browser/media/router/media_router_type_converters.h b/chrome/browser/media/router/media_router_type_converters.h index 8dc5dbea1ed485..05f587afdd11c0 100644 --- a/chrome/browser/media/router/media_router_type_converters.h +++ b/chrome/browser/media/router/media_router_type_converters.h @@ -10,9 +10,11 @@ #include "base/memory/scoped_ptr.h" #include "chrome/browser/media/router/issue.h" +#include "chrome/browser/media/router/media_router.h" #include "chrome/browser/media/router/media_router.mojom.h" #include "chrome/browser/media/router/media_sink.h" #include "chrome/browser/media/router/media_source.h" +#include "chrome/browser/media/router/route_request_result.h" #include "content/public/browser/presentation_session.h" #include "mojo/common/common_type_converters.h" @@ -67,6 +69,10 @@ struct TypeConverter { content::PresentationConnectionState PresentationConnectionStateFromMojo( media_router::interfaces::MediaRouter::PresentationConnectionState state); +// RouteRequestResult conversion. +media_router::RouteRequestResult::ResultCode RouteRequestResultCodeFromMojo( + media_router::interfaces::RouteRequestResultCode result_code); + } // namespace mojo #endif // CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTER_TYPE_CONVERTERS_H_ diff --git a/chrome/browser/media/router/mock_media_router.h b/chrome/browser/media/router/mock_media_router.h index 68466d6d43c793..b57b76c6f14950 100644 --- a/chrome/browser/media/router/mock_media_router.h +++ b/chrome/browser/media/router/mock_media_router.h @@ -25,24 +25,27 @@ class MockMediaRouter : public MediaRouter { MockMediaRouter(); ~MockMediaRouter() override; - MOCK_METHOD5(CreateRoute, + MOCK_METHOD6(CreateRoute, void(const MediaSource::Id& source, const MediaSink::Id& sink_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks)); - MOCK_METHOD5(JoinRoute, + const std::vector& callbacks, + base::TimeDelta timeout)); + MOCK_METHOD6(JoinRoute, void(const MediaSource::Id& source, const std::string& presentation_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks)); - MOCK_METHOD5(ConnectRouteByRouteId, + const std::vector& callbacks, + base::TimeDelta timeout)); + MOCK_METHOD6(ConnectRouteByRouteId, void(const MediaSource::Id& source, const MediaRoute::Id& route_id, const GURL& origin, content::WebContents* web_contents, - const std::vector& callbacks)); + const std::vector& callbacks, + base::TimeDelta timeout)); MOCK_METHOD1(DetachRoute, void(const MediaRoute::Id& route_id)); MOCK_METHOD1(TerminateRoute, void(const MediaRoute::Id& route_id)); MOCK_METHOD3(SendRouteMessage, diff --git a/chrome/browser/media/router/presentation_service_delegate_impl.cc b/chrome/browser/media/router/presentation_service_delegate_impl.cc index ee50d71f6a75cb..c0a85e5c54ebb6 100644 --- a/chrome/browser/media/router/presentation_service_delegate_impl.cc +++ b/chrome/browser/media/router/presentation_service_delegate_impl.cc @@ -21,6 +21,7 @@ #include "chrome/browser/media/router/media_source_helper.h" #include "chrome/browser/media/router/presentation_media_sinks_observer.h" #include "chrome/browser/media/router/presentation_session_messages_observer.h" +#include "chrome/browser/media/router/route_request_result.h" #include "chrome/browser/sessions/session_tab_helper.h" #include "content/public/browser/presentation_screen_availability_listener.h" #include "content/public/browser/presentation_session.h" @@ -661,21 +662,19 @@ void PresentationServiceDelegateImpl::OnJoinRouteResponse( const content::PresentationSessionInfo& session, const content::PresentationSessionStartedCallback& success_cb, const content::PresentationSessionErrorCallback& error_cb, - const MediaRoute* route, - const std::string& presentation_id, - const std::string& error_text) { - if (!route) { + const RouteRequestResult& result) { + if (!result.route()) { error_cb.Run(content::PresentationError( - content::PRESENTATION_ERROR_NO_PRESENTATION_FOUND, error_text)); + content::PRESENTATION_ERROR_NO_PRESENTATION_FOUND, result.error())); } else { DVLOG(1) << "OnJoinRouteResponse: " - << "route_id: " << route->media_route_id() + << "route_id: " << result.route()->media_route_id() << ", presentation URL: " << session.presentation_url << ", presentation ID: " << session.presentation_id; - DCHECK_EQ(session.presentation_id, presentation_id); + DCHECK_EQ(session.presentation_id, result.presentation_id()); frame_manager_->OnPresentationSessionStarted( RenderFrameHostId(render_process_id, render_frame_id), session, - route->media_route_id()); + result.route()->media_route_id()); success_cb.Run(session); } } @@ -745,7 +744,7 @@ void PresentationServiceDelegateImpl::JoinSession( GetLastCommittedURLForFrame( RenderFrameHostId(render_process_id, render_frame_id)) .GetOrigin(), - web_contents_, route_response_callbacks); + web_contents_, route_response_callbacks, base::TimeDelta()); } void PresentationServiceDelegateImpl::CloseConnection( @@ -828,16 +827,14 @@ void PresentationServiceDelegateImpl::ListenForConnectionStateChange( void PresentationServiceDelegateImpl::OnRouteResponse( const PresentationRequest& presentation_request, - const MediaRoute* route, - const std::string& presentation_id, - const std::string& error) { - if (!route) + const RouteRequestResult& result) { + if (!result.route()) return; content::PresentationSessionInfo session_info( - presentation_request.presentation_url(), presentation_id); + presentation_request.presentation_url(), result.presentation_id()); frame_manager_->OnDefaultPresentationSessionStarted( - presentation_request, session_info, route->media_route_id()); + presentation_request, session_info, result.route()->media_route_id()); } void PresentationServiceDelegateImpl::AddDefaultPresentationRequestObserver( diff --git a/chrome/browser/media/router/presentation_service_delegate_impl.h b/chrome/browser/media/router/presentation_service_delegate_impl.h index 53a818da018f37..54d40da5f7936c 100644 --- a/chrome/browser/media/router/presentation_service_delegate_impl.h +++ b/chrome/browser/media/router/presentation_service_delegate_impl.h @@ -35,6 +35,7 @@ namespace media_router { class MediaRoute; class MediaSinksObserver; class PresentationFrameManager; +class RouteRequestResult; // Implementation of PresentationServiceDelegate that interfaces an // instance of WebContents with the Chrome Media Router. It uses the Media @@ -132,9 +133,7 @@ class PresentationServiceDelegateImpl // Callback invoked when a default PresentationRequest is started from a // browser-initiated dialog. void OnRouteResponse(const PresentationRequest& request, - const MediaRoute* route, - const std::string& presentation_id, - const std::string& error); + const RouteRequestResult& result); // Adds / removes an observer for listening to default PresentationRequest // changes. This class does not own |observer|. When |observer| is about to @@ -188,9 +187,7 @@ class PresentationServiceDelegateImpl const content::PresentationSessionInfo& session, const content::PresentationSessionStartedCallback& success_cb, const content::PresentationSessionErrorCallback& error_cb, - const MediaRoute* route, - const std::string& presentation_id, - const std::string& error_text); + const RouteRequestResult& result); void OnStartSessionSucceeded( int render_process_id, diff --git a/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc b/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc index 260ef430e9642d..f3f20845e64c84 100644 --- a/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc +++ b/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc @@ -10,6 +10,7 @@ #include "chrome/browser/media/router/mock_media_router.h" #include "chrome/browser/media/router/mock_screen_availability_listener.h" #include "chrome/browser/media/router/presentation_service_delegate_impl.h" +#include "chrome/browser/media/router/route_request_result.h" #include "chrome/browser/media/router/test_helper.h" #include "chrome/grit/generated_resources.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" @@ -193,7 +194,9 @@ TEST_F(PresentationServiceDelegateImplTest, DefaultPresentationUrlCallback) { PresentationRequest request = delegate_impl_->GetDefaultPresentationRequest(); // Should not trigger callback since route response is error. - delegate_impl_->OnRouteResponse(request, nullptr, "", "Error"); + scoped_ptr result = + RouteRequestResult::FromError("Error", RouteRequestResult::UNKNOWN_ERROR); + delegate_impl_->OnRouteResponse(request, *result); EXPECT_TRUE(Mock::VerifyAndClearExpectations(this)); // Should not trigger callback since request doesn't match. @@ -201,18 +204,22 @@ TEST_F(PresentationServiceDelegateImplTest, DefaultPresentationUrlCallback) { PresentationRequest different_request(RenderFrameHostId(100, 200), presentation_url2, GURL("http://anotherFrameUrl.fakeUrl")); - MediaRoute different_route("differentRouteId", - MediaSourceForPresentationUrl(presentation_url2), - "mediaSinkId", "", true, "", true); - delegate_impl_->OnRouteResponse(different_request, &different_route, - "differentPresentationId", ""); + result = RouteRequestResult::FromSuccess( + make_scoped_ptr(new MediaRoute( + "differentRouteId", MediaSourceForPresentationUrl(presentation_url2), + "mediaSinkId", "", true, "", true)), + "differentPresentationId"); + delegate_impl_->OnRouteResponse(different_request, *result); EXPECT_TRUE(Mock::VerifyAndClearExpectations(this)); // Should trigger callback since request matches. - MediaRoute route("routeId", MediaSourceForPresentationUrl(presentation_url1), - "mediaSinkId", "", true, "", true); EXPECT_CALL(*this, OnDefaultPresentationStarted(_)).Times(1); - delegate_impl_->OnRouteResponse(request, &route, "presentationId", ""); + result = RouteRequestResult::FromSuccess( + make_scoped_ptr(new MediaRoute( + "routeId", MediaSourceForPresentationUrl(presentation_url1), + "mediaSinkId", "", true, "", true)), + "presentationId"); + delegate_impl_->OnRouteResponse(request, *result); } TEST_F(PresentationServiceDelegateImplTest, @@ -276,7 +283,7 @@ TEST_F(PresentationServiceDelegateImplTest, ListenForConnnectionStateChange) { // Set up a PresentationConnection so we can listen to it. std::vector route_response_callbacks; - EXPECT_CALL(router_, JoinRoute(_, _, _, _, _)) + EXPECT_CALL(router_, JoinRoute(_, _, _, _, _, _)) .WillOnce(SaveArg<4>(&route_response_callbacks)); const std::string kPresentationUrl("http://url1.fakeUrl"); @@ -293,10 +300,13 @@ TEST_F(PresentationServiceDelegateImplTest, ListenForConnnectionStateChange) { EXPECT_CALL(mock_create_connection_callbacks, OnCreateConnectionSuccess(_)) .Times(1); - MediaRoute route("routeId", MediaSourceForPresentationUrl(kPresentationUrl), - "mediaSinkId", "description", true, "", true); + scoped_ptr result = RouteRequestResult::FromSuccess( + make_scoped_ptr(new MediaRoute( + "routeId", MediaSourceForPresentationUrl(kPresentationUrl), + "mediaSinkId", "description", true, "", true)), + kPresentationId); for (const auto& route_response_callback : route_response_callbacks) - route_response_callback.Run(&route, kPresentationId, ""); + route_response_callback.Run(*result); MockPresentationConnectionStateChangedCallback mock_callback; content::PresentationConnectionStateChangedCallback callback = diff --git a/chrome/browser/media/router/route_request_result.cc b/chrome/browser/media/router/route_request_result.cc new file mode 100644 index 00000000000000..39b3687cb4b37c --- /dev/null +++ b/chrome/browser/media/router/route_request_result.cc @@ -0,0 +1,39 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/media/router/route_request_result.h" + +#include "chrome/browser/media/router/media_route.h" + +namespace media_router { + +// static +scoped_ptr RouteRequestResult::FromSuccess( + scoped_ptr route, + const std::string& presentation_id) { + DCHECK(route); + return make_scoped_ptr(new RouteRequestResult( + std::move(route), presentation_id, "", RouteRequestResult::OK)); +} + +// static +scoped_ptr RouteRequestResult::FromError( + const std::string& error, + ResultCode result_code) { + return make_scoped_ptr( + new RouteRequestResult(nullptr, "", error, result_code)); +} + +RouteRequestResult::RouteRequestResult(scoped_ptr route, + const std::string& presentation_id, + const std::string& error, + ResultCode result_code) + : route_(std::move(route)), + presentation_id_(presentation_id), + error_(error), + result_code_(result_code) {} + +RouteRequestResult::~RouteRequestResult() = default; + +} // namespace media_router diff --git a/chrome/browser/media/router/route_request_result.h b/chrome/browser/media/router/route_request_result.h new file mode 100644 index 00000000000000..6fbfa6064cc54a --- /dev/null +++ b/chrome/browser/media/router/route_request_result.h @@ -0,0 +1,66 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_MEDIA_ROUTER_ROUTE_REQUEST_RESULT_H_ +#define CHROME_BROWSER_MEDIA_ROUTER_ROUTE_REQUEST_RESULT_H_ + +#include + +#include "base/macros.h" +#include "base/memory/scoped_ptr.h" + +namespace media_router { + +class MediaRoute; + +// Holds the result of a successful or failed route request. +// On success: +// |route|: The route created or joined. +// |presentation_id|: +// The presentation ID of the route created or joined. In the case of +// |CreateRoute()|, the ID is generated by MediaRouter and is guaranteed to +// be unique. +// |error|: Empty string. +// |result_code|: RouteRequestResult::OK +// On failure: +// |route|: nullptr +// |presentation_id|: Empty string. +// |error|: Non-empty string describing the error. +// |result_code|: A value from RouteRequestResult describing the error. +class RouteRequestResult { + public: + enum ResultCode { UNKNOWN_ERROR, OK, TIMED_OUT, INVALID_ORIGIN }; + + static scoped_ptr FromSuccess( + scoped_ptr route, + const std::string& presentation_id); + static scoped_ptr FromError(const std::string& error, + ResultCode result_code); + + ~RouteRequestResult(); + + // Note the caller does not own the returned MediaRoute. The caller must + // create a copy if they wish to use it after this object is destroyed. + const MediaRoute* route() const { return route_.get(); } + std::string presentation_id() const { return presentation_id_; } + std::string error() const { return error_; } + ResultCode result_code() const { return result_code_; } + + private: + RouteRequestResult(scoped_ptr route, + const std::string& presentation_id, + const std::string& error, + ResultCode result_code); + + scoped_ptr route_; + std::string presentation_id_; + std::string error_; + ResultCode result_code_; + + DISALLOW_COPY_AND_ASSIGN(RouteRequestResult); +}; + +} // namespace media_router + +#endif // CHROME_BROWSER_MEDIA_ROUTER_ROUTE_REQUEST_RESULT_H_ diff --git a/chrome/browser/media/router/test_helper.h b/chrome/browser/media/router/test_helper.h index 066e6e19af07d9..ca87e7a4a0bbf1 100644 --- a/chrome/browser/media/router/test_helper.h +++ b/chrome/browser/media/router/test_helper.h @@ -88,25 +88,28 @@ class MockMediaRouteProvider : public interfaces::MediaRouteProvider { MockMediaRouteProvider(); ~MockMediaRouteProvider() override; - MOCK_METHOD6(CreateRoute, + MOCK_METHOD7(CreateRoute, void(const mojo::String& source_urn, const mojo::String& sink_id, const mojo::String& presentation_id, const mojo::String& origin, int tab_id, + int64_t timeout_secs, const CreateRouteCallback& callback)); - MOCK_METHOD5(JoinRoute, + MOCK_METHOD6(JoinRoute, void(const mojo::String& source_urn, const mojo::String& presentation_id, const mojo::String& origin, int tab_id, + int64_t timeout_secs, const JoinRouteCallback& callback)); - MOCK_METHOD6(ConnectRouteByRouteId, + MOCK_METHOD7(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, 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/resources/media_router/elements/media_router_container/media_router_container.js b/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js index 15659452b66c53..531df0dba31d95 100644 --- a/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js +++ b/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js @@ -954,13 +954,6 @@ Polymer({ this.mouseIsPositionedOverDialog_ = true; }, - /** - * Handles timeout of previous create route attempt. - */ - onNotifyRouteCreationTimeout: function() { - this.resetRouteCreationProperties_(false); - }, - /** * Called when a sink is clicked. * diff --git a/chrome/browser/resources/media_router/media_router_ui_interface.js b/chrome/browser/resources/media_router/media_router_ui_interface.js index a90ad914ff741d..031cf2b03a34db 100644 --- a/chrome/browser/resources/media_router/media_router_ui_interface.js +++ b/chrome/browser/resources/media_router/media_router_ui_interface.js @@ -10,13 +10,6 @@ cr.define('media_router.ui', function() { // The media-router-container element. var container = null; - /** - * Handles timeout of previous create route attempt. - */ - function onNotifyRouteCreationTimeout() { - container.onNotifyRouteCreationTimeout(); - } - /** * Handles response of previous create route attempt. * @@ -130,7 +123,6 @@ cr.define('media_router.ui', function() { } return { - onNotifyRouteCreationTimeout: onNotifyRouteCreationTimeout, onCreateRouteResponseReceived: onCreateRouteResponseReceived, setCastModeList: setCastModeList, setContainer: setContainer, diff --git a/chrome/browser/ui/ash/cast_config_delegate_media_router.cc b/chrome/browser/ui/ash/cast_config_delegate_media_router.cc index db84ed37587b53..2fe6a874938927 100644 --- a/chrome/browser/ui/ash/cast_config_delegate_media_router.cc +++ b/chrome/browser/ui/ash/cast_config_delegate_media_router.cc @@ -192,10 +192,12 @@ void CastConfigDelegateMediaRouter::RequestDeviceRefresh() { void CastConfigDelegateMediaRouter::CastToReceiver( const std::string& receiver_id) { + // TODO(imcheng): Pass in tab casting timeout. GetMediaRouter()->CreateRoute( media_router::MediaSourceForDesktop().id(), receiver_id, GURL("http://cros-cast-origin/"), nullptr, - std::vector()); + std::vector(), + base::TimeDelta()); } void CastConfigDelegateMediaRouter::StopCasting(const std::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 9ceaded12d2a37..5036350f0a3fda 100644 --- a/chrome/browser/ui/webui/media_router/media_router_ui.cc +++ b/chrome/browser/ui/webui/media_router/media_router_ui.cc @@ -29,6 +29,7 @@ #include "chrome/browser/media/router/media_source.h" #include "chrome/browser/media/router/media_source_helper.h" #include "chrome/browser/media/router/presentation_service_delegate_impl.h" +#include "chrome/browser/media/router/route_request_result.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sessions/session_tab_helper.h" #include "chrome/browser/ui/webui/media_router/media_router_localized_strings_provider.h" @@ -52,6 +53,8 @@ namespace { // The amount of time to wait for a response when creating a new route. const int kCreateRouteTimeoutSeconds = 20; +const int kCreateRouteTimeoutSecondsForTab = 60; +const int kCreateRouteTimeoutSecondsForDesktop = 120; std::string GetHostFromURL(const GURL& gurl) { if (gurl.is_empty()) return std::string(); @@ -70,6 +73,20 @@ std::string TruncateHost(const std::string& host) { return truncated.empty() ? host : truncated; } +base::TimeDelta GetRouteRequestTimeout(MediaCastMode cast_mode) { + switch (cast_mode) { + case DEFAULT: + return base::TimeDelta::FromSeconds(kCreateRouteTimeoutSeconds); + case TAB_MIRROR: + return base::TimeDelta::FromSeconds(kCreateRouteTimeoutSecondsForTab); + case DESKTOP_MIRROR: + return base::TimeDelta::FromSeconds(kCreateRouteTimeoutSecondsForDesktop); + default: + NOTREACHED(); + return base::TimeDelta(); + } +} + } // namespace // static @@ -158,8 +175,8 @@ MediaRouterUI::MediaRouterUI(content::WebUI* web_ui) content::WebContents* wc = web_ui->GetWebContents(); DCHECK(wc); - router_ = static_cast( - MediaRouterFactory::GetApiForBrowserContext(wc->GetBrowserContext())); + router_ = + MediaRouterFactory::GetApiForBrowserContext(wc->GetBrowserContext()); // Allows UI to load extensionview. // TODO(haibinlu): limit object-src to current extension once crbug/514866 @@ -258,6 +275,14 @@ void MediaRouterUI::InitCommon(content::WebContents* initiator) { UpdateCastModes(); } +void MediaRouterUI::InitForTest(MediaRouter* router, + content::WebContents* initiator, + MediaRouterWebUIMessageHandler* handler) { + router_ = router; + handler_ = handler; + InitCommon(initiator); +} + void MediaRouterUI::OnDefaultPresentationChanged( const PresentationRequest& presentation_request) { MediaSource source = presentation_request.GetMediaSource(); @@ -379,17 +404,13 @@ bool MediaRouterUI::CreateOrConnectRoute(const MediaSink::Id& sink_id, } } - // Start the timer. - route_creation_timer_.Start( - FROM_HERE, base::TimeDelta::FromSeconds(kCreateRouteTimeoutSeconds), this, - &MediaRouterUI::RouteCreationTimeout); - + base::TimeDelta timeout = GetRouteRequestTimeout(cast_mode); if (route_id.empty()) { router_->CreateRoute(source.id(), sink_id, origin, initiator_, - route_response_callbacks); + route_response_callbacks, timeout); } else { - router_->ConnectRouteByRouteId(source.id(), route_id, origin, - initiator_, route_response_callbacks); + router_->ConnectRouteByRouteId(source.id(), route_id, origin, initiator_, + route_response_callbacks, timeout); } return true; } @@ -451,29 +472,28 @@ void MediaRouterUI::OnRoutesUpdated( if (ui_initialized_) handler_->UpdateRoutes(routes_, joinable_route_ids_); } -void MediaRouterUI::OnRouteResponseReceived(const int route_request_id, +void MediaRouterUI::OnRouteResponseReceived(int route_request_id, const MediaSink::Id& sink_id, - const MediaRoute* route, - const std::string& presentation_id, - const std::string& error) { + const RouteRequestResult& result) { DVLOG(1) << "OnRouteResponseReceived"; // If we receive a new route that we aren't expecting, do nothing. if (route_request_id != current_route_request_id_) return; + const MediaRoute* route = result.route(); if (!route) { // The provider will handle sending an issue for a failed route request. - DVLOG(0) << "MediaRouteResponse returned error: " << error; + DVLOG(1) << "MediaRouteResponse returned error: " << result.error(); } std::string route_id = route ? route->media_route_id() : std::string(); handler_->OnCreateRouteResponseReceived(sink_id, route_id); current_route_request_id_ = -1; - route_creation_timer_.Stop(); -} -void MediaRouterUI::RouteCreationTimeout() { - current_route_request_id_ = -1; + if (result.result_code() == RouteRequestResult::TIMED_OUT) + SendIssueForRouteTimeout(); +} +void MediaRouterUI::SendIssueForRouteTimeout() { base::string16 host = base::UTF8ToUTF16(GetTruncatedPresentationRequestSourceName()); @@ -490,7 +510,6 @@ void MediaRouterUI::RouteCreationTimeout() { std::vector(), std::string(), Issue::NOTIFICATION, false, std::string()); AddIssue(issue); - handler_->NotifyRouteCreationTimeout(); } GURL MediaRouterUI::GetFrameURL() const { @@ -514,7 +533,9 @@ std::string MediaRouterUI::GetTruncatedPresentationRequestSourceName() const { } const std::string& MediaRouterUI::GetRouteProviderExtensionId() const { - return router_->media_route_provider_extension_id(); + // TODO(imcheng): Get rid of this hack once we no longer need this method. + return static_cast(router_) + ->media_route_provider_extension_id(); } void MediaRouterUI::SetUIInitializationTimer(const base::Time& start_time) { diff --git a/chrome/browser/ui/webui/media_router/media_router_ui.h b/chrome/browser/ui/webui/media_router/media_router_ui.h index 575b8f565b4cb4..017417cf07bf6c 100644 --- a/chrome/browser/ui/webui/media_router/media_router_ui.h +++ b/chrome/browser/ui/webui/media_router/media_router_ui.h @@ -38,16 +38,16 @@ class Collator; namespace media_router { +class CreatePresentationConnectionRequest; class IssuesObserver; class MediaRoute; class MediaRouter; class MediaRouterDialogCallbacks; -class MediaRouterMojoImpl; -class MediaRouterWebUIMessageHandler; class MediaRoutesObserver; +class MediaRouterWebUIMessageHandler; class MediaSink; class MediaSinksObserver; -class CreatePresentationConnectionRequest; +class RouteRequestResult; // Implements the chrome://media-router user interface. class MediaRouterUI : public ConstrainedWebDialogUI, @@ -140,6 +140,10 @@ class MediaRouterUI : public ConstrainedWebDialogUI, void UpdateMaxDialogHeight(int height); + void InitForTest(MediaRouter* router, + content::WebContents* initiator, + MediaRouterWebUIMessageHandler* handler); + private: FRIEND_TEST_ALL_PREFIXES(MediaRouterUITest, SortedSinks); FRIEND_TEST_ALL_PREFIXES(MediaRouterUITest, @@ -151,6 +155,8 @@ class MediaRouterUI : public ConstrainedWebDialogUI, GetExtensionNameEmptyWhenNotInstalled); FRIEND_TEST_ALL_PREFIXES(MediaRouterUITest, GetExtensionNameEmptyWhenNotExtensionURL); + FRIEND_TEST_ALL_PREFIXES(MediaRouterUITest, + RouteCreationTimeoutForPresentation); class UIIssuesObserver; @@ -193,14 +199,12 @@ class MediaRouterUI : public ConstrainedWebDialogUI, // Callback passed to MediaRouter to receive response to route creation // requests. - void OnRouteResponseReceived(const int route_request_id, + void OnRouteResponseReceived(int route_request_id, const MediaSink::Id& sink_id, - const MediaRoute* route, - const std::string& presentation_id, - const std::string& error); + const RouteRequestResult& result); - // Creates and sends an issue if route creation times out. - void RouteCreationTimeout(); + // Creates and sends an issue if route creation timed out. + void SendIssueForRouteTimeout(); // Initializes the dialog with mirroring sources derived from |initiator|. void InitCommon(content::WebContents* initiator); @@ -277,10 +281,7 @@ class MediaRouterUI : public ConstrainedWebDialogUI, content::WebContents* initiator_; // Pointer to the MediaRouter for this instance's BrowserContext. - MediaRouterMojoImpl* router_; - - // Timer used to implement a timeout on a create route request. - base::OneShotTimer route_creation_timer_; + MediaRouter* router_; // The start time for UI initialization metrics timer. When a dialog has been // been painted and initialized with initial data, this should be cleared. diff --git a/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc b/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc index 7acfb6fa6108d3..2a92d49cd155a7 100644 --- a/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc +++ b/chrome/browser/ui/webui/media_router/media_router_ui_unittest.cc @@ -5,7 +5,9 @@ #include "base/bind.h" #include "chrome/browser/media/router/media_route.h" #include "chrome/browser/media/router/mock_media_router.h" +#include "chrome/browser/media/router/route_request_result.h" #include "chrome/browser/ui/webui/media_router/media_router_ui.h" +#include "chrome/browser/ui/webui/media_router/media_router_webui_message_handler.h" #include "chrome/test/base/testing_profile.h" #include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_web_ui.h" @@ -18,7 +20,9 @@ #include "testing/gtest/include/gtest/gtest.h" using testing::_; +using testing::AnyNumber; using testing::SaveArg; +using testing::Return; namespace media_router { @@ -32,22 +36,76 @@ class MockRoutesUpdatedCallback { class MediaRouterUITest : public ::testing::Test { public: MediaRouterUITest() { + initiator_.reset(content::WebContents::Create( + content::WebContents::CreateParams(&profile_))); web_contents_.reset(content::WebContents::Create( content::WebContents::CreateParams(&profile_))); web_ui_.set_web_contents(web_contents_.get()); media_router_ui_.reset(new MediaRouterUI(&web_ui_)); + message_handler_.reset( + new MediaRouterWebUIMessageHandler(media_router_ui_.get())); + EXPECT_CALL(mock_router_, RegisterMediaSinksObserver(_)) + .WillRepeatedly(Return(true)); + EXPECT_CALL(mock_router_, RegisterMediaRoutesObserver(_)) + .Times(AnyNumber()); + media_router_ui_->InitForTest(&mock_router_, initiator_.get(), + message_handler_.get()); + message_handler_->SetWebUIForTest(&web_ui_); } - ~MediaRouterUITest() override = default; + ~MediaRouterUITest() override { + EXPECT_CALL(mock_router_, UnregisterMediaSinksObserver(_)) + .Times(AnyNumber()); + EXPECT_CALL(mock_router_, UnregisterMediaRoutesObserver(_)) + .Times(AnyNumber()); + } protected: + MockMediaRouter mock_router_; content::TestBrowserThreadBundle thread_bundle_; TestingProfile profile_; + scoped_ptr initiator_; content::TestWebUI web_ui_; scoped_ptr web_contents_; scoped_ptr media_router_ui_; + scoped_ptr message_handler_; }; +TEST_F(MediaRouterUITest, RouteRequestTimedOut) { + std::vector callbacks; + EXPECT_CALL(mock_router_, CreateRoute(_, _, _, _, _, _)) + .WillOnce(SaveArg<4>(&callbacks)); + media_router_ui_->CreateRoute("sinkId", MediaCastMode::TAB_MIRROR); + + EXPECT_CALL(mock_router_, AddIssue(_)); + scoped_ptr result = + RouteRequestResult::FromError("Timed out", RouteRequestResult::TIMED_OUT); + for (const auto& callback : callbacks) + callback.Run(*result); +} + +TEST_F(MediaRouterUITest, RouteCreationTimeoutForTab) { + EXPECT_CALL(mock_router_, + CreateRoute(_, _, _, _, _, base::TimeDelta::FromSeconds(60))); + media_router_ui_->CreateRoute("sinkId", MediaCastMode::TAB_MIRROR); +} + +TEST_F(MediaRouterUITest, RouteCreationTimeoutForDesktop) { + EXPECT_CALL(mock_router_, + CreateRoute(_, _, _, _, _, base::TimeDelta::FromSeconds(120))); + media_router_ui_->CreateRoute("sinkId", MediaCastMode::DESKTOP_MIRROR); +} + +TEST_F(MediaRouterUITest, RouteCreationTimeoutForPresentation) { + PresentationRequest presentation_request( + RenderFrameHostId(0, 0), "https://fooUrl", GURL("https://frameUrl")); + media_router_ui_->OnDefaultPresentationChanged(presentation_request); + + EXPECT_CALL(mock_router_, + CreateRoute(_, _, _, _, _, base::TimeDelta::FromSeconds(20))); + media_router_ui_->CreateRoute("sinkId", MediaCastMode::DEFAULT); +} + TEST_F(MediaRouterUITest, SortedSinks) { std::vector unsorted_sinks; std::string sink_id1("sink3"); @@ -77,13 +135,12 @@ TEST_F(MediaRouterUITest, SortedSinks) { } TEST_F(MediaRouterUITest, UIMediaRoutesObserverFiltersNonDisplayRoutes) { - MockMediaRouter mock_router; - EXPECT_CALL(mock_router, RegisterMediaRoutesObserver(_)).Times(1); + EXPECT_CALL(mock_router_, RegisterMediaRoutesObserver(_)).Times(1); MediaSource media_source("mediaSource"); MockRoutesUpdatedCallback mock_callback; scoped_ptr observer( new MediaRouterUI::UIMediaRoutesObserver( - &mock_router, media_source.id(), + &mock_router_, media_source.id(), base::Bind(&MockRoutesUpdatedCallback::OnRoutesUpdated, base::Unretained(&mock_callback)))); @@ -110,21 +167,20 @@ TEST_F(MediaRouterUITest, UIMediaRoutesObserverFiltersNonDisplayRoutes) { EXPECT_TRUE(display_route_2.Equals(filtered_routes[1])); EXPECT_TRUE(filtered_routes[1].for_display()); - EXPECT_CALL(mock_router, UnregisterMediaRoutesObserver(_)).Times(1); + EXPECT_CALL(mock_router_, UnregisterMediaRoutesObserver(_)).Times(1); observer.reset(); } TEST_F(MediaRouterUITest, UIMediaRoutesObserverFiltersNonDisplayJoinableRoutes) { - MockMediaRouter mock_router; - EXPECT_CALL(mock_router, RegisterMediaRoutesObserver(_)).Times(1); + EXPECT_CALL(mock_router_, RegisterMediaRoutesObserver(_)).Times(1); MediaSource media_source("mediaSource"); MockRoutesUpdatedCallback mock_callback; scoped_ptr observer( new MediaRouterUI::UIMediaRoutesObserver( - &mock_router, media_source.id(), + &mock_router_, media_source.id(), base::Bind(&MockRoutesUpdatedCallback::OnRoutesUpdated, - base::Unretained(&mock_callback)))); + base::Unretained(&mock_callback)))); MediaRoute display_route_1("routeId1", media_source, "sinkId1", "desc 1", true, "", true); @@ -153,7 +209,7 @@ TEST_F(MediaRouterUITest, EXPECT_EQ(display_route_1.media_route_id(), filtered_joinable_route_ids[0]); EXPECT_EQ(display_route_2.media_route_id(), filtered_joinable_route_ids[1]); - EXPECT_CALL(mock_router, UnregisterMediaRoutesObserver(_)).Times(1); + EXPECT_CALL(mock_router_, UnregisterMediaRoutesObserver(_)).Times(1); observer.reset(); } diff --git a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc index 5416645dded23d..13d80283657403 100644 --- a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc +++ b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc @@ -19,6 +19,7 @@ #include "chrome/common/pref_names.h" #include "chrome/grit/generated_resources.h" #include "components/prefs/pref_service.h" +#include "content/public/browser/web_ui.h" #include "extensions/common/constants.h" #include "ui/base/l10n/l10n_util.h" @@ -59,8 +60,6 @@ const char kOnInitialDataReceived[] = "onInitialDataReceived"; // JS function names. const char kSetInitialData[] = "media_router.ui.setInitialData"; -const char kNotifyRouteCreationTimeout[] = - "media_router.ui.onNotifyRouteCreationTimeout"; const char kOnCreateRouteResponseReceived[] = "media_router.ui.onCreateRouteResponseReceived"; const char kSetIssue[] = "media_router.ui.setIssue"; @@ -251,11 +250,6 @@ void MediaRouterWebUIMessageHandler::UpdateMaxDialogHeight(int height) { base::FundamentalValue(height)); } -void MediaRouterWebUIMessageHandler::NotifyRouteCreationTimeout() { - DVLOG(2) << "NotifyRouteCreationTimeout"; - web_ui()->CallJavascriptFunction(kNotifyRouteCreationTimeout); -} - void MediaRouterWebUIMessageHandler::RegisterMessages() { web_ui()->RegisterMessageCallback( kRequestInitialData, @@ -691,4 +685,8 @@ bool MediaRouterWebUIMessageHandler::ActOnIssueType( } } +void MediaRouterWebUIMessageHandler::SetWebUIForTest(content::WebUI* web_ui) { + set_web_ui(web_ui); +} + } // namespace media_router diff --git a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.h b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.h index 562ba43a09ee12..8c6ea5a72cf899 100644 --- a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.h +++ b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.h @@ -19,6 +19,10 @@ class DictionaryValue; class ListValue; } // namespace base +namespace content { +class WebUI; +} + namespace media_router { class Issue; @@ -48,8 +52,7 @@ class MediaRouterWebUIMessageHandler : public content::WebUIMessageHandler { // the browser window changes. void UpdateMaxDialogHeight(int height); - // Notifies the dialog that the route creation attempt timed out. - void NotifyRouteCreationTimeout(); + void SetWebUIForTest(content::WebUI* webui); private: // WebUIMessageHandler implementation. diff --git a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler_unittest.cc b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler_unittest.cc index 39b534100368d6..1521000a799498 100644 --- a/chrome/browser/ui/webui/media_router/media_router_webui_message_handler_unittest.cc +++ b/chrome/browser/ui/webui/media_router/media_router_webui_message_handler_unittest.cc @@ -31,21 +31,6 @@ class MockMediaRouterUI : public MediaRouterUI { MOCK_CONST_METHOD0(GetRouteProviderExtensionId, const std::string&()); }; -class TestingMediaRouterWebUIMessageHandler - : public MediaRouterWebUIMessageHandler { - public: - TestingMediaRouterWebUIMessageHandler(content::WebUI* web_ui, - MediaRouterUI* media_router_ui) - : MediaRouterWebUIMessageHandler(media_router_ui) { - set_web_ui(web_ui); - } - - ~TestingMediaRouterWebUIMessageHandler() override { set_web_ui(nullptr); } - - private: - DISALLOW_COPY_AND_ASSIGN(TestingMediaRouterWebUIMessageHandler); -}; - class MediaRouterWebUIMessageHandlerTest : public MediaRouterTest { public: MediaRouterWebUIMessageHandlerTest() @@ -60,8 +45,9 @@ class MediaRouterWebUIMessageHandlerTest : public MediaRouterTest { web_ui_->set_web_contents( browser()->tab_strip_model()->GetActiveWebContents()); mock_media_router_ui_.reset(new MockMediaRouterUI(web_ui_.get())); - handler_.reset(new TestingMediaRouterWebUIMessageHandler( - web_ui_.get(), mock_media_router_ui_.get())); + handler_.reset( + new MediaRouterWebUIMessageHandler(mock_media_router_ui_.get())); + handler_->SetWebUIForTest(web_ui_.get()); } void TearDown() override { diff --git a/chrome/test/media_router/media_router_e2e_browsertest.cc b/chrome/test/media_router/media_router_e2e_browsertest.cc index 5395ab75a8be3f..5f0d671b81b0d3 100644 --- a/chrome/test/media_router/media_router_e2e_browsertest.cc +++ b/chrome/test/media_router/media_router_e2e_browsertest.cc @@ -13,6 +13,7 @@ #include "chrome/browser/media/router/media_router_mojo_impl.h" #include "chrome/browser/media/router/media_source.h" #include "chrome/browser/media/router/media_source_helper.h" +#include "chrome/browser/media/router/route_request_result.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sessions/session_tab_helper.h" #include "chrome/browser/ui/browser_finder.h" @@ -71,11 +72,9 @@ void MediaRouterE2EBrowserTest::TearDownOnMainThread() { } void MediaRouterE2EBrowserTest::OnRouteResponseReceived( - const MediaRoute* route, - const std::string& presentation_id, - const std::string& error) { - ASSERT_TRUE(route); - route_id_ = route->media_route_id(); + const RouteRequestResult& result) { + ASSERT_TRUE(result.route()); + route_id_ = result.route()->media_route_id(); } void MediaRouterE2EBrowserTest::CreateMediaRoute( @@ -103,7 +102,7 @@ void MediaRouterE2EBrowserTest::CreateMediaRoute( base::Bind(&MediaRouterE2EBrowserTest::OnRouteResponseReceived, base::Unretained(this))); media_router_->CreateRoute(source.id(), sink.id(), origin, web_contents, - route_response_callbacks); + route_response_callbacks, base::TimeDelta()); // Wait for the route request to be fulfilled (and route to be started). ASSERT_TRUE(ConditionalWait( diff --git a/chrome/test/media_router/media_router_e2e_browsertest.h b/chrome/test/media_router/media_router_e2e_browsertest.h index 0419becb3c46a0..ded831d028a3c7 100644 --- a/chrome/test/media_router/media_router_e2e_browsertest.h +++ b/chrome/test/media_router/media_router_e2e_browsertest.h @@ -16,6 +16,7 @@ namespace media_router { class MediaRouter; +class RouteRequestResult; class MediaRouterE2EBrowserTest : public MediaRouterIntegrationBrowserTest { public: @@ -30,9 +31,7 @@ class MediaRouterE2EBrowserTest : public MediaRouterIntegrationBrowserTest { // Callback from MediaRouter when a response to a media route request is // received. - void OnRouteResponseReceived(const MediaRoute* route, - const std::string& presentation_id, - const std::string& error); + void OnRouteResponseReceived(const RouteRequestResult& result); // Initializes |observer_| to listen for sinks compatible with |source|, // finds sink with name matching receiver_, and establishes media diff --git a/extensions/renderer/resources/media_router_bindings.js b/extensions/renderer/resources/media_router_bindings.js index 7d793b604745c8..0ad0ae88b49782 100644 --- a/extensions/renderer/resources/media_router_bindings.js +++ b/extensions/renderer/resources/media_router_bindings.js @@ -122,6 +122,43 @@ define('media_router_bindings', [ } } + /** + * Parses the given route request Error object and converts it to the + * corresponding result code. + * @param {!Error} error + * @return {!mediaRouterMojom.RouteRequestResultCode} + */ + function getRouteRequestResultCode_(error) { + if (error.message.startsWith('timeout')) + return mediaRouterMojom.RouteRequestResultCode.TIMED_OUT; + else + return mediaRouterMojom.RouteRequestResultCode.UNKNOWN_ERROR; + } + + /** + * Creates and returns a successful route response from given route. + * @param {!MediaRoute} route + * @return {!Object} + */ + function toSuccessRouteResponse_(route) { + return { + route: routeToMojo_(route), + result_code: mediaRouterMojom.RouteRequestResultCode.OK + }; + } + + /** + * Creates and returns a error route response from given Error object + * @param {!Error} error + * @return {!Object} + */ + function toErrorRouteResponse_(error) { + return { + error_text: 'Error creating route: ' + error.message, + result_code: getRouteRequestResultCode_(error) + }; + } + /** * Creates a new MediaRouter. * Converts a route struct to its Mojo form. @@ -481,19 +518,23 @@ define('media_router_bindings', [ * requesting presentation. TODO(mfoltz): Remove. * @param {!string} origin Origin of site requesting presentation. * @param {!number} tabId ID of tab requesting presentation. + * @param {!number} timeoutMillis If positive, the timeout duration for the + * request, measured in seconds. Otherwise, the default duration will be + * used. * @return {!Promise.} A Promise resolving to an object describing * the newly created media route, or rejecting with an error message on * failure. */ MediaRouteProvider.prototype.createRoute = - function(sourceUrn, sinkId, presentationId, origin, tabId) { + function(sourceUrn, sinkId, presentationId, origin, tabId, + timeoutMillis) { return this.handlers_.createRoute( - sourceUrn, sinkId, presentationId, origin, tabId) + sourceUrn, sinkId, presentationId, origin, tabId, timeoutMillis) .then(function(route) { - return {route: routeToMojo_(route)}; - }.bind(this)) - .catch(function(err) { - return {error_text: 'Error creating route: ' + err.message}; + return toSuccessRouteResponse_(route); + }, + function(err) { + return toErrorRouteResponse_(err); }); }; @@ -505,18 +546,22 @@ define('media_router_bindings', [ * @param {!string} presentationId Presentation ID to join. * @param {!string} origin Origin of site requesting join. * @param {!number} tabId ID of tab requesting join. + * @param {!number} timeoutMillis If positive, the timeout duration for the + * request, measured in seconds. Otherwise, the default duration will be + * used. * @return {!Promise.} A Promise resolving to an object describing * the newly created media route, or rejecting with an error message on * failure. */ MediaRouteProvider.prototype.joinRoute = - function(sourceUrn, presentationId, origin, tabId) { - return this.handlers_.joinRoute(sourceUrn, presentationId, origin, tabId) - .then(function(newRoute) { - return {route: routeToMojo_(newRoute)}; + function(sourceUrn, presentationId, origin, tabId, timeoutMillis) { + return this.handlers_.joinRoute( + sourceUrn, presentationId, origin, tabId, timeoutMillis) + .then(function(route) { + return toSuccessRouteResponse_(route); }, function(err) { - return {error_text: 'Error joining route: ' + err.message}; + return toErrorRouteResponse_(err); }); }; @@ -529,19 +574,23 @@ define('media_router_bindings', [ * @param {!string} presentationId Presentation ID to join. * @param {!string} origin Origin of site requesting join. * @param {!number} tabId ID of tab requesting join. + * @param {!number} timeoutMillis If positive, the timeout duration for the + * request, measured in seconds. Otherwise, the default duration will be + * used. * @return {!Promise.} 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) { + function(sourceUrn, routeId, presentationId, origin, tabId, + timeoutMillis) { return this.handlers_.connectRouteByRouteId( - sourceUrn, routeId, presentationId, origin, tabId) - .then(function(newRoute) { - return {route: routeToMojo_(newRoute)}; + sourceUrn, routeId, presentationId, origin, tabId, timeoutMillis) + .then(function(route) { + return toSuccessRouteResponse_(route); }, function(err) { - return {error_text: 'Error joining route: ' + err.message}; + return toErrorRouteResponse_(err); }); };