From ed10e96f38114815495521e730d5cf29aed9f9b6 Mon Sep 17 00:00:00 2001 From: imcheng Date: Mon, 22 Feb 2016 22:34:44 -0800 Subject: [MediaRouter] Plumb PresentationConnectionClose from MR to Blink. Added some follow up plumbing in Blink for close reason, as well as to send the new event via a new method in PresentationConnection. Added plumbing in PresentationDispatcher and PresentationServiceImpl. Added a new class to hold the (state change, close) callbacks in content public API which will be used by PresentationServiceImpl and MR. Changed existing MR API to take the new callbacks object. Added new MediaRouterMojoImpl API to be notified of presentation connection close. Added plumbing in media_router_bindings.js TODO: add tests BUG=579360,574233,574234 Review URL: https://codereview.chromium.org/1701143002 Cr-Commit-Position: refs/heads/master@{#376935} --- chrome/browser/media/router/media_router.h | 2 +- chrome/browser/media/router/media_router.mojom | 12 ++++ chrome/browser/media/router/media_router_base.cc | 17 +++++- chrome/browser/media/router/media_router_base.h | 10 +++- .../browser/media/router/media_router_mojo_impl.cc | 9 +++ .../browser/media/router/media_router_mojo_impl.h | 4 ++ .../router/media_router_mojo_impl_unittest.cc | 69 ++++++++++++++-------- .../media/router/media_router_type_converters.cc | 18 ++++++ .../media/router/media_router_type_converters.h | 6 ++ chrome/browser/media/router/mock_media_router.h | 3 +- chrome/browser/media/router/test_helper.h | 8 ++- .../presentation/presentation_service_impl.cc | 15 +++-- .../presentation/presentation_service_impl.h | 7 ++- .../presentation_service_impl_unittest.cc | 61 ++++++++++++++++--- .../presentation/presentation_type_converters.cc | 15 +++++ .../presentation/presentation_type_converters.h | 3 + .../common/presentation/presentation_service.mojom | 12 ++++ .../public/browser/presentation_service_delegate.h | 19 +++++- content/public/browser/presentation_session.h | 8 +++ .../presentation/presentation_dispatcher.cc | 30 ++++++++++ .../presentation/presentation_dispatcher.h | 4 ++ .../renderer/resources/media_router_bindings.js | 39 +++++++++++- .../presentation/PresentationConnection.cpp | 31 +++++++++- .../modules/presentation/PresentationConnection.h | 3 + .../presentation/PresentationController.cpp | 10 ++++ .../modules/presentation/PresentationController.h | 2 + .../presentation/WebPresentationConnectionClient.h | 6 ++ .../presentation/WebPresentationController.h | 4 ++ 28 files changed, 376 insertions(+), 51 deletions(-) diff --git a/chrome/browser/media/router/media_router.h b/chrome/browser/media/router/media_router.h index 349c8b2..82a17aa 100644 --- a/chrome/browser/media/router/media_router.h +++ b/chrome/browser/media/router/media_router.h @@ -45,7 +45,7 @@ using MediaRouteResponseCallback = // |AddPresentationConnectionStateChangedCallback|. See the method comments for // details. using PresentationConnectionStateSubscription = base::CallbackList::Subscription; + const content::PresentationConnectionStateChangeInfo&)>::Subscription; // An interface for handling resources related to media routing. // Responsible for registering observers for receiving sink availability diff --git a/chrome/browser/media/router/media_router.mojom b/chrome/browser/media/router/media_router.mojom index fe45555..a7acc5aa 100644 --- a/chrome/browser/media/router/media_router.mojom +++ b/chrome/browser/media/router/media_router.mojom @@ -277,6 +277,13 @@ interface MediaRouter { TERMINATED }; + // Keep in sync with content/public/browser/presentation_session.h. + enum PresentationConnectionCloseReason { + CONNECTION_ERROR, + CLOSED, + WENT_AWAY + }; + // Registers a MediaRouteProvider with the MediaRouter. // Returns a string that uniquely identifies the Media Router browser // process. @@ -302,5 +309,10 @@ interface MediaRouter { // changed to |state|. OnPresentationConnectionStateChanged( string route_id, PresentationConnectionState state); + + // Called when the presentation connected to route |route_id| has closed. + OnPresentationConnectionClosed( + string route_id, PresentationConnectionCloseReason reason, + string message); }; diff --git a/chrome/browser/media/router/media_router_base.cc b/chrome/browser/media/router/media_router_base.cc index 1078152..0e8a236 100644 --- a/chrome/browser/media/router/media_router_base.cc +++ b/chrome/browser/media/router/media_router_base.cc @@ -38,7 +38,22 @@ void MediaRouterBase::NotifyPresentationConnectionStateChange( if (!callbacks) return; - callbacks->Notify(state); + callbacks->Notify(content::PresentationConnectionStateChangeInfo(state)); +} + +void MediaRouterBase::NotifyPresentationConnectionClose( + const MediaRoute::Id& route_id, + content::PresentationConnectionCloseReason reason, + const std::string& message) { + auto* callbacks = presentation_connection_state_callbacks_.get(route_id); + if (!callbacks) + return; + + content::PresentationConnectionStateChangeInfo info( + content::PRESENTATION_CONNECTION_STATE_CLOSED); + info.close_reason = reason; + info.message = message; + callbacks->Notify(info); } void MediaRouterBase::OnPresentationConnectionStateCallbackRemoved( diff --git a/chrome/browser/media/router/media_router_base.h b/chrome/browser/media/router/media_router_base.h index a202016..d342d9b 100644 --- a/chrome/browser/media/router/media_router_base.h +++ b/chrome/browser/media/router/media_router_base.h @@ -28,13 +28,19 @@ class MediaRouterBase : public MediaRouter { protected: FRIEND_TEST_ALL_PREFIXES(MediaRouterMojoImplTest, PresentationConnectionStateChangedCallback); + FRIEND_TEST_ALL_PREFIXES(MediaRouterMojoImplTest, + PresentationConnectionStateChangedCallbackRemoved); void NotifyPresentationConnectionStateChange( const MediaRoute::Id& route_id, content::PresentationConnectionState state); + void NotifyPresentationConnectionClose( + const MediaRoute::Id& route_id, + content::PresentationConnectionCloseReason reason, + const std::string& message); - using PresentationConnectionStateChangedCallbacks = - base::CallbackList; + using PresentationConnectionStateChangedCallbacks = base::CallbackList; base::ScopedPtrHashMap< MediaRoute::Id, scoped_ptr> diff --git a/chrome/browser/media/router/media_router_mojo_impl.cc b/chrome/browser/media/router/media_router_mojo_impl.cc index d8fa963..454a140 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.cc +++ b/chrome/browser/media/router/media_router_mojo_impl.cc @@ -710,6 +710,15 @@ void MediaRouterMojoImpl::OnPresentationConnectionStateChanged( route_id, mojo::PresentationConnectionStateFromMojo(state)); } +void MediaRouterMojoImpl::OnPresentationConnectionClosed( + const mojo::String& route_id, + interfaces::MediaRouter::PresentationConnectionCloseReason reason, + const mojo::String& message) { + NotifyPresentationConnectionClose( + route_id, mojo::PresentationConnectionCloseReasonFromMojo(reason), + message); +} + void MediaRouterMojoImpl::DoStartObservingMediaSinks( const MediaSource::Id& source_id) { DVLOG_WITH_INSTANCE(1) << "DoStartObservingMediaSinks: " << source_id; diff --git a/chrome/browser/media/router/media_router_mojo_impl.h b/chrome/browser/media/router/media_router_mojo_impl.h index cce100c..aaca9eb 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.h +++ b/chrome/browser/media/router/media_router_mojo_impl.h @@ -269,6 +269,10 @@ class MediaRouterMojoImpl : public MediaRouterBase, void OnPresentationConnectionStateChanged( const mojo::String& route_id, interfaces::MediaRouter::PresentationConnectionState state) override; + void OnPresentationConnectionClosed( + const mojo::String& route_id, + interfaces::MediaRouter::PresentationConnectionCloseReason reason, + const mojo::String& message) override; // Converts the callback result of calling Mojo CreateRoute()/JoinRoute() // into a local callback. 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 65c3f26..7b585a7 100644 --- a/chrome/browser/media/router/media_router_mojo_impl_unittest.cc +++ b/chrome/browser/media/router/media_router_mojo_impl_unittest.cc @@ -48,6 +48,11 @@ using testing::SaveArg; namespace media_router { +using PresentationConnectionState = + interfaces::MediaRouter::PresentationConnectionState; +using PresentationConnectionCloseReason = + interfaces::MediaRouter::PresentationConnectionCloseReason; + namespace { const char kDescription[] = "description"; @@ -928,9 +933,6 @@ TEST_F(MediaRouterMojoImplTest, PresentationSessionMessagesError) { } TEST_F(MediaRouterMojoImplTest, PresentationConnectionStateChangedCallback) { - using PresentationConnectionState = - interfaces::MediaRouter::PresentationConnectionState; - MediaRoute::Id route_id("route-id"); const std::string kPresentationUrl("http://foo.fakeUrl"); const std::string kPresentationId("pid"); @@ -943,36 +945,53 @@ TEST_F(MediaRouterMojoImplTest, PresentationConnectionStateChangedCallback) { base::Bind(&MockPresentationConnectionStateChangedCallback::Run, base::Unretained(&callback))); - base::RunLoop run_loop; - EXPECT_CALL(callback, Run(content::PRESENTATION_CONNECTION_STATE_CLOSED)) - .WillOnce(InvokeWithoutArgs([&run_loop]() { - run_loop.Quit(); - })); - media_router_proxy_->OnPresentationConnectionStateChanged( - route_id, PresentationConnectionState::CLOSED); - run_loop.Run(); + { + base::RunLoop run_loop; + content::PresentationConnectionStateChangeInfo closed_info( + content::PRESENTATION_CONNECTION_STATE_CLOSED); + closed_info.close_reason = + content::PRESENTATION_CONNECTION_CLOSE_REASON_WENT_AWAY; + closed_info.message = "Foo"; + + EXPECT_CALL(callback, Run(StateChageInfoEquals(closed_info))) + .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); + media_router_proxy_->OnPresentationConnectionClosed( + route_id, PresentationConnectionCloseReason::WENT_AWAY, "Foo"); + run_loop.Run(); + EXPECT_TRUE(Mock::VerifyAndClearExpectations(&callback)); + } - EXPECT_TRUE(Mock::VerifyAndClearExpectations(&callback)); + content::PresentationConnectionStateChangeInfo terminated_info( + content::PRESENTATION_CONNECTION_STATE_TERMINATED); + { + base::RunLoop run_loop; + EXPECT_CALL(callback, Run(StateChageInfoEquals(terminated_info))) + .WillOnce(InvokeWithoutArgs([&run_loop]() { run_loop.Quit(); })); + media_router_proxy_->OnPresentationConnectionStateChanged( + route_id, PresentationConnectionState::TERMINATED); + run_loop.Run(); + + EXPECT_TRUE(Mock::VerifyAndClearExpectations(&callback)); + } +} - base::RunLoop run_loop2; - // Right now we don't keep track of previous state so the callback will be - // invoked with the same state again. - EXPECT_CALL(callback, Run(content::PRESENTATION_CONNECTION_STATE_CLOSED)) - .WillOnce(InvokeWithoutArgs([&run_loop2]() { - run_loop2.Quit(); - })); - media_router_proxy_->OnPresentationConnectionStateChanged( - route_id, PresentationConnectionState::CLOSED); - run_loop2.Run(); +TEST_F(MediaRouterMojoImplTest, + PresentationConnectionStateChangedCallbackRemoved) { + MediaRoute::Id route_id("route-id"); + MockPresentationConnectionStateChangedCallback callback; + scoped_ptr subscription = + router()->AddPresentationConnectionStateChangedCallback( + route_id, + base::Bind(&MockPresentationConnectionStateChangedCallback::Run, + base::Unretained(&callback))); // Callback has been removed, so we don't expect it to be called anymore. subscription.reset(); EXPECT_TRUE(router()->presentation_connection_state_callbacks_.empty()); - EXPECT_CALL(callback, Run(content::PRESENTATION_CONNECTION_STATE_CLOSED)) - .Times(0); + EXPECT_CALL(callback, Run(_)).Times(0); media_router_proxy_->OnPresentationConnectionStateChanged( - route_id, PresentationConnectionState::CLOSED); + route_id, PresentationConnectionState::TERMINATED); ProcessEventLoop(); } diff --git a/chrome/browser/media/router/media_router_type_converters.cc b/chrome/browser/media/router/media_router_type_converters.cc index 82b6062..2c4ebc8 100644 --- a/chrome/browser/media/router/media_router_type_converters.cc +++ b/chrome/browser/media/router/media_router_type_converters.cc @@ -12,6 +12,8 @@ using media_router::interfaces::MediaSinkPtr; using PresentationConnectionState = media_router::interfaces::MediaRouter::PresentationConnectionState; +using PresentationConnectionCloseReason = + media_router::interfaces::MediaRouter::PresentationConnectionCloseReason; using RouteRequestResultCode = media_router::interfaces::RouteRequestResultCode; namespace mojo { @@ -151,6 +153,22 @@ content::PresentationConnectionState PresentationConnectionStateFromMojo( } } +content::PresentationConnectionCloseReason +PresentationConnectionCloseReasonFromMojo( + PresentationConnectionCloseReason reason) { + switch (reason) { + case PresentationConnectionCloseReason::CONNECTION_ERROR: + return content::PRESENTATION_CONNECTION_CLOSE_REASON_CONNECTION_ERROR; + case PresentationConnectionCloseReason::CLOSED: + return content::PRESENTATION_CONNECTION_CLOSE_REASON_CLOSED; + case PresentationConnectionCloseReason::WENT_AWAY: + return content::PRESENTATION_CONNECTION_CLOSE_REASON_WENT_AWAY; + default: + NOTREACHED() << "Unknown PresentationConnectionCloseReason " << reason; + return content::PRESENTATION_CONNECTION_CLOSE_REASON_CONNECTION_ERROR; + } +} + media_router::RouteRequestResult::ResultCode RouteRequestResultCodeFromMojo( RouteRequestResultCode result_code) { switch (result_code) { diff --git a/chrome/browser/media/router/media_router_type_converters.h b/chrome/browser/media/router/media_router_type_converters.h index 05f587a..9cc8f52 100644 --- a/chrome/browser/media/router/media_router_type_converters.h +++ b/chrome/browser/media/router/media_router_type_converters.h @@ -69,6 +69,12 @@ struct TypeConverter { content::PresentationConnectionState PresentationConnectionStateFromMojo( media_router::interfaces::MediaRouter::PresentationConnectionState state); +// PresentationConnectionCloseReason conversion. +content::PresentationConnectionCloseReason +PresentationConnectionCloseReasonFromMojo( + media_router::interfaces::MediaRouter::PresentationConnectionCloseReason + reason); + // RouteRequestResult conversion. media_router::RouteRequestResult::ResultCode RouteRequestResultCodeFromMojo( media_router::interfaces::RouteRequestResultCode result_code); diff --git a/chrome/browser/media/router/mock_media_router.h b/chrome/browser/media/router/mock_media_router.h index b57b76c..53fc584 100644 --- a/chrome/browser/media/router/mock_media_router.h +++ b/chrome/browser/media/router/mock_media_router.h @@ -92,7 +92,8 @@ class MockMediaRouter : public MediaRouter { void(PresentationSessionMessagesObserver* observer)); private: - base::CallbackList + base::CallbackList connection_state_callbacks_; }; diff --git a/chrome/browser/media/router/test_helper.h b/chrome/browser/media/router/test_helper.h index ca87e7a..2406c6a 100644 --- a/chrome/browser/media/router/test_helper.h +++ b/chrome/browser/media/router/test_helper.h @@ -75,6 +75,11 @@ MATCHER_P(EqualsIssue, other, "") { return true; } +MATCHER_P(StateChageInfoEquals, other, "") { + return arg.state == other.state && arg.close_reason == other.close_reason && + arg.message == other.message; +} + class MockIssuesObserver : public IssuesObserver { public: explicit MockIssuesObserver(MediaRouter* router); @@ -176,7 +181,8 @@ class MockPresentationConnectionStateChangedCallback { public: MockPresentationConnectionStateChangedCallback(); ~MockPresentationConnectionStateChangedCallback(); - MOCK_METHOD1(Run, void(content::PresentationConnectionState)); + MOCK_METHOD1(Run, + void(const content::PresentationConnectionStateChangeInfo&)); }; } // namespace media_router diff --git a/content/browser/presentation/presentation_service_impl.cc b/content/browser/presentation/presentation_service_impl.cc index 26947c2..1244b20 100644 --- a/content/browser/presentation/presentation_service_impl.cc +++ b/content/browser/presentation/presentation_service_impl.cc @@ -432,11 +432,18 @@ void PresentationServiceImpl::Terminate(const mojo::String& presentation_url, void PresentationServiceImpl::OnConnectionStateChanged( const PresentationSessionInfo& connection, - PresentationConnectionState state) { + const PresentationConnectionStateChangeInfo& info) { DCHECK(client_.get()); - client_->OnConnectionStateChanged( - presentation::PresentationSessionInfo::From(connection), - PresentationConnectionStateToMojo(state)); + if (info.state == PRESENTATION_CONNECTION_STATE_CLOSED) { + client_->OnConnectionClosed( + presentation::PresentationSessionInfo::From(connection), + PresentationConnectionCloseReasonToMojo(info.close_reason), + info.message); + } else { + client_->OnConnectionStateChanged( + presentation::PresentationSessionInfo::From(connection), + PresentationConnectionStateToMojo(info.state)); + } } bool PresentationServiceImpl::FrameMatches( diff --git a/content/browser/presentation/presentation_service_impl.h b/content/browser/presentation/presentation_service_impl.h index 88a7054..de52653 100644 --- a/content/browser/presentation/presentation_service_impl.h +++ b/content/browser/presentation/presentation_service_impl.h @@ -89,6 +89,8 @@ class CONTENT_EXPORT PresentationServiceImpl MaxPendingJoinSessionRequests); FRIEND_TEST_ALL_PREFIXES(PresentationServiceImplTest, ListenForConnectionStateChange); + FRIEND_TEST_ALL_PREFIXES(PresentationServiceImplTest, + ListenForConnectionClose); // Maximum number of pending JoinSession requests at any given time. static const int kMaxNumQueuedSessionRequests = 10; @@ -237,8 +239,9 @@ class CONTENT_EXPORT PresentationServiceImpl // Invoked by the embedder's PresentationServiceDelegate when a // PresentationConnection's state has changed. - void OnConnectionStateChanged(const PresentationSessionInfo& connection, - PresentationConnectionState state); + void OnConnectionStateChanged( + const PresentationSessionInfo& connection, + const PresentationConnectionStateChangeInfo& info); // Returns true if this object is associated with |render_frame_host|. bool FrameMatches(content::RenderFrameHost* render_frame_host) const; diff --git a/content/browser/presentation/presentation_service_impl_unittest.cc b/content/browser/presentation/presentation_service_impl_unittest.cc index c7d2c96..26bc9f1 100644 --- a/content/browser/presentation/presentation_service_impl_unittest.cc +++ b/content/browser/presentation/presentation_service_impl_unittest.cc @@ -163,6 +163,17 @@ class MockPresentationServiceClient : void(const presentation::PresentationSessionInfo& connection, presentation::PresentationConnectionState new_state)); + void OnConnectionClosed( + presentation::PresentationSessionInfoPtr connection, + presentation::PresentationConnectionCloseReason reason, + const mojo::String& message) override { + OnConnectionClosed(*connection, reason, message); + } + MOCK_METHOD3(OnConnectionClosed, + void(const presentation::PresentationSessionInfo& connection, + presentation::PresentationConnectionCloseReason reason, + const mojo::String& message)); + MOCK_METHOD1(OnScreenAvailabilityNotSupported, void(const mojo::String& url)); void OnSessionMessagesReceived( @@ -472,14 +483,48 @@ TEST_F(PresentationServiceImplTest, ListenForConnectionStateChange) { presentation::PresentationSessionInfo presentation_connection; presentation_connection.url = kPresentationUrl; presentation_connection.id = kPresentationId; - base::RunLoop run_loop; - EXPECT_CALL(mock_client_, - OnConnectionStateChanged( - Equals(presentation_connection), - presentation::PresentationConnectionState::CLOSED)) - .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit)); - state_changed_cb.Run(PRESENTATION_CONNECTION_STATE_CLOSED); - run_loop.Run(); + { + base::RunLoop run_loop; + EXPECT_CALL(mock_client_, + OnConnectionStateChanged( + Equals(presentation_connection), + presentation::PresentationConnectionState::TERMINATED)) + .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit)); + state_changed_cb.Run(PresentationConnectionStateChangeInfo( + PRESENTATION_CONNECTION_STATE_TERMINATED)); + run_loop.Run(); + } +} + +TEST_F(PresentationServiceImplTest, ListenForConnectionClose) { + content::PresentationSessionInfo connection(kPresentationUrl, + kPresentationId); + content::PresentationConnectionStateChangedCallback state_changed_cb; + EXPECT_CALL(mock_delegate_, ListenForConnectionStateChange(_, _, _, _)) + .WillOnce(SaveArg<3>(&state_changed_cb)); + service_impl_->ListenForConnectionStateChange(connection); + + // Trigger connection close. It should be propagated back up to + // |mock_client_|. + presentation::PresentationSessionInfo presentation_connection; + presentation_connection.url = kPresentationUrl; + presentation_connection.id = kPresentationId; + { + base::RunLoop run_loop; + PresentationConnectionStateChangeInfo closed_info( + PRESENTATION_CONNECTION_STATE_CLOSED); + closed_info.close_reason = PRESENTATION_CONNECTION_CLOSE_REASON_WENT_AWAY; + closed_info.message = "Foo"; + + EXPECT_CALL(mock_client_, + OnConnectionClosed( + Equals(presentation_connection), + presentation::PresentationConnectionCloseReason::WENT_AWAY, + mojo::String("Foo"))) + .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit)); + state_changed_cb.Run(closed_info); + run_loop.Run(); + } } TEST_F(PresentationServiceImplTest, SetSameDefaultPresentationUrl) { diff --git a/content/browser/presentation/presentation_type_converters.cc b/content/browser/presentation/presentation_type_converters.cc index 6068950..1f94d09 100644 --- a/content/browser/presentation/presentation_type_converters.cc +++ b/content/browser/presentation/presentation_type_converters.cc @@ -40,5 +40,20 @@ presentation::PresentationConnectionState PresentationConnectionStateToMojo( return presentation::PresentationConnectionState::TERMINATED; } +presentation::PresentationConnectionCloseReason +PresentationConnectionCloseReasonToMojo( + content::PresentationConnectionCloseReason reason) { + switch (reason) { + case content::PRESENTATION_CONNECTION_CLOSE_REASON_CONNECTION_ERROR: + return presentation::PresentationConnectionCloseReason::CONNECTION_ERROR; + case content::PRESENTATION_CONNECTION_CLOSE_REASON_CLOSED: + return presentation::PresentationConnectionCloseReason::CLOSED; + case content::PRESENTATION_CONNECTION_CLOSE_REASON_WENT_AWAY: + return presentation::PresentationConnectionCloseReason::WENT_AWAY; + } + NOTREACHED(); + return presentation::PresentationConnectionCloseReason::CONNECTION_ERROR; +} + } // namespace content diff --git a/content/browser/presentation/presentation_type_converters.h b/content/browser/presentation/presentation_type_converters.h index c707f4f..07a7fee 100644 --- a/content/browser/presentation/presentation_type_converters.h +++ b/content/browser/presentation/presentation_type_converters.h @@ -17,6 +17,9 @@ CONTENT_EXPORT presentation::PresentationErrorType PresentationErrorTypeToMojo( CONTENT_EXPORT presentation::PresentationConnectionState PresentationConnectionStateToMojo(PresentationConnectionState state); +CONTENT_EXPORT presentation::PresentationConnectionCloseReason +PresentationConnectionCloseReasonToMojo( + PresentationConnectionCloseReason reason); } // namespace content namespace mojo { diff --git a/content/common/presentation/presentation_service.mojom b/content/common/presentation/presentation_service.mojom index ae39815..2c06b8b 100644 --- a/content/common/presentation/presentation_service.mojom +++ b/content/common/presentation/presentation_service.mojom @@ -16,6 +16,12 @@ enum PresentationConnectionState { TERMINATED }; +enum PresentationConnectionCloseReason { + CONNECTION_ERROR, + CLOSED, + WENT_AWAY +}; + enum PresentationErrorType { NO_AVAILABLE_SCREENS, SESSION_REQUEST_CANCELLED, @@ -119,6 +125,12 @@ interface PresentationServiceClient { OnConnectionStateChanged(PresentationSessionInfo connection, PresentationConnectionState newState); + // Caled when the state of |connection| started on this frame has changed to + // CLOSED. + OnConnectionClosed(PresentationSessionInfo connection, + PresentationConnectionCloseReason reason, + string message); + // See PresentationService::ListenForSessionMessages. OnSessionMessagesReceived(PresentationSessionInfo sessionInfo, array messages); diff --git a/content/public/browser/presentation_service_delegate.h b/content/public/browser/presentation_service_delegate.h index d3b92be..fdd25c3 100644 --- a/content/public/browser/presentation_service_delegate.h +++ b/content/public/browser/presentation_service_delegate.h @@ -23,8 +23,6 @@ using PresentationSessionStartedCallback = base::Callback; using PresentationSessionErrorCallback = base::Callback; -using PresentationConnectionStateChangedCallback = - base::Callback; // Param #0: a vector of messages that are received. // Param #1: tells the callback handler that it may reuse strings or buffers @@ -32,6 +30,23 @@ using PresentationConnectionStateChangedCallback = using PresentationSessionMessageCallback = base::Callback< void(const ScopedVector&, bool)>; +struct PresentationConnectionStateChangeInfo { + explicit PresentationConnectionStateChangeInfo( + PresentationConnectionState state) + : state(state), + close_reason(PRESENTATION_CONNECTION_CLOSE_REASON_CONNECTION_ERROR) {} + ~PresentationConnectionStateChangeInfo() = default; + + PresentationConnectionState state; + + // |close_reason| and |messsage| are only used for state change to CLOSED. + PresentationConnectionCloseReason close_reason; + std::string message; +}; + +using PresentationConnectionStateChangedCallback = + base::Callback; + // An interface implemented by embedders to handle presentation API calls // forwarded from PresentationServiceImpl. class CONTENT_EXPORT PresentationServiceDelegate { diff --git a/content/public/browser/presentation_session.h b/content/public/browser/presentation_session.h index 336912b..8662f30 100644 --- a/content/public/browser/presentation_session.h +++ b/content/public/browser/presentation_session.h @@ -18,6 +18,14 @@ enum PresentationConnectionState { PRESENTATION_CONNECTION_STATE_TERMINATED }; +// TODO(imcheng): Use WENT_AWAY for 1-UA mode when it is implemented +// (crbug.com/513859). +enum PresentationConnectionCloseReason { + PRESENTATION_CONNECTION_CLOSE_REASON_CONNECTION_ERROR, + PRESENTATION_CONNECTION_CLOSE_REASON_CLOSED, + PRESENTATION_CONNECTION_CLOSE_REASON_WENT_AWAY +}; + // TODO(imcheng): Rename to PresentationConnectionInfo. // Represents a presentation session that has been established via either // browser actions or Presentation API. diff --git a/content/renderer/presentation/presentation_dispatcher.cc b/content/renderer/presentation/presentation_dispatcher.cc index 367f694..c79b6e3 100644 --- a/content/renderer/presentation/presentation_dispatcher.cc +++ b/content/renderer/presentation/presentation_dispatcher.cc @@ -55,6 +55,22 @@ blink::WebPresentationConnectionState GetWebPresentationConnectionStateFromMojo( } } +blink::WebPresentationConnectionCloseReason +GetWebPresentationConnectionCloseReasonFromMojo( + presentation::PresentationConnectionCloseReason mojoConnectionCloseReason) { + switch (mojoConnectionCloseReason) { + case presentation::PresentationConnectionCloseReason::CONNECTION_ERROR: + return blink::WebPresentationConnectionCloseReason::Error; + case presentation::PresentationConnectionCloseReason::CLOSED: + return blink::WebPresentationConnectionCloseReason::Closed; + case presentation::PresentationConnectionCloseReason::WENT_AWAY: + return blink::WebPresentationConnectionCloseReason::WentAway; + default: + NOTREACHED(); + return blink::WebPresentationConnectionCloseReason::Error; + } +} + } // namespace namespace content { @@ -382,6 +398,20 @@ void PresentationDispatcher::OnConnectionStateChanged( GetWebPresentationConnectionStateFromMojo(state)); } +void PresentationDispatcher::OnConnectionClosed( + presentation::PresentationSessionInfoPtr connection, + presentation::PresentationConnectionCloseReason reason, + const mojo::String& message) { + if (controller_) + return; + + DCHECK(!connection.is_null()); + controller_->didCloseConnection( + new PresentationConnectionClient(std::move(connection)), + GetWebPresentationConnectionCloseReasonFromMojo(reason), + blink::WebString::fromUTF8(message)); +} + void PresentationDispatcher::OnSessionMessagesReceived( presentation::PresentationSessionInfoPtr session_info, mojo::Array messages) { diff --git a/content/renderer/presentation/presentation_dispatcher.h b/content/renderer/presentation/presentation_dispatcher.h index ad09549..a97f25a 100644 --- a/content/renderer/presentation/presentation_dispatcher.h +++ b/content/renderer/presentation/presentation_dispatcher.h @@ -102,6 +102,10 @@ class CONTENT_EXPORT PresentationDispatcher void OnConnectionStateChanged( presentation::PresentationSessionInfoPtr connection, presentation::PresentationConnectionState state) override; + void OnConnectionClosed( + presentation::PresentationSessionInfoPtr connection, + presentation::PresentationConnectionCloseReason reason, + const mojo::String& message) override; void OnSessionMessagesReceived( presentation::PresentationSessionInfoPtr session_info, mojo::Array messages) override; diff --git a/extensions/renderer/resources/media_router_bindings.js b/extensions/renderer/resources/media_router_bindings.js index 0ad0ae8..0b0bee0 100644 --- a/extensions/renderer/resources/media_router_bindings.js +++ b/extensions/renderer/resources/media_router_bindings.js @@ -123,6 +123,28 @@ define('media_router_bindings', [ } /** + * Converts presentation connection close reason to Mojo enum value. + * @param {!string} reason + * @return {!mediaRouterMojom.MediaRouter.PresentationConnectionCloseReason} + */ + function presentationConnectionCloseReasonToMojo_(reason) { + var PresentationConnectionCloseReason = + mediaRouterMojom.MediaRouter.PresentationConnectionCloseReason; + switch (state) { + case 'error': + return PresentationConnectionCloseReason.CONNECTION_ERROR; + case 'closed': + return PresentationConnectionCloseReason.CLOSED; + case 'went_away': + return PresentationConnectionCloseReason.WENT_AWAY; + default: + console.error('Unknown presentation connection close reason : ' + + reason); + return PresentationConnectionCloseReason.CONNECTION_ERROR; + } + } + + /** * Parses the given route request Error object and converts it to the * corresponding result code. * @param {!Error} error @@ -351,8 +373,8 @@ define('media_router_bindings', [ /** * Called by the provider manager when the state of a presentation connected * to a route has changed. - * @param {!string} routeId - * @param {!string} state + * @param {string} routeId + * @param {string} state */ MediaRouter.prototype.onPresentationConnectionStateChanged = function(routeId, state) { @@ -361,6 +383,19 @@ define('media_router_bindings', [ }; /** + * Called by the provider manager when the state of a presentation connected + * to a route has closed. + * @param {string} routeId + * @param {string} reason + * @param {string} message + */ + MediaRouter.prototype.onPresentationConnectionClosed = + function(routeId, reason, message) { + this.service_.onPresentationConnectionClosed( + routeId, presentationConnectionCloseReasonToMojo_(state), message); + }; + + /** * Object containing callbacks set by the provider manager. * * @constructor diff --git a/third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp b/third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp index 281582b..5f08a4a 100644 --- a/third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp +++ b/third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp @@ -18,6 +18,7 @@ #include "modules/EventTargetModules.h" #include "modules/presentation/Presentation.h" #include "modules/presentation/PresentationConnectionAvailableEvent.h" +#include "modules/presentation/PresentationConnectionCloseEvent.h" #include "modules/presentation/PresentationController.h" #include "modules/presentation/PresentationRequest.h" #include "public/platform/modules/presentation/WebPresentationConnectionClient.h" @@ -60,6 +61,25 @@ const AtomicString& connectionStateToString(WebPresentationConnectionState state return terminatedValue; } +const AtomicString& connectionCloseReasonToString(WebPresentationConnectionCloseReason reason) +{ + DEFINE_STATIC_LOCAL(const AtomicString, errorValue, ("error", AtomicString::ConstructFromLiteral)); + DEFINE_STATIC_LOCAL(const AtomicString, closedValue, ("closed", AtomicString::ConstructFromLiteral)); + DEFINE_STATIC_LOCAL(const AtomicString, wentAwayValue, ("wentaway", AtomicString::ConstructFromLiteral)); + + switch (reason) { + case WebPresentationConnectionCloseReason::Error: + return errorValue; + case WebPresentationConnectionCloseReason::Closed: + return closedValue; + case WebPresentationConnectionCloseReason::WentAway: + return wentAwayValue; + } + + ASSERT_NOT_REACHED(); + return errorValue; +} + void throwPresentationDisconnectedError(ExceptionState& exceptionState) { exceptionState.throwDOMException(InvalidStateError, "Presentation connection is disconnected."); @@ -361,14 +381,21 @@ void PresentationConnection::didChangeState(WebPresentationConnectionState state dispatchEvent(Event::create(EventTypeNames::terminate)); return; // Closed state is handled in |didClose()|. - // TODO(imcheng): Add didClose() method and provide reason and message. - // (crbug.com/574234) case WebPresentationConnectionState::Closed: return; } ASSERT_NOT_REACHED(); } +void PresentationConnection::didClose(WebPresentationConnectionCloseReason reason, const String& message) +{ + if (m_state == WebPresentationConnectionState::Closed) + return; + + m_state = WebPresentationConnectionState::Closed; + dispatchEvent(PresentationConnectionCloseEvent::create(EventTypeNames::close, connectionCloseReasonToString(reason), message)); +} + void PresentationConnection::didFinishLoadingBlob(PassRefPtr buffer) { ASSERT(!m_messages.isEmpty() && m_messages.first()->type == MessageTypeBlob); diff --git a/third_party/WebKit/Source/modules/presentation/PresentationConnection.h b/third_party/WebKit/Source/modules/presentation/PresentationConnection.h index 600c95f..0bcf82c 100644 --- a/third_party/WebKit/Source/modules/presentation/PresentationConnection.h +++ b/third_party/WebKit/Source/modules/presentation/PresentationConnection.h @@ -70,6 +70,9 @@ public: // Notifies the connection about its state change. void didChangeState(WebPresentationConnectionState); + // Notifies the connection about its state change to 'closed'. + void didClose(WebPresentationConnectionCloseReason, const String& message); + // Notifies the presentation about new message. void didReceiveTextMessage(const String& message); void didReceiveBinaryMessage(const uint8_t* data, size_t length); diff --git a/third_party/WebKit/Source/modules/presentation/PresentationController.cpp b/third_party/WebKit/Source/modules/presentation/PresentationController.cpp index ee134c5..3efd6ce 100644 --- a/third_party/WebKit/Source/modules/presentation/PresentationController.cpp +++ b/third_party/WebKit/Source/modules/presentation/PresentationController.cpp @@ -78,6 +78,16 @@ void PresentationController::didChangeSessionState(WebPresentationConnectionClie connection->didChangeState(state); } +void PresentationController::didCloseConnection(WebPresentationConnectionClient* connectionClient, WebPresentationConnectionCloseReason reason, const WebString& message) +{ + OwnPtr client = adoptPtr(connectionClient); + + PresentationConnection* connection = findConnection(client.get()); + if (!connection) + return; + connection->didClose(reason, message); +} + void PresentationController::didReceiveSessionTextMessage(WebPresentationConnectionClient* connectionClient, const WebString& message) { OwnPtr client = adoptPtr(connectionClient); diff --git a/third_party/WebKit/Source/modules/presentation/PresentationController.h b/third_party/WebKit/Source/modules/presentation/PresentationController.h index f6436f0..8fab614 100644 --- a/third_party/WebKit/Source/modules/presentation/PresentationController.h +++ b/third_party/WebKit/Source/modules/presentation/PresentationController.h @@ -20,6 +20,7 @@ class LocalFrame; class PresentationConnection; class WebPresentationAvailabilityCallback; class WebPresentationConnectionClient; +enum class WebPresentationConnectionCloseReason; enum class WebPresentationConnectionState; // The coordinator between the various page exposed properties and the content @@ -50,6 +51,7 @@ public: // Implementation of WebPresentationController. void didStartDefaultSession(WebPresentationConnectionClient*) override; void didChangeSessionState(WebPresentationConnectionClient*, WebPresentationConnectionState) override; + void didCloseConnection(WebPresentationConnectionClient*, WebPresentationConnectionCloseReason, const WebString& message) override; void didReceiveSessionTextMessage(WebPresentationConnectionClient*, const WebString&) override; void didReceiveSessionBinaryMessage(WebPresentationConnectionClient*, const uint8_t* data, size_t length) override; diff --git a/third_party/WebKit/public/platform/modules/presentation/WebPresentationConnectionClient.h b/third_party/WebKit/public/platform/modules/presentation/WebPresentationConnectionClient.h index 749a0bd..b399118 100644 --- a/third_party/WebKit/public/platform/modules/presentation/WebPresentationConnectionClient.h +++ b/third_party/WebKit/public/platform/modules/presentation/WebPresentationConnectionClient.h @@ -15,6 +15,12 @@ enum class WebPresentationConnectionState { Terminated, }; +enum class WebPresentationConnectionCloseReason { + Error = 0, + Closed, + WentAway +}; + // The implementation the embedder has to provide for the Presentation API to work. class WebPresentationConnectionClient { public: diff --git a/third_party/WebKit/public/platform/modules/presentation/WebPresentationController.h b/third_party/WebKit/public/platform/modules/presentation/WebPresentationController.h index e50221f..3a7bdaf 100644 --- a/third_party/WebKit/public/platform/modules/presentation/WebPresentationController.h +++ b/third_party/WebKit/public/platform/modules/presentation/WebPresentationController.h @@ -11,6 +11,7 @@ namespace blink { class WebPresentationConnectionClient; class WebString; +enum class WebPresentationConnectionCloseReason; enum class WebPresentationConnectionState; // The delegate Blink provides to WebPresentationClient in order to get updates. @@ -25,6 +26,9 @@ public: // Called when the state of a session changes. virtual void didChangeSessionState(WebPresentationConnectionClient*, WebPresentationConnectionState) = 0; + // Called when a connection closes. + virtual void didCloseConnection(WebPresentationConnectionClient*, WebPresentationConnectionCloseReason, const WebString& message) = 0; + // Called when a text message of a session is received. virtual void didReceiveSessionTextMessage(WebPresentationConnectionClient*, const WebString& message) = 0; -- cgit v1.1