diff options
35 files changed, 733 insertions, 681 deletions
diff --git a/chrome/browser/media/android/router/media_router_android.cc b/chrome/browser/media/android/router/media_router_android.cc index 688c7e2..9dafb48 100644 --- a/chrome/browser/media/android/router/media_router_android.cc +++ b/chrome/browser/media/android/router/media_router_android.cc @@ -39,23 +39,15 @@ void MediaRouterAndroid::SendRouteMessage( const SendRouteMessageCallback& callback) { NOTIMPLEMENTED(); } - void MediaRouterAndroid::SendRouteBinaryMessage( const MediaRoute::Id& route_id, scoped_ptr<std::vector<uint8>> data, const SendRouteMessageCallback& callback) { NOTIMPLEMENTED(); } - -void MediaRouterAndroid::ListenForRouteMessages( - const std::vector<MediaRoute::Id>& route_ids, - const PresentationSessionMessageCallback& message_cb) { - NOTIMPLEMENTED(); -} void MediaRouterAndroid::ClearIssue(const Issue::Id& issue_id) { NOTIMPLEMENTED(); } - void MediaRouterAndroid::RegisterMediaSinksObserver( MediaSinksObserver* observer) { NOTIMPLEMENTED(); @@ -78,5 +70,13 @@ void MediaRouterAndroid::RegisterIssuesObserver(IssuesObserver* observer) { void MediaRouterAndroid::UnregisterIssuesObserver(IssuesObserver* observer) { NOTIMPLEMENTED(); } +void MediaRouterAndroid::RegisterPresentationSessionMessagesObserver( + PresentationSessionMessagesObserver* observer) { + NOTIMPLEMENTED(); +} +void MediaRouterAndroid::UnregisterPresentationSessionMessagesObserver( + PresentationSessionMessagesObserver* observer) { + NOTIMPLEMENTED(); +} } // namespace media_router diff --git a/chrome/browser/media/android/router/media_router_android.h b/chrome/browser/media/android/router/media_router_android.h index f8aa6fb..eb14600 100644 --- a/chrome/browser/media/android/router/media_router_android.h +++ b/chrome/browser/media/android/router/media_router_android.h @@ -40,9 +40,6 @@ class MediaRouterAndroid : public MediaRouter { const MediaRoute::Id& route_id, scoped_ptr<std::vector<uint8>> data, const SendRouteMessageCallback& callback) override; - void ListenForRouteMessages( - const std::vector<MediaRoute::Id>& route_ids, - const PresentationSessionMessageCallback& message_cb) override; void ClearIssue(const Issue::Id& issue_id) override; private: @@ -57,6 +54,10 @@ class MediaRouterAndroid : public MediaRouter { void UnregisterMediaRoutesObserver(MediaRoutesObserver* observer) override; void RegisterIssuesObserver(IssuesObserver* observer) override; void UnregisterIssuesObserver(IssuesObserver* observer) override; + void RegisterPresentationSessionMessagesObserver( + PresentationSessionMessagesObserver* observer) override; + void UnregisterPresentationSessionMessagesObserver( + PresentationSessionMessagesObserver* observer) override; DISALLOW_COPY_AND_ASSIGN(MediaRouterAndroid); }; diff --git a/chrome/browser/media/router/create_presentation_session_request.cc b/chrome/browser/media/router/create_presentation_session_request.cc index 4999389..1f1e6e9 100644 --- a/chrome/browser/media/router/create_presentation_session_request.cc +++ b/chrome/browser/media/router/create_presentation_session_request.cc @@ -35,12 +35,12 @@ CreatePresentationSessionRequest::~CreatePresentationSessionRequest() { } void CreatePresentationSessionRequest::MaybeInvokeSuccessCallback( + const std::string& presentation_id, const MediaRoute::Id& route_id) { if (!cb_invoked_) { // Overwrite presentation ID. success_cb_.Run(content::PresentationSessionInfo( - presentation_info_.presentation_url, - GetPresentationIdAndUrl(route_id).first), + presentation_info_.presentation_url, presentation_id), route_id); cb_invoked_ = true; } diff --git a/chrome/browser/media/router/create_presentation_session_request.h b/chrome/browser/media/router/create_presentation_session_request.h index a6e4281..3b076fd 100644 --- a/chrome/browser/media/router/create_presentation_session_request.h +++ b/chrome/browser/media/router/create_presentation_session_request.h @@ -53,7 +53,8 @@ class CreatePresentationSessionRequest { // Invokes |success_cb_| or |error_cb_| with the given arguments. // These functions can only be invoked once per instance. Further invocations // are no-op. - void MaybeInvokeSuccessCallback(const MediaRoute::Id& route_id); + void MaybeInvokeSuccessCallback(const std::string& presentation_id, + const MediaRoute::Id& route_id); void MaybeInvokeErrorCallback(const content::PresentationError& error); private: diff --git a/chrome/browser/media/router/create_presentation_session_request_unittest.cc b/chrome/browser/media/router/create_presentation_session_request_unittest.cc index c198a0b..ba0b6d6c 100644 --- a/chrome/browser/media/router/create_presentation_session_request_unittest.cc +++ b/chrome/browser/media/router/create_presentation_session_request_unittest.cc @@ -86,7 +86,7 @@ TEST_F(CreatePresentationSessionRequestTest, SuccessCallback) { base::Unretained(this), session_info), base::Bind(&CreatePresentationSessionRequestTest::FailOnError, base::Unretained(this))); - context.MaybeInvokeSuccessCallback(kRouteId); + context.MaybeInvokeSuccessCallback(kPresentationId, kRouteId); // No-op since success callback is already invoked. context.MaybeInvokeErrorCallback(content::PresentationError( content::PRESENTATION_ERROR_NO_AVAILABLE_SCREENS, "Error message")); @@ -108,7 +108,7 @@ TEST_F(CreatePresentationSessionRequestTest, ErrorCallback) { base::Unretained(this), error)); context.MaybeInvokeErrorCallback(error); // No-op since error callback is already invoked. - context.MaybeInvokeSuccessCallback(kRouteId); + context.MaybeInvokeSuccessCallback(kPresentationId, kRouteId); EXPECT_TRUE(cb_invoked_); } diff --git a/chrome/browser/media/router/media_route.cc b/chrome/browser/media/router/media_route.cc index 8fab827..b5838d3 100644 --- a/chrome/browser/media/router/media_route.cc +++ b/chrome/browser/media/router/media_route.cc @@ -7,10 +7,6 @@ #include "base/logging.h" #include "chrome/browser/media/router/media_source.h" -namespace { -const char kRouteUrnPrefix[] = "urn:x-org.chromium:media:route:"; -} - namespace media_router { MediaRoute::MediaRoute(const MediaRoute::Id& media_route_id, @@ -33,39 +29,6 @@ bool MediaRoute::Equals(const MediaRoute& other) const { return media_route_id_ == other.media_route_id_; } -// <route-id> = -// urn:x-org.chromium:media:route:<presentation-id>/<sink>/<source> -// <source> = <url>|<capture-source> -// <sink> = <provider-name>-<sink-id> -std::pair<std::string, std::string> GetPresentationIdAndUrl( - const MediaRoute::Id& id) { - if (id.find(kRouteUrnPrefix) != 0) { - LOG(ERROR) << "Invalid media route ID. Expecting prefix " - << kRouteUrnPrefix; - return std::make_pair(std::string(), std::string()); - } - size_t prefix_len = arraysize(kRouteUrnPrefix) - 1; - size_t first_delim = id.find("/"); - if (first_delim == std::string::npos || first_delim == prefix_len) { - LOG(ERROR) << "Invalid media route ID. Expecting presentation ID."; - return std::make_pair(std::string(), std::string()); - } - - std::string presentation_id = id.substr(prefix_len, first_delim - prefix_len); - size_t second_delim = id.find("/", first_delim + 1); - if (second_delim == std::string::npos || second_delim == first_delim + 1) { - LOG(ERROR) << "Invalid media route ID. Expecting sink."; - return std::make_pair(std::string(), std::string()); - } - - if (second_delim == id.size() - 1) { - LOG(ERROR) << "Invalid media route ID. Expecting source."; - return std::make_pair(std::string(), std::string()); - } - std::string source = id.substr(second_delim + 1); - return std::make_pair(presentation_id, source); -} - MediaRouteIdToPresentationSessionMapping:: MediaRouteIdToPresentationSessionMapping() { } diff --git a/chrome/browser/media/router/media_route.h b/chrome/browser/media/router/media_route.h index 7c0fb52f..0ccd097 100644 --- a/chrome/browser/media/router/media_route.h +++ b/chrome/browser/media/router/media_route.h @@ -100,11 +100,6 @@ class MediaRouteIdToPresentationSessionMapping { DISALLOW_COPY_AND_ASSIGN(MediaRouteIdToPresentationSessionMapping); }; -// Return a pair of Presentation ID and URL. If the input route ID is invalid, -// a pair of empty strings is returned. -std::pair<std::string, std::string> GetPresentationIdAndUrl( - const MediaRoute::Id& id); - } // namespace media_router #endif // CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTE_H_ diff --git a/chrome/browser/media/router/media_route_unittest.cc b/chrome/browser/media/router/media_route_unittest.cc index 780b90a..5978029 100644 --- a/chrome/browser/media/router/media_route_unittest.cc +++ b/chrome/browser/media/router/media_route_unittest.cc @@ -44,19 +44,4 @@ TEST(MediaRouteTest, Equals) { EXPECT_FALSE(route1.Equals(route5)); } -TEST(MediaRouteTest, ParseId) { - EXPECT_EQ("1", GetPresentationIdAndUrl(kRouteId1).first); - EXPECT_EQ("http://foo.com", GetPresentationIdAndUrl(kRouteId1).second); - auto invalid = std::make_pair(std::string(), std::string()); - EXPECT_EQ(invalid, GetPresentationIdAndUrl("invalid")); - EXPECT_EQ(invalid, GetPresentationIdAndUrl( - "urn:x-org.chromium:media:route:2")); - EXPECT_EQ(invalid, GetPresentationIdAndUrl( - "urn:x-org.chromium:media:route:2/")); - EXPECT_EQ(invalid, GetPresentationIdAndUrl( - "urn:x-org.chromium:media:route:2/cast-sink2/")); - EXPECT_EQ(invalid, GetPresentationIdAndUrl( - "urn:x-org.chromium:media:route:2//http://foo.com")); -} - } // namespace media_router diff --git a/chrome/browser/media/router/media_router.gypi b/chrome/browser/media/router/media_router.gypi index a991d66..264bcab 100644 --- a/chrome/browser/media/router/media_router.gypi +++ b/chrome/browser/media/router/media_router.gypi @@ -35,6 +35,8 @@ 'presentation_media_sinks_observer.h', 'presentation_service_delegate_impl.cc', 'presentation_service_delegate_impl.h', + 'presentation_session_messages_observer.cc', + 'presentation_session_messages_observer.h', 'presentation_session_state_observer.cc', 'presentation_session_state_observer.h', ], diff --git a/chrome/browser/media/router/media_router.h b/chrome/browser/media/router/media_router.h index 7fd66fa..7405f4f 100644 --- a/chrome/browser/media/router/media_router.h +++ b/chrome/browser/media/router/media_router.h @@ -21,15 +21,25 @@ namespace media_router { class IssuesObserver; class MediaRoutesObserver; class MediaSinksObserver; - -// Type of callback used in |CreateRoute()|. Callback is invoked when the -// route request either succeeded or failed. -// The first argument is the route created. If the route request failed, this -// will be a nullptr. -// The second argument is the error string, which will be nonempty if the route -// request failed. +class PresentationSessionMessagesObserver; + +// Type of callback used in |CreateRoute()| and |JoinRoute()|. 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. using MediaRouteResponseCallback = - base::Callback<void(const MediaRoute*, const std::string&)>; + base::Callback<void(const MediaRoute* route, + const std::string& presentation_id, + const std::string& error)>; // Used in cases where a tab ID is not applicable in CreateRoute/JoinRoute. const int kInvalidTabId = -1; @@ -93,14 +103,6 @@ class MediaRouter : public KeyedService { scoped_ptr<std::vector<uint8>> data, const SendRouteMessageCallback& callback) = 0; - // Gets the next batch of messages from one of the routes in |route_ids|. - // |message_cb|: Invoked with a non-empty list of messages when there are - // messages, an empty list when messaging channel had error. - // It is not invoked until there are messages available or error. - virtual void ListenForRouteMessages( - const std::vector<MediaRoute::Id>& route_ids, - const PresentationSessionMessageCallback& message_cb) = 0; - // Clears the issue with the id |issue_id|. virtual void ClearIssue(const Issue::Id& issue_id) = 0; @@ -108,9 +110,9 @@ class MediaRouter : public KeyedService { friend class IssuesObserver; friend class MediaSinksObserver; friend class MediaRoutesObserver; + friend class PresentationSessionMessagesObserver; - // The following functions are called by IssuesObserver, MediaSinksObserver, - // and MediaRoutesObserver. + // The following functions are called by friend Observer classes above. // Registers |observer| with this MediaRouter. |observer| specifies a media // source and will receive updates with media sinks that are compatible with @@ -130,7 +132,7 @@ class MediaRouter : public KeyedService { // Adds a MediaRoutesObserver to listen for updates on MediaRoutes. // The initial update may happen synchronously. - // MediaRouter does not own |observer|. |RemoveMediaRoutesObserver| should + // MediaRouter does not own |observer|. |UnregisterMediaRoutesObserver| should // be called before |observer| is destroyed. // It is invalid to register the same observer more than once and will result // in undefined behavior. @@ -147,6 +149,20 @@ class MediaRouter : public KeyedService { // Removes the IssuesObserver |observer|. virtual void UnregisterIssuesObserver(IssuesObserver* observer) = 0; + + // Registers |observer| with this MediaRouter. |observer| specifies a media + // route corresponding to a presentation and will receive messages from the + // MediaSink connected to the route. Note that MediaRouter does not own + // |observer|. |observer| should be unregistered before it is destroyed. + // Registering the same observer more than once will result in undefined + // behavior. + virtual void RegisterPresentationSessionMessagesObserver( + PresentationSessionMessagesObserver* observer) = 0; + + // Unregisters a previously registered PresentationSessionMessagesObserver. + // |observer| will stop receiving further updates. + virtual void UnregisterPresentationSessionMessagesObserver( + PresentationSessionMessagesObserver* observer) = 0; }; } // namespace media_router diff --git a/chrome/browser/media/router/media_router.mojom b/chrome/browser/media/router/media_router.mojom index 1a09eb7..b571e8b 100644 --- a/chrome/browser/media/router/media_router.mojom +++ b/chrome/browser/media/router/media_router.mojom @@ -89,8 +89,6 @@ struct RouteMessage { TEXT, BINARY }; - // The route ID of this message. - string route_id; // The type of this message. Type type; // Used when the |type| is TEXT. @@ -163,9 +161,8 @@ interface MediaRouteProvider { ClearIssue(string issue_id); // Called when the MediaRouter is ready to get the next batch of messages - // associated with one of the |route_ids|. - ListenForRouteMessages(array<string> route_ids) - => (array<RouteMessage> messages); + // associated with |route_id|. + ListenForRouteMessages(string route_id) => (array<RouteMessage> messages); }; // Interface for a service which observes state changes across media diff --git a/chrome/browser/media/router/media_router_mojo_impl.cc b/chrome/browser/media/router/media_router_mojo_impl.cc index ace0a47..b597875 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.cc +++ b/chrome/browser/media/router/media_router_mojo_impl.cc @@ -15,6 +15,7 @@ #include "chrome/browser/media/router/media_router_type_converters.h" #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 "extensions/browser/process_manager.h" #define DVLOG_WITH_INSTANCE(level) \ @@ -28,10 +29,12 @@ namespace { // Converts the callback result of calling Mojo CreateRoute()/JoinRoute() // into a local callback. void RouteResponseReceived( + const std::string& presentation_id, const std::vector<MediaRouteResponseCallback>& callbacks, interfaces::MediaRoutePtr media_route, const mojo::String& error_text) { scoped_ptr<MediaRoute> route; + std::string actual_presentation_id; std::string error; if (media_route.is_null()) { // An error occurred. @@ -39,10 +42,11 @@ void RouteResponseReceived( error = !error_text.get().empty() ? error_text.get() : "Unknown error."; } else { route = media_route.To<scoped_ptr<MediaRoute>>(); + actual_presentation_id = presentation_id; } for (const MediaRouteResponseCallback& callback : callbacks) - callback.Run(route.get(), error); + callback.Run(route.get(), actual_presentation_id, error); } // TODO(imcheng): We should handle failure in this case. One way is to invoke @@ -55,25 +59,22 @@ void EventPageWakeComplete(bool success) { scoped_ptr<content::PresentationSessionMessage> ConvertToPresentationSessionMessage(interfaces::RouteMessagePtr input) { DCHECK(!input.is_null()); - const auto& id_and_url = GetPresentationIdAndUrl(input->route_id); - const std::string& presentation_id = id_and_url.first; - const std::string& presentation_url = id_and_url.second; scoped_ptr<content::PresentationSessionMessage> output; switch (input->type) { case interfaces::RouteMessage::Type::TYPE_TEXT: { DCHECK(!input->message.is_null()); DCHECK(input->data.is_null()); - output = content::PresentationSessionMessage::CreateStringMessage( - presentation_url, presentation_id, make_scoped_ptr(new std::string)); - input->message.Swap(output->message.get()); + output.reset(new content::PresentationSessionMessage( + content::PresentationMessageType::TEXT)); + input->message.Swap(&output->message); return output.Pass(); } case interfaces::RouteMessage::Type::TYPE_BINARY: { DCHECK(!input->data.is_null()); DCHECK(input->message.is_null()); - output = content::PresentationSessionMessage::CreateArrayBufferMessage( - presentation_url, presentation_id, - make_scoped_ptr(new std::vector<uint8_t>)); + output.reset(new content::PresentationSessionMessage( + content::PresentationMessageType::ARRAY_BUFFER)); + output->data.reset(new std::vector<uint8_t>); input->data.Swap(output->data.get()); return output.Pass(); } @@ -201,7 +202,7 @@ void MediaRouterMojoImpl::CreateRoute( if (!origin.is_valid()) { DVLOG_WITH_INSTANCE(1) << "Invalid origin: " << origin; for (const MediaRouteResponseCallback& callback : callbacks) - callback.Run(nullptr, "Invalid origin"); + callback.Run(nullptr, "", "Invalid origin"); return; } RunOrDefer(base::Bind( @@ -220,7 +221,7 @@ void MediaRouterMojoImpl::JoinRoute( if (!origin.is_valid()) { DVLOG_WITH_INSTANCE(1) << "Invalid origin: " << origin; for (const MediaRouteResponseCallback& callback : callbacks) - callback.Run(nullptr, "Invalid origin"); + callback.Run(nullptr, "", "Invalid origin"); return; } RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoJoinRoute, @@ -257,14 +258,6 @@ void MediaRouterMojoImpl::SendRouteBinaryMessage( base::Passed(data.Pass()), callback)); } -void MediaRouterMojoImpl::ListenForRouteMessages( - const std::vector<MediaRoute::Id>& route_ids, - const PresentationSessionMessageCallback& message_cb) { - DCHECK(thread_checker_.CalledOnValidThread()); - RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoListenForRouteMessages, - base::Unretained(this), route_ids, message_cb)); -} - void MediaRouterMojoImpl::ClearIssue(const Issue::Id& issue_id) { DCHECK(thread_checker_.CalledOnValidThread()); issue_manager_.ClearIssue(issue_id); @@ -300,7 +293,7 @@ void MediaRouterMojoImpl::UnregisterMediaSinksObserver( MediaSinksObserver* observer) { DCHECK(thread_checker_.CalledOnValidThread()); - const std::string& source_id = observer->source().id(); + const MediaSource::Id& source_id = observer->source().id(); auto* observer_list = sinks_observers_.get(source_id); if (!observer_list || !observer_list->HasObserver(observer)) { return; @@ -350,6 +343,46 @@ void MediaRouterMojoImpl::UnregisterIssuesObserver(IssuesObserver* observer) { issue_manager_.UnregisterObserver(observer); } +void MediaRouterMojoImpl::RegisterPresentationSessionMessagesObserver( + PresentationSessionMessagesObserver* observer) { + DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK(observer); + const MediaRoute::Id& route_id = observer->route_id(); + auto* observer_list = messages_observers_.get(route_id); + if (!observer_list) { + observer_list = new base::ObserverList<PresentationSessionMessagesObserver>; + messages_observers_.add(route_id, make_scoped_ptr(observer_list)); + } else { + DCHECK(!observer_list->HasObserver(observer)); + } + + bool should_listen = !observer_list->might_have_observers(); + observer_list->AddObserver(observer); + if (should_listen) { + RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoListenForRouteMessages, + base::Unretained(this), route_id)); + } +} + +void MediaRouterMojoImpl::UnregisterPresentationSessionMessagesObserver( + PresentationSessionMessagesObserver* observer) { + DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK(observer); + + const MediaRoute::Id& route_id = observer->route_id(); + auto* observer_list = messages_observers_.get(route_id); + if (!observer_list || !observer_list->HasObserver(observer)) + return; + + observer_list->RemoveObserver(observer); + if (!observer_list->might_have_observers()) + messages_observers_.erase(route_id); + // TODO(imcheng): Queue a task to stop listening for messages by asking + // the extension to invoke the oustanding Mojo callback with empty list. We + // don't want the Mojo callback to exist indefinitely on the extension side + // and there is currently no way to cancel the callback from this side. +} + void MediaRouterMojoImpl::DoCreateRoute( const MediaSource::Id& source_id, const MediaSink::Id& sink_id, @@ -362,7 +395,7 @@ void MediaRouterMojoImpl::DoCreateRoute( << ", presentation ID: " << presentation_id; media_route_provider_->CreateRoute( source_id, sink_id, presentation_id, origin, tab_id, - base::Bind(&RouteResponseReceived, callbacks)); + base::Bind(&RouteResponseReceived, presentation_id, callbacks)); } void MediaRouterMojoImpl::DoJoinRoute( @@ -375,7 +408,7 @@ void MediaRouterMojoImpl::DoJoinRoute( << ", presentation ID: " << presentation_id; media_route_provider_->JoinRoute( source_id, presentation_id, origin, tab_id, - base::Bind(&RouteResponseReceived, callbacks)); + base::Bind(&RouteResponseReceived, presentation_id, callbacks)); } void MediaRouterMojoImpl::DoCloseRoute(const MediaRoute::Id& route_id) { @@ -403,25 +436,52 @@ void MediaRouterMojoImpl::DoSendSessionBinaryMessage( } void MediaRouterMojoImpl::DoListenForRouteMessages( - const std::vector<MediaRoute::Id>& route_ids, - const PresentationSessionMessageCallback& message_cb) { + const MediaRoute::Id& route_id) { DVLOG_WITH_INSTANCE(1) << "ListenForRouteMessages"; - media_route_provider_->ListenForRouteMessages( - mojo::Array<mojo::String>::From(route_ids), - base::Bind(&MediaRouterMojoImpl::OnRouteMessageReceived, - base::Unretained(this), message_cb)); + if (!ContainsValue(route_ids_listening_for_messages_, route_id)) { + route_ids_listening_for_messages_.insert(route_id); + media_route_provider_->ListenForRouteMessages( + route_id, base::Bind(&MediaRouterMojoImpl::OnRouteMessagesReceived, + base::Unretained(this), route_id)); + } } -void MediaRouterMojoImpl::OnRouteMessageReceived( - const PresentationSessionMessageCallback& message_cb, +void MediaRouterMojoImpl::OnRouteMessagesReceived( + const MediaRoute::Id& route_id, mojo::Array<interfaces::RouteMessagePtr> messages) { - scoped_ptr<ScopedVector<content::PresentationSessionMessage>> - session_messages(new ScopedVector<content::PresentationSessionMessage>()); - for (size_t i = 0; i < messages.size(); ++i) { - session_messages->push_back( - ConvertToPresentationSessionMessage(messages[i].Pass()).Pass()); + DVLOG(1) << "OnRouteMessagesReceived"; + + // Check if there are any observers remaining. If not, the messages + // can be discarded and we can stop listening for the next batch of messages. + auto* observer_list = messages_observers_.get(route_id); + if (!observer_list) { + route_ids_listening_for_messages_.erase(route_id); + return; + } + + // Empty |messages| means we told the extension that we were no longer + // listening for messages on that route. But since now we have observers + // again, we should keep listening. + if (messages.storage().empty()) { + DVLOG(2) << "Received empty messages for " << route_id; + } else { + ScopedVector<content::PresentationSessionMessage> session_messages; + session_messages.reserve(messages.size()); + for (size_t i = 0; i < messages.size(); ++i) { + session_messages.push_back( + ConvertToPresentationSessionMessage(messages[i].Pass()).Pass()); + } + + // TODO(imcheng): If there is only 1 observer, we should be able to pass + // the messages to avoid additional copies. (crbug.com/517234) + FOR_EACH_OBSERVER(PresentationSessionMessagesObserver, *observer_list, + OnMessagesReceived(session_messages)); } - message_cb.Run(session_messages.Pass()); + + // Listen for more messages. + media_route_provider_->ListenForRouteMessages( + route_id, base::Bind(&MediaRouterMojoImpl::OnRouteMessagesReceived, + base::Unretained(this), route_id)); } void MediaRouterMojoImpl::DoClearIssue(const Issue::Id& issue_id) { diff --git a/chrome/browser/media/router/media_router_mojo_impl.h b/chrome/browser/media/router/media_router_mojo_impl.h index 723163e..32a9821 100644 --- a/chrome/browser/media/router/media_router_mojo_impl.h +++ b/chrome/browser/media/router/media_router_mojo_impl.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTER_MOJO_IMPL_H_ #include <map> +#include <set> #include <string> #include <vector> @@ -77,16 +78,7 @@ class MediaRouterMojoImpl : public MediaRouter, const MediaRoute::Id& route_id, scoped_ptr<std::vector<uint8>> data, const SendRouteMessageCallback& callback) override; - void ListenForRouteMessages( - const std::vector<MediaRoute::Id>& route_ids, - const PresentationSessionMessageCallback& message_cb) override; void ClearIssue(const Issue::Id& issue_id) override; - void RegisterMediaSinksObserver(MediaSinksObserver* observer) override; - void UnregisterMediaSinksObserver(MediaSinksObserver* observer) override; - void RegisterMediaRoutesObserver(MediaRoutesObserver* observer) override; - void UnregisterMediaRoutesObserver(MediaRoutesObserver* observer) override; - void RegisterIssuesObserver(IssuesObserver* observer) override; - void UnregisterIssuesObserver(IssuesObserver* observer) override; const std::string& media_route_provider_extension_id() const { return media_route_provider_extension_id_; @@ -100,6 +92,11 @@ class MediaRouterMojoImpl : public MediaRouter, friend class MediaRouterFactory; friend class MediaRouterMojoTest; + FRIEND_TEST_ALL_PREFIXES(MediaRouterMojoImplTest, + RegisterAndUnregisterMediaSinksObserver); + FRIEND_TEST_ALL_PREFIXES(MediaRouterMojoImplTest, + RegisterAndUnregisterMediaRoutesObserver); + FRIEND_TEST_ALL_PREFIXES(MediaRouterMojoImplTest, HandleIssue); FRIEND_TEST_ALL_PREFIXES(MediaRouterMojoExtensionTest, DeferredBindingAndSuspension); @@ -125,6 +122,18 @@ class MediaRouterMojoImpl : public MediaRouter, // Dispatches the Mojo requests queued in |pending_requests_|. void ExecutePendingRequests(); + // MediaRouter implementation. + void RegisterMediaSinksObserver(MediaSinksObserver* observer) override; + void UnregisterMediaSinksObserver(MediaSinksObserver* observer) override; + void RegisterMediaRoutesObserver(MediaRoutesObserver* observer) override; + void UnregisterMediaRoutesObserver(MediaRoutesObserver* observer) override; + void RegisterIssuesObserver(IssuesObserver* observer) override; + void UnregisterIssuesObserver(IssuesObserver* observer) override; + void RegisterPresentationSessionMessagesObserver( + PresentationSessionMessagesObserver* observer) override; + void UnregisterPresentationSessionMessagesObserver( + PresentationSessionMessagesObserver* observer) override; + // These calls invoke methods in the component extension via Mojo. void DoCreateRoute(const MediaSource::Id& source_id, const MediaSink::Id& sink_id, @@ -143,9 +152,7 @@ class MediaRouterMojoImpl : public MediaRouter, void DoSendSessionBinaryMessage(const MediaRoute::Id& route_id, scoped_ptr<std::vector<uint8>> data, const SendRouteMessageCallback& callback); - void DoListenForRouteMessages( - const std::vector<MediaRoute::Id>& route_ids, - const PresentationSessionMessageCallback& message_cb); + void DoListenForRouteMessages(const MediaRoute::Id& route_id); void DoClearIssue(const Issue::Id& issue_id); void DoStartObservingMediaSinks(const MediaSource::Id& source_id); void DoStopObservingMediaSinks(const MediaSource::Id& source_id); @@ -153,10 +160,10 @@ class MediaRouterMojoImpl : public MediaRouter, void DoStopObservingMediaRoutes(); // Invoked when the next batch of messages arrives. + // |route_id|: ID of route of the messages. // |messages|: A list of messages received. - // |message_cb|: The callback to invoke to pass on the messages received. - void OnRouteMessageReceived( - const PresentationSessionMessageCallback& message_cb, + void OnRouteMessagesReceived( + const MediaRoute::Id& route_id, mojo::Array<interfaces::RouteMessagePtr> messages); // Error handler callback for |binding_| and |media_route_provider_|. @@ -182,6 +189,17 @@ class MediaRouterMojoImpl : public MediaRouter, base::ObserverList<MediaRoutesObserver> routes_observers_; + using PresentationSessionMessagesObserverList = + base::ObserverList<PresentationSessionMessagesObserver>; + base::ScopedPtrHashMap<MediaRoute::Id, + scoped_ptr<PresentationSessionMessagesObserverList>> + messages_observers_; + // IDs of MediaRoutes being listened for messages. Note that this is + // different from |message_observers_| because we might be waiting for + // |OnRouteMessagesReceived()| to be invoked after all observers for that + // route have been removed. + std::set<MediaRoute::Id> route_ids_listening_for_messages_; + IssueManager issue_manager_; // Binds |this| to a Mojo connection stub for interfaces::MediaRouter. 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 d8aa557..5b2f38c 100644 --- a/chrome/browser/media/router/media_router_mojo_impl_unittest.cc +++ b/chrome/browser/media/router/media_router_mojo_impl_unittest.cc @@ -14,6 +14,7 @@ #include "chrome/browser/media/router/media_router_mojo_test.h" #include "chrome/browser/media/router/media_router_type_converters.h" #include "chrome/browser/media/router/mock_media_router.h" +#include "chrome/browser/media/router/presentation_session_messages_observer.h" #include "chrome/browser/media/router/test_helper.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "chrome/test/base/testing_profile.h" @@ -28,6 +29,7 @@ using testing::_; using testing::Eq; using testing::Invoke; using testing::Mock; +using testing::Not; using testing::Pointee; using testing::Return; using testing::ReturnRef; @@ -56,13 +58,11 @@ const uint8 kBinaryMessage[] = {0x01, 0x02, 0x03, 0x04}; bool ArePresentationSessionMessagesEqual( const content::PresentationSessionMessage* expected, const content::PresentationSessionMessage* actual) { - if (expected->presentation_url != actual->presentation_url || - expected->presentation_id != actual->presentation_id || - expected->type != actual->type) { + if (expected->type != actual->type) return false; - } - return expected->is_binary() ? *(expected->data) == *(actual->data) - : *(expected->message) == *(actual->message); + + return expected->is_binary() ? *expected->data == *actual->data + : expected->message == actual->message; } interfaces::IssuePtr CreateMojoIssue(const std::string& title) { @@ -83,8 +83,10 @@ interfaces::IssuePtr CreateMojoIssue(const std::string& title) { class RouteResponseCallbackHandler { public: - MOCK_METHOD2(Invoke, - void(const MediaRoute* route, const std::string& error_text)); + MOCK_METHOD3(Invoke, + void(const MediaRoute* route, + const std::string& presentation_id, + const std::string& error_text)); }; class SendMessageCallbackHandler { @@ -94,25 +96,23 @@ class SendMessageCallbackHandler { class ListenForMessagesCallbackHandler { public: - ListenForMessagesCallbackHandler(scoped_ptr< - ScopedVector<content::PresentationSessionMessage>> expected_messages) + ListenForMessagesCallbackHandler( + ScopedVector<content::PresentationSessionMessage> expected_messages) : expected_messages_(expected_messages.Pass()) {} void Invoke( - scoped_ptr<ScopedVector<content::PresentationSessionMessage>> messages) { + const ScopedVector<content::PresentationSessionMessage>& messages) { InvokeObserver(); - EXPECT_EQ(messages->size(), expected_messages_->size()); - const auto& expected = expected_messages_->get(); - const auto& actual = messages->get(); - for (size_t i = 0; i < expected_messages_->size(); ++i) { - EXPECT_TRUE(ArePresentationSessionMessagesEqual(expected[i], actual[i])); + EXPECT_EQ(messages.size(), expected_messages_.size()); + for (size_t i = 0; i < expected_messages_.size(); ++i) { + EXPECT_TRUE(ArePresentationSessionMessagesEqual(expected_messages_[i], + messages[i])); } } MOCK_METHOD0(InvokeObserver, void()); private: - scoped_ptr<ScopedVector<content::PresentationSessionMessage>> - expected_messages_; + ScopedVector<content::PresentationSessionMessage> expected_messages_; }; template <typename T> @@ -184,7 +184,7 @@ TEST_F(MediaRouterMojoImplTest, CreateRoute) { })); RouteResponseCallbackHandler handler; - EXPECT_CALL(handler, Invoke(Pointee(Equals(expected_route)), "")); + EXPECT_CALL(handler, Invoke(Pointee(Equals(expected_route)), Not(""), "")); std::vector<MediaRouteResponseCallback> route_response_callbacks; route_response_callbacks.push_back(base::Bind( &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); @@ -206,7 +206,7 @@ TEST_F(MediaRouterMojoImplTest, CreateRouteFails) { })); RouteResponseCallbackHandler handler; - EXPECT_CALL(handler, Invoke(nullptr, kError)); + EXPECT_CALL(handler, Invoke(nullptr, "", kError)); std::vector<MediaRouteResponseCallback> route_response_callbacks; route_response_callbacks.push_back(base::Bind( &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); @@ -240,7 +240,7 @@ TEST_F(MediaRouterMojoImplTest, JoinRoute) { })); RouteResponseCallbackHandler handler; - EXPECT_CALL(handler, Invoke(Pointee(Equals(expected_route)), "")); + EXPECT_CALL(handler, Invoke(Pointee(Equals(expected_route)), Not(""), "")); std::vector<MediaRouteResponseCallback> route_response_callbacks; route_response_callbacks.push_back(base::Bind( &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); @@ -261,7 +261,7 @@ TEST_F(MediaRouterMojoImplTest, JoinRouteFails) { })); RouteResponseCallbackHandler handler; - EXPECT_CALL(handler, Invoke(nullptr, kError)); + EXPECT_CALL(handler, Invoke(nullptr, "", kError)); std::vector<MediaRouteResponseCallback> route_response_callbacks; route_response_callbacks.push_back(base::Bind( &RouteResponseCallbackHandler::Invoke, base::Unretained(&handler))); @@ -460,41 +460,62 @@ TEST_F(MediaRouterMojoImplTest, SendRouteBinaryMessage) { ProcessEventLoop(); } -TEST_F(MediaRouterMojoImplTest, ListenForRouteMessages) { +TEST_F(MediaRouterMojoImplTest, PresentationSessionMessagesObserver) { mojo::Array<interfaces::RouteMessagePtr> mojo_messages(2); mojo_messages[0] = interfaces::RouteMessage::New(); - mojo_messages[0]->route_id = "r1"; mojo_messages[0]->type = interfaces::RouteMessage::Type::TYPE_TEXT; mojo_messages[0]->message = "text"; mojo_messages[1] = interfaces::RouteMessage::New(); - mojo_messages[1]->route_id = "r2"; mojo_messages[1]->type = interfaces::RouteMessage::Type::TYPE_BINARY; mojo_messages[1]->data.push_back(1); - scoped_ptr<ScopedVector<content::PresentationSessionMessage>> - expected_messages( - new ScopedVector<content::PresentationSessionMessage>()); - expected_messages->push_back( - content::PresentationSessionMessage::CreateStringMessage( - "", "", make_scoped_ptr(new std::string("text")))); - scoped_ptr<std::vector<uint8_t>> expected_binary_data( - new std::vector<uint8_t>(1, 1)); - expected_messages->push_back( - content::PresentationSessionMessage::CreateArrayBufferMessage( - "", "", expected_binary_data.Pass())); - - EXPECT_CALL(mock_media_route_provider_, ListenForRouteMessagesInteral(_, _)) - .WillOnce(Invoke([&mojo_messages]( - const std::vector<mojo::String>& route_ids, - const interfaces::MediaRouteProvider::ListenForRouteMessagesCallback& - cb) { cb.Run(mojo_messages.Pass()); })); + ScopedVector<content::PresentationSessionMessage> expected_messages; + scoped_ptr<content::PresentationSessionMessage> message; + message.reset(new content::PresentationSessionMessage( + content::PresentationMessageType::TEXT)); + message->message = "text"; + expected_messages.push_back(message.Pass()); + + message.reset(new content::PresentationSessionMessage( + content::PresentationMessageType::ARRAY_BUFFER)); + message->data.reset(new std::vector<uint8_t>(1, 1)); + expected_messages.push_back(message.Pass()); + + MediaRoute::Id expected_route_id("foo"); + interfaces::MediaRouteProvider::ListenForRouteMessagesCallback mojo_callback; + EXPECT_CALL(mock_media_route_provider_, + ListenForRouteMessages(Eq(expected_route_id), _)) + .WillOnce(SaveArg<1>(&mojo_callback)); ListenForMessagesCallbackHandler handler(expected_messages.Pass()); EXPECT_CALL(handler, InvokeObserver()); - std::vector<MediaRoute::Id> route_ids; - router()->ListenForRouteMessages( - route_ids, base::Bind(&ListenForMessagesCallbackHandler::Invoke, - base::Unretained(&handler))); + // Creating PresentationSessionMessagesObserver will register itself to the + // MediaRouter, which in turn will start listening for route messages. + scoped_ptr<PresentationSessionMessagesObserver> observer( + new PresentationSessionMessagesObserver( + base::Bind(&ListenForMessagesCallbackHandler::Invoke, + base::Unretained(&handler)), + expected_route_id, router())); + ProcessEventLoop(); + + // Simulate messages by invoking the saved mojo callback. + // We expect one more ListenForRouteMessages call since |observer| was + // still registered when the first set of messages arrived. + mojo_callback.Run(mojo_messages.Pass()); + interfaces::MediaRouteProvider::ListenForRouteMessagesCallback + mojo_callback_2; + EXPECT_CALL(mock_media_route_provider_, ListenForRouteMessages(_, _)) + .WillOnce(SaveArg<1>(&mojo_callback_2)); + ProcessEventLoop(); + + // Stop listening for messages. In particular, MediaRouterMojoImpl will not + // call ListenForRouteMessages again when it sees there are no more observers. + mojo::Array<interfaces::RouteMessagePtr> mojo_messages_2(1); + mojo_messages_2[0] = interfaces::RouteMessage::New(); + mojo_messages_2[0]->type = interfaces::RouteMessage::Type::TYPE_TEXT; + mojo_messages_2[0]->message = "foo"; + observer.reset(); + mojo_callback_2.Run(mojo_messages_2.Pass()); ProcessEventLoop(); } diff --git a/chrome/browser/media/router/mock_media_router.h b/chrome/browser/media/router/mock_media_router.h index 3ee7e6b7..8de685c 100644 --- a/chrome/browser/media/router/mock_media_router.h +++ b/chrome/browser/media/router/mock_media_router.h @@ -50,13 +50,9 @@ class MockMediaRouter : public MediaRouter { void(const MediaRoute::Id& route_id, std::vector<uint8>* data, const SendRouteMessageCallback& callback)); - MOCK_METHOD2(ListenForRouteMessages, - void(const std::vector<MediaRoute::Id>& route_ids, - const PresentationSessionMessageCallback& message_cb)); MOCK_METHOD1(ClearIssue, void(const Issue::Id& issue_id)); MOCK_METHOD1(RegisterIssuesObserver, void(IssuesObserver* observer)); MOCK_METHOD1(UnregisterIssuesObserver, void(IssuesObserver* observer)); - MOCK_METHOD1(RegisterMediaSinksObserver, void(MediaSinksObserver* observer)); MOCK_METHOD1(UnregisterMediaSinksObserver, void(MediaSinksObserver* observer)); @@ -64,6 +60,10 @@ class MockMediaRouter : public MediaRouter { void(MediaRoutesObserver* observer)); MOCK_METHOD1(UnregisterMediaRoutesObserver, void(MediaRoutesObserver* observer)); + MOCK_METHOD1(RegisterPresentationSessionMessagesObserver, + void(PresentationSessionMessagesObserver* observer)); + MOCK_METHOD1(UnregisterPresentationSessionMessagesObserver, + void(PresentationSessionMessagesObserver* observer)); }; } // namespace media_router diff --git a/chrome/browser/media/router/presentation_service_delegate_impl.cc b/chrome/browser/media/router/presentation_service_delegate_impl.cc index f49c788..16e7e5aa 100644 --- a/chrome/browser/media/router/presentation_service_delegate_impl.cc +++ b/chrome/browser/media/router/presentation_service_delegate_impl.cc @@ -19,6 +19,7 @@ #include "chrome/browser/media/router/media_sink.h" #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/presentation_session_state_observer.h" #include "chrome/browser/sessions/session_tab_helper.h" #include "content/public/browser/presentation_screen_availability_listener.h" @@ -40,6 +41,7 @@ using PresentationSessionErrorCallback = content::PresentationServiceDelegate::PresentationSessionErrorCallback; using PresentationSessionSuccessCallback = content::PresentationServiceDelegate::PresentationSessionSuccessCallback; + using RenderFrameHostId = std::pair<int, int>; // Returns the unique identifier for the supplied RenderFrameHost. @@ -81,6 +83,9 @@ class PresentationFrame { std::string GetDefaultPresentationId() const; void ListenForSessionStateChange( const content::SessionStateChangedCallback& state_changed_cb); + void ListenForSessionMessages( + const content::PresentationSessionInfo& session, + const content::PresentationSessionMessageCallback& message_cb); void Reset(); const MediaRoute::Id GetRouteId(const std::string& presentation_id) const; @@ -110,6 +115,7 @@ class PresentationFrame { std::string default_presentation_url_; scoped_ptr<PresentationMediaSinksObserver> sinks_observer_; scoped_ptr<PresentationSessionStateObserver> session_state_observer_; + ScopedVector<PresentationSessionMessagesObserver> session_messages_observers_; // References to the owning WebContents, and the corresponding MediaRouter. const content::WebContents* web_contents_; @@ -202,6 +208,7 @@ void PresentationFrame::Reset() { default_presentation_url_.clear(); if (session_state_observer_) session_state_observer_->Reset(); + session_messages_observers_.clear(); } void PresentationFrame::ListenForSessionStateChange( @@ -211,6 +218,20 @@ void PresentationFrame::ListenForSessionStateChange( state_changed_cb, &route_id_to_presentation_, router_)); } +void PresentationFrame::ListenForSessionMessages( + const content::PresentationSessionInfo& session, + const content::PresentationSessionMessageCallback& message_cb) { + auto it = presentation_id_to_route_id_.find(session.presentation_id); + if (it == presentation_id_to_route_id_.end()) { + DVLOG(2) << "ListenForSessionMessages: no route for " + << session.presentation_id; + return; + } + + session_messages_observers_.push_back( + new PresentationSessionMessagesObserver(message_cb, it->second, router_)); +} + MediaSource PresentationFrame::GetMediaSourceFromListener( content::PresentationScreenAvailabilityListener* listener) const { // If the default presentation URL is empty then fall back to tab mirroring. @@ -239,6 +260,10 @@ class PresentationFrameManager { void ListenForSessionStateChange( const RenderFrameHostId& render_frame_host_id, const content::SessionStateChangedCallback& state_changed_cb); + void ListenForSessionMessages( + const RenderFrameHostId& render_frame_host_id, + const content::PresentationSessionInfo& session, + const content::PresentationSessionMessageCallback& message_cb); void AddDelegateObserver(const RenderFrameHostId& render_frame_host_id, DelegateObserver* observer); void RemoveDelegateObserver(const RenderFrameHostId& render_frame_host_id); @@ -363,6 +388,21 @@ void PresentationFrameManager::ListenForSessionStateChange( presentation_frame->ListenForSessionStateChange(state_changed_cb); } +void PresentationFrameManager::ListenForSessionMessages( + const RenderFrameHostId& render_frame_host_id, + const content::PresentationSessionInfo& session, + const content::PresentationSessionMessageCallback& message_cb) { + PresentationFrame* presentation_frame = + presentation_frames_.get(render_frame_host_id); + if (!presentation_frame) { + DVLOG(2) << "ListenForSessionMessages: PresentationFrame does not exist " + << "for: (" << render_frame_host_id.first << ", " + << render_frame_host_id.second << ")"; + return; + } + presentation_frame->ListenForSessionMessages(session, message_cb); +} + void PresentationFrameManager::AddDelegateObserver( const RenderFrameHostId& render_frame_host_id, DelegateObserver* observer) { @@ -513,6 +553,7 @@ void PresentationServiceDelegateImpl::OnJoinRouteResponse( const PresentationSessionSuccessCallback& success_cb, const PresentationSessionErrorCallback& error_cb, const MediaRoute* route, + const std::string& presentation_id, const std::string& error_text) { if (!route) { error_cb.Run(content::PresentationError( @@ -522,6 +563,7 @@ void PresentationServiceDelegateImpl::OnJoinRouteResponse( << "route_id: " << route->media_route_id() << ", presentation URL: " << session.presentation_url << ", presentation ID: " << session.presentation_id; + DCHECK_EQ(session.presentation_id, presentation_id); frame_manager_->OnPresentationSessionStarted( RenderFrameHostId(render_process_id, render_frame_id), false, session, route->media_route_id()); @@ -614,39 +656,33 @@ void PresentationServiceDelegateImpl::CloseSession( void PresentationServiceDelegateImpl::ListenForSessionMessages( int render_process_id, int render_frame_id, - const PresentationSessionMessageCallback& message_cb) { - const std::vector<MediaRoute::Id>& route_ids = frame_manager_->GetRouteIds( - RenderFrameHostId(render_process_id, render_frame_id)); - if (route_ids.empty()) { - DVLOG(1) << "No media routes found"; - message_cb.Run( - scoped_ptr<ScopedVector<content::PresentationSessionMessage>>()); - return; - } - - router_->ListenForRouteMessages(route_ids, message_cb); + const content::PresentationSessionInfo& session, + const content::PresentationSessionMessageCallback& message_cb) { + frame_manager_->ListenForSessionMessages( + RenderFrameHostId(render_process_id, render_frame_id), session, + message_cb); } void PresentationServiceDelegateImpl::SendMessage( int render_process_id, int render_frame_id, - scoped_ptr<content::PresentationSessionMessage> message_request, + const content::PresentationSessionInfo& session, + scoped_ptr<content::PresentationSessionMessage> message, const SendMessageCallback& send_message_cb) { const MediaRoute::Id& route_id = frame_manager_->GetRouteId( RenderFrameHostId(render_process_id, render_frame_id), - message_request->presentation_id); + session.presentation_id); if (route_id.empty()) { - DVLOG(1) << "No active route for " << message_request->presentation_id; + DVLOG(1) << "No active route for " << session.presentation_id; send_message_cb.Run(false); return; } - if (message_request->is_binary()) { - router_->SendRouteBinaryMessage(route_id, message_request->data.Pass(), + if (message->is_binary()) { + router_->SendRouteBinaryMessage(route_id, message->data.Pass(), send_message_cb); } else { - router_->SendRouteMessage(route_id, *(message_request->message), - send_message_cb); + router_->SendRouteMessage(route_id, message->message, send_message_cb); } } @@ -660,6 +696,7 @@ void PresentationServiceDelegateImpl::ListenForSessionStateChange( void PresentationServiceDelegateImpl::OnRouteResponse( const MediaRoute* route, + const std::string& presentation_id, const std::string& error) { if (!route) return; @@ -671,8 +708,6 @@ void PresentationServiceDelegateImpl::OnRouteResponse( if (!main_frame) return; RenderFrameHostId render_frame_host_id(GetRenderFrameHostId(main_frame)); - std::string presentation_id = - GetPresentationIdAndUrl(route->media_route_id()).first; frame_manager_->OnPresentationSessionStarted( render_frame_host_id, true, content::PresentationSessionInfo(PresentationUrlFromMediaSource(source), diff --git a/chrome/browser/media/router/presentation_service_delegate_impl.h b/chrome/browser/media/router/presentation_service_delegate_impl.h index 2085fce..c26adbf 100644 --- a/chrome/browser/media/router/presentation_service_delegate_impl.h +++ b/chrome/browser/media/router/presentation_service_delegate_impl.h @@ -91,12 +91,13 @@ class PresentationServiceDelegateImpl void ListenForSessionMessages( int render_process_id, int render_frame_id, - const PresentationSessionMessageCallback& message_cb) override; - void SendMessage( - int render_process_id, - int render_frame_id, - scoped_ptr<content::PresentationSessionMessage> message_request, - const SendMessageCallback& send_message_cb) override; + const content::PresentationSessionInfo& session, + const content::PresentationSessionMessageCallback& message_cb) override; + void SendMessage(int render_process_id, + int render_frame_id, + const content::PresentationSessionInfo& session, + scoped_ptr<content::PresentationSessionMessage> message, + const SendMessageCallback& send_message_cb) override; void ListenForSessionStateChange( int render_process_id, int render_frame_id, @@ -106,7 +107,9 @@ class PresentationServiceDelegateImpl // outside of a Presentation API request. This could be due to // browser action (e.g., browser initiated media router dialog) or // a media route provider (e.g., autojoin). - void OnRouteResponse(const MediaRoute* route, const std::string& error); + void OnRouteResponse(const MediaRoute* route, + const std::string& presentation_id, + const std::string& error); // Returns the default MediaSource for this tab if there is one. // Returns an empty MediaSource otherwise. @@ -160,6 +163,7 @@ class PresentationServiceDelegateImpl const PresentationSessionSuccessCallback& success_cb, const PresentationSessionErrorCallback& error_cb, const MediaRoute* route, + const std::string& presentation_id, const std::string& error_text); void OnStartSessionSucceeded( diff --git a/chrome/browser/media/router/presentation_session_messages_observer.cc b/chrome/browser/media/router/presentation_session_messages_observer.cc new file mode 100644 index 0000000..c20c1ae --- /dev/null +++ b/chrome/browser/media/router/presentation_session_messages_observer.cc @@ -0,0 +1,33 @@ +// Copyright 2015 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/presentation_session_messages_observer.h" + +#include "chrome/browser/media/router/media_router.h" + +namespace media_router { + +PresentationSessionMessagesObserver::PresentationSessionMessagesObserver( + const content::PresentationSessionMessageCallback& message_cb, + const MediaRoute::Id& route_id, + MediaRouter* router) + : message_cb_(message_cb), route_id_(route_id), router_(router) { + DCHECK(!message_cb_.is_null()); + DCHECK(!route_id_.empty()); + DCHECK(router_); + router_->RegisterPresentationSessionMessagesObserver(this); +} + +PresentationSessionMessagesObserver::~PresentationSessionMessagesObserver() { + router_->UnregisterPresentationSessionMessagesObserver(this); +} + +void PresentationSessionMessagesObserver::OnMessagesReceived( + const ScopedVector<content::PresentationSessionMessage>& messages) { + DVLOG(2) << __FUNCTION__ << ", number of messages : " << messages.size(); + DCHECK(!messages.empty()); + message_cb_.Run(messages); +} + +} // namespace media_router diff --git a/chrome/browser/media/router/presentation_session_messages_observer.h b/chrome/browser/media/router/presentation_session_messages_observer.h new file mode 100644 index 0000000..c345358 --- /dev/null +++ b/chrome/browser/media/router/presentation_session_messages_observer.h @@ -0,0 +1,50 @@ +// Copyright 2015 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_PRESENTATION_SESSION_MESSAGES_OBSERVER_H_ +#define CHROME_BROWSER_MEDIA_ROUTER_PRESENTATION_SESSION_MESSAGES_OBSERVER_H_ + +#include "base/macros.h" +#include "base/memory/scoped_vector.h" +#include "chrome/browser/media/router/media_route.h" +#include "content/public/browser/presentation_service_delegate.h" +#include "content/public/browser/presentation_session.h" + +namespace media_router { + +class MediaRouter; + +// Observes messages originating from the MediaSink connected to a MediaRoute +// that represents a presentation. +// Messages are received from MediaRouter via |OnMessagesReceived|, where +// |message_cb_| will be invoked. +class PresentationSessionMessagesObserver { + public: + // |message_cb|: The callback to invoke whenever messages are received. + // |route_id|: ID of MediaRoute to listen for messages. + PresentationSessionMessagesObserver( + const content::PresentationSessionMessageCallback& message_cb, + const MediaRoute::Id& route_id, + MediaRouter* router); + ~PresentationSessionMessagesObserver(); + + // Invoked by |router_| whenever messages are received. Invokes |message_cb_| + // with |messages|. + // |messages| is guaranteed to be non-empty. + void OnMessagesReceived( + const ScopedVector<content::PresentationSessionMessage>& messages); + + const MediaRoute::Id& route_id() const { return route_id_; } + + private: + content::PresentationSessionMessageCallback message_cb_; + const MediaRoute::Id route_id_; + MediaRouter* const router_; + + DISALLOW_COPY_AND_ASSIGN(PresentationSessionMessagesObserver); +}; + +} // namespace media_router + +#endif // CHROME_BROWSER_MEDIA_ROUTER_PRESENTATION_SESSION_MESSAGES_OBSERVER_H_ diff --git a/chrome/browser/media/router/test_helper.h b/chrome/browser/media/router/test_helper.h index 73e0038..c58b7b9 100644 --- a/chrome/browser/media/router/test_helper.h +++ b/chrome/browser/media/router/test_helper.h @@ -114,12 +114,8 @@ class MockMediaRouteProvider : public interfaces::MediaRouteProvider { void(const mojo::String& media_route_id, const std::vector<uint8>& data, const SendRouteMessageCallback& callback)); - void ListenForRouteMessages(mojo::Array<mojo::String> route_ids, - const ListenForRouteMessagesCallback& callback) { - ListenForRouteMessagesInteral(route_ids.storage(), callback); - } - MOCK_METHOD2(ListenForRouteMessagesInteral, - void(const std::vector<mojo::String>& route_ids, + MOCK_METHOD2(ListenForRouteMessages, + void(const mojo::String& route_id, const ListenForRouteMessagesCallback& callback)); MOCK_METHOD1(ClearIssue, void(const mojo::String& issue_id)); MOCK_METHOD0(StartObservingMediaRoutes, void()); 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 bfc2865..706b63d 100644 --- a/chrome/browser/ui/webui/media_router/media_router_ui.cc +++ b/chrome/browser/ui/webui/media_router/media_router_ui.cc @@ -38,13 +38,15 @@ namespace { void HandleRouteResponseForPresentationApi( scoped_ptr<CreatePresentationSessionRequest> presentation_request, const MediaRoute* route, + const std::string& presentation_id, const std::string& error) { DCHECK(presentation_request); if (!route) { presentation_request->MaybeInvokeErrorCallback( content::PresentationError(content::PRESENTATION_ERROR_UNKNOWN, error)); } else { - presentation_request->MaybeInvokeSuccessCallback(route->media_route_id()); + presentation_request->MaybeInvokeSuccessCallback(presentation_id, + route->media_route_id()); } } @@ -284,6 +286,7 @@ void MediaRouterUI::OnRoutesUpdated(const std::vector<MediaRoute>& routes) { } void MediaRouterUI::OnRouteResponseReceived(const MediaRoute* route, + const std::string& presentation_id, const std::string& error) { DVLOG(1) << "OnRouteResponseReceived"; // TODO(imcheng): Display error in UI. (crbug.com/490372) 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 bb8c5b6..d1da21f 100644 --- a/chrome/browser/ui/webui/media_router/media_router_ui.h +++ b/chrome/browser/ui/webui/media_router/media_router_ui.h @@ -146,6 +146,7 @@ class MediaRouterUI // Callback passed to MediaRouter to receive response to route creation // requests. void OnRouteResponseReceived(const MediaRoute* route, + const std::string& presentation_id, const std::string& error); bool DoCreateRoute(const MediaSink::Id& sink_id, MediaCastMode cast_mode); diff --git a/chrome/test/media_router/media_router_e2e_browsertest.cc b/chrome/test/media_router/media_router_e2e_browsertest.cc index 93ee592..e4347ff 100644 --- a/chrome/test/media_router/media_router_e2e_browsertest.cc +++ b/chrome/test/media_router/media_router_e2e_browsertest.cc @@ -64,6 +64,7 @@ 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(); diff --git a/chrome/test/media_router/media_router_e2e_browsertest.h b/chrome/test/media_router/media_router_e2e_browsertest.h index 385fd30..17affe1 100644 --- a/chrome/test/media_router/media_router_e2e_browsertest.h +++ b/chrome/test/media_router/media_router_e2e_browsertest.h @@ -33,6 +33,7 @@ class MediaRouterE2EBrowserTest : public MediaRouterBaseBrowserTest { // 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); // Initializes |observer_| to listen for sinks compatible with |source|, diff --git a/content/browser/presentation/presentation_service_impl.cc b/content/browser/presentation/presentation_service_impl.cc index a73cdc0..dfba7ee 100644 --- a/content/browser/presentation/presentation_service_impl.cc +++ b/content/browser/presentation/presentation_service_impl.cc @@ -29,68 +29,61 @@ int GetNextRequestSessionId() { return ++next_request_session_id; } -// The return value takes ownership of the contents of |input|. presentation::SessionMessagePtr ToMojoSessionMessage( - content::PresentationSessionMessage* input) { + const content::PresentationSessionMessage& input) { presentation::SessionMessagePtr output(presentation::SessionMessage::New()); - output->presentation_url.Swap(&input->presentation_url); - output->presentation_id.Swap(&input->presentation_id); - if (input->is_binary()) { + if (input.is_binary()) { // binary data output->type = presentation::PresentationMessageType:: PRESENTATION_MESSAGE_TYPE_ARRAY_BUFFER; - output->data.Swap(input->data.get()); + output->data = mojo::Array<uint8_t>::From(*input.data); } else { // string message output->type = presentation::PresentationMessageType::PRESENTATION_MESSAGE_TYPE_TEXT; - output->message.Swap(input->message.get()); + output->message = input.message; } return output.Pass(); } -scoped_ptr<content::PresentationSessionMessage> GetPresentationSessionMessage( +scoped_ptr<PresentationSessionMessage> GetPresentationSessionMessage( presentation::SessionMessagePtr input) { DCHECK(!input.is_null()); scoped_ptr<content::PresentationSessionMessage> output; switch (input->type) { - case presentation::PresentationMessageType:: - PRESENTATION_MESSAGE_TYPE_TEXT: { + case presentation::PRESENTATION_MESSAGE_TYPE_TEXT: { DCHECK(!input->message.is_null()); DCHECK(input->data.is_null()); // Return null PresentationSessionMessage if size exceeds. if (input->message.size() > content::kMaxPresentationSessionMessageSize) return output.Pass(); - output = content::PresentationSessionMessage::CreateStringMessage( - input->presentation_url, input->presentation_id, - make_scoped_ptr(new std::string)); - input->message.Swap(output->message.get()); + output.reset( + new PresentationSessionMessage(PresentationMessageType::TEXT)); + input->message.Swap(&output->message); return output.Pass(); } - case presentation::PresentationMessageType:: - PRESENTATION_MESSAGE_TYPE_ARRAY_BUFFER: { + case presentation::PRESENTATION_MESSAGE_TYPE_ARRAY_BUFFER: { DCHECK(!input->data.is_null()); DCHECK(input->message.is_null()); if (input->data.size() > content::kMaxPresentationSessionMessageSize) return output.Pass(); - output = content::PresentationSessionMessage::CreateArrayBufferMessage( - input->presentation_url, input->presentation_id, - make_scoped_ptr(new std::vector<uint8_t>)); + output.reset(new PresentationSessionMessage( + PresentationMessageType::ARRAY_BUFFER)); + output->data.reset(new std::vector<uint8_t>); input->data.Swap(output->data.get()); return output.Pass(); } - case presentation::PresentationMessageType:: - PRESENTATION_MESSAGE_TYPE_BLOB: { + case presentation::PRESENTATION_MESSAGE_TYPE_BLOB: { DCHECK(!input->data.is_null()); DCHECK(input->message.is_null()); if (input->data.size() > content::kMaxPresentationSessionMessageSize) return output.Pass(); - output = content::PresentationSessionMessage::CreateBlobMessage( - input->presentation_url, input->presentation_id, - make_scoped_ptr(new std::vector<uint8_t>)); + output.reset( + new PresentationSessionMessage(PresentationMessageType::BLOB)); + output->data.reset(new std::vector<uint8_t>); input->data.Swap(output->data.get()); return output.Pass(); } @@ -374,8 +367,8 @@ void PresentationServiceImpl::SetDefaultPresentationURL( default_presentation_url_ = default_presentation_url; } - void PresentationServiceImpl::SendSessionMessage( + presentation::PresentationSessionInfoPtr session, presentation::SessionMessagePtr session_message, const SendMessageMojoCallback& callback) { DVLOG(2) << "SendSessionMessage"; @@ -389,8 +382,8 @@ void PresentationServiceImpl::SendSessionMessage( send_message_callback_.reset(new SendMessageMojoCallback(callback)); delegate_->SendMessage( - render_process_id_, - render_frame_id_, + render_process_id_, render_frame_id_, + session.To<PresentationSessionInfo>(), GetPresentationSessionMessage(session_message.Pass()), base::Bind(&PresentationServiceImpl::OnSendMessageCallback, weak_factory_.GetWeakPtr())); @@ -443,43 +436,31 @@ bool PresentationServiceImpl::FrameMatches( } void PresentationServiceImpl::ListenForSessionMessages( - const SessionMessagesCallback& callback) { + presentation::PresentationSessionInfoPtr session) { DVLOG(2) << "ListenForSessionMessages"; - if (!delegate_) { - callback.Run(mojo::Array<presentation::SessionMessagePtr>()); + if (!delegate_) return; - } - // Crash early if renderer is misbehaving. - CHECK(!on_session_messages_callback_.get()); - - on_session_messages_callback_.reset(new SessionMessagesCallback(callback)); + PresentationSessionInfo session_info(session.To<PresentationSessionInfo>()); delegate_->ListenForSessionMessages( - render_process_id_, render_frame_id_, + render_process_id_, render_frame_id_, session_info, base::Bind(&PresentationServiceImpl::OnSessionMessages, - weak_factory_.GetWeakPtr())); + weak_factory_.GetWeakPtr(), session_info)); } void PresentationServiceImpl::OnSessionMessages( - scoped_ptr<ScopedVector<PresentationSessionMessage>> messages) { - if (!on_session_messages_callback_.get()) { - // The Reset method of this class was invoked. - return; - } - - if (!messages.get() || messages->empty()) { - // Error handling. Session is either closed or in error state. - on_session_messages_callback_->Run( - mojo::Array<presentation::SessionMessagePtr>()); - } else { - mojo::Array<presentation::SessionMessagePtr> mojoMessages(messages->size()); - for (size_t i = 0; i < messages->size(); ++i) { - mojoMessages[i] = ToMojoSessionMessage((*messages)[i]); - } - on_session_messages_callback_->Run(mojoMessages.Pass()); - } - - on_session_messages_callback_.reset(); + const PresentationSessionInfo& session, + const ScopedVector<PresentationSessionMessage>& messages) { + DCHECK(client_); + + DVLOG(2) << "OnSessionMessages"; + mojo::Array<presentation::SessionMessagePtr> mojoMessages(messages.size()); + for (size_t i = 0; i < messages.size(); ++i) + mojoMessages[i] = ToMojoSessionMessage(*messages[i]); + + client_->OnSessionMessagesReceived( + presentation::PresentationSessionInfo::From(session), + mojoMessages.Pass()); } void PresentationServiceImpl::DidNavigateAnyFrame( diff --git a/content/browser/presentation/presentation_service_impl.h b/content/browser/presentation/presentation_service_impl.h index fee2d73..fa7928c 100644 --- a/content/browser/presentation/presentation_service_impl.h +++ b/content/browser/presentation/presentation_service_impl.h @@ -182,15 +182,15 @@ class CONTENT_EXPORT PresentationServiceImpl const mojo::String& presentation_url, const mojo::String& presentation_id, const NewSessionMojoCallback& callback) override; - void SendSessionMessage( - presentation::SessionMessagePtr session_message, - const SendMessageMojoCallback& callback) override; + void SendSessionMessage(presentation::PresentationSessionInfoPtr session_info, + presentation::SessionMessagePtr session_message, + const SendMessageMojoCallback& callback) override; void CloseSession( const mojo::String& presentation_url, const mojo::String& presentation_id) override; void ListenForSessionStateChange() override; void ListenForSessionMessages( - const SessionMessagesCallback& callback) override; + presentation::PresentationSessionInfoPtr session) override; // Creates a binding between this object and |request|. void Bind(mojo::InterfaceRequest<presentation::PresentationService> request); @@ -247,10 +247,9 @@ class CONTENT_EXPORT PresentationServiceImpl // Passed to embedder's implementation of PresentationServiceDelegate for // later invocation when session messages arrive. - // For optimization purposes, this method will empty the messages - // passed to it. void OnSessionMessages( - scoped_ptr<ScopedVector<PresentationSessionMessage>> messages); + const content::PresentationSessionInfo& session, + const ScopedVector<PresentationSessionMessage>& messages); // Associates a JoinSession |callback| with a unique request ID and // stores it in a map. diff --git a/content/browser/presentation/presentation_service_impl_unittest.cc b/content/browser/presentation/presentation_service_impl_unittest.cc index ef2106f3..eec340d 100644 --- a/content/browser/presentation/presentation_service_impl_unittest.cc +++ b/content/browser/presentation/presentation_service_impl_unittest.cc @@ -46,9 +46,7 @@ bool ArePresentationSessionsEqual( bool ArePresentationSessionMessagesEqual( const presentation::SessionMessage* expected, const presentation::SessionMessage* actual) { - return expected->presentation_url == actual->presentation_url && - expected->presentation_id == actual->presentation_id && - expected->type == actual->type && + return expected->type == actual->type && expected->message == actual->message && expected->data.Equals(actual->data); } @@ -106,26 +104,24 @@ class MockPresentationServiceDelegate : public PresentationServiceDelegate { void(int render_process_id, int render_frame_id, const std::string& presentation_id)); - MOCK_METHOD3(ListenForSessionMessages, - void( - int render_process_id, - int render_frame_id, - const PresentationSessionMessageCallback& message_cb)); - MOCK_METHOD4(SendMessageRawPtr, - void( - int render_process_id, - int render_frame_id, - PresentationSessionMessage* message_request, - const SendMessageCallback& send_message_cb)); + MOCK_METHOD4(ListenForSessionMessages, + void(int render_process_id, + int render_frame_id, + const content::PresentationSessionInfo& session, + const PresentationSessionMessageCallback& message_cb)); + MOCK_METHOD5(SendMessageRawPtr, + void(int render_process_id, + int render_frame_id, + const content::PresentationSessionInfo& session, + PresentationSessionMessage* message_request, + const SendMessageCallback& send_message_cb)); void SendMessage(int render_process_id, int render_frame_id, + const content::PresentationSessionInfo& session, scoped_ptr<PresentationSessionMessage> message_request, const SendMessageCallback& send_message_cb) override { - SendMessageRawPtr( - render_process_id, - render_frame_id, - message_request.release(), - send_message_cb); + SendMessageRawPtr(render_process_id, render_frame_id, session, + message_request.release(), send_message_cb); } MOCK_METHOD3( ListenForSessionStateChange, @@ -138,6 +134,7 @@ class MockPresentationServiceClient : public presentation::PresentationServiceClient { public: MOCK_METHOD1(OnScreenAvailabilityUpdated, void(bool available)); + void OnSessionStateChanged( presentation::PresentationSessionInfoPtr session_info, presentation::PresentationSessionState new_state) override { @@ -146,9 +143,20 @@ class MockPresentationServiceClient : MOCK_METHOD2(OnSessionStateChanged, void(const presentation::PresentationSessionInfo& session_info, presentation::PresentationSessionState new_state)); + void OnScreenAvailabilityNotSupported() override { NOTIMPLEMENTED(); } + + void OnSessionMessagesReceived( + presentation::PresentationSessionInfoPtr session_info, + mojo::Array<presentation::SessionMessagePtr> messages) override { + messages_received_ = messages.Pass(); + MessagesReceived(); + } + MOCK_METHOD0(MessagesReceived, void()); + + mojo::Array<presentation::SessionMessagePtr> messages_received_; }; class PresentationServiceImplTest : public RenderViewHostImplTestHarness { @@ -270,14 +278,13 @@ class PresentationServiceImplTest : public RenderViewHostImplTestHarness { } void ExpectSessionMessages( - mojo::Array<presentation::SessionMessagePtr> actual_msgs) { - EXPECT_TRUE(actual_msgs.size() == expected_msgs_.size()); + const mojo::Array<presentation::SessionMessagePtr>& expected_msgs, + const mojo::Array<presentation::SessionMessagePtr>& actual_msgs) { + EXPECT_EQ(expected_msgs.size(), actual_msgs.size()); for (size_t i = 0; i < actual_msgs.size(); ++i) { - EXPECT_TRUE(ArePresentationSessionMessagesEqual(expected_msgs_[i].get(), + EXPECT_TRUE(ArePresentationSessionMessagesEqual(expected_msgs[i].get(), actual_msgs[i].get())); } - if (!run_loop_quit_closure_.is_null()) - run_loop_quit_closure_.Run(); } void ExpectSendMessageMojoCallback(bool success) { @@ -287,49 +294,53 @@ class PresentationServiceImplTest : public RenderViewHostImplTestHarness { run_loop_quit_closure_.Run(); } - void RunListenForSessionMessages(std::string& text_msg, - std::vector<uint8_t>& binary_data) { - - - expected_msgs_ = mojo::Array<presentation::SessionMessagePtr>::New(2); - expected_msgs_[0] = presentation::SessionMessage::New(); - expected_msgs_[0]->presentation_url = kPresentationUrl; - expected_msgs_[0]->presentation_id = kPresentationId; - expected_msgs_[0]->type = + void RunListenForSessionMessages(const std::string& text_msg, + const std::vector<uint8_t>& binary_data) { + mojo::Array<presentation::SessionMessagePtr> expected_msgs(2); + expected_msgs[0] = presentation::SessionMessage::New(); + expected_msgs[0]->type = presentation::PresentationMessageType::PRESENTATION_MESSAGE_TYPE_TEXT; - expected_msgs_[0]->message = text_msg; - expected_msgs_[1] = presentation::SessionMessage::New(); - expected_msgs_[1]->presentation_url = kPresentationUrl; - expected_msgs_[1]->presentation_id = kPresentationId; - expected_msgs_[1]->type = presentation::PresentationMessageType:: + expected_msgs[0]->message = text_msg; + expected_msgs[1] = presentation::SessionMessage::New(); + expected_msgs[1]->type = presentation::PresentationMessageType:: PRESENTATION_MESSAGE_TYPE_ARRAY_BUFFER; - expected_msgs_[1]->data = mojo::Array<uint8_t>::From(binary_data); + expected_msgs[1]->data = mojo::Array<uint8_t>::From(binary_data); - service_ptr_->ListenForSessionMessages( - base::Bind(&PresentationServiceImplTest::ExpectSessionMessages, - base::Unretained(this))); + presentation::PresentationSessionInfoPtr session( + presentation::PresentationSessionInfo::New()); + session->url = kPresentationUrl; + session->id = kPresentationId; + PresentationSessionMessageCallback message_cb; + { base::RunLoop run_loop; - base::Callback<void(scoped_ptr<ScopedVector<PresentationSessionMessage>>)> - message_cb; - EXPECT_CALL(mock_delegate_, ListenForSessionMessages(_, _, _)) + EXPECT_CALL(mock_delegate_, ListenForSessionMessages(_, _, _, _)) .WillOnce(DoAll(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit), - SaveArg<2>(&message_cb))); + SaveArg<3>(&message_cb))); + service_ptr_->ListenForSessionMessages(session.Clone()); run_loop.Run(); + } - scoped_ptr<ScopedVector<PresentationSessionMessage>> messages( - new ScopedVector<PresentationSessionMessage>()); - messages->push_back( - content::PresentationSessionMessage::CreateStringMessage( - kPresentationUrl, kPresentationId, - scoped_ptr<std::string>(new std::string(text_msg)))); - messages->push_back( - content::PresentationSessionMessage::CreateArrayBufferMessage( - kPresentationUrl, kPresentationId, - scoped_ptr<std::vector<uint8_t>>( - new std::vector<uint8_t>(binary_data)))); + ScopedVector<PresentationSessionMessage> messages; + scoped_ptr<content::PresentationSessionMessage> message; + message.reset( + new content::PresentationSessionMessage(PresentationMessageType::TEXT)); + message->message = text_msg; + messages.push_back(message.Pass()); + message.reset(new content::PresentationSessionMessage( + PresentationMessageType::ARRAY_BUFFER)); + message->data.reset(new std::vector<uint8_t>(binary_data)); + messages.push_back(message.Pass()); + + std::vector<presentation::SessionMessagePtr> actual_msgs; + { + base::RunLoop run_loop; + EXPECT_CALL(mock_client_, MessagesReceived()) + .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit)); message_cb.Run(messages.Pass()); - SaveQuitClosureAndRunLoop(); + run_loop.Run(); + } + ExpectSessionMessages(expected_msgs, mock_client_.messages_received_); } MockPresentationServiceDelegate mock_delegate_; @@ -343,7 +354,6 @@ class PresentationServiceImplTest : public RenderViewHostImplTestHarness { base::Closure run_loop_quit_closure_; int default_session_started_count_; - mojo::Array<presentation::SessionMessagePtr> expected_msgs_; }; TEST_F(PresentationServiceImplTest, ListenForScreenAvailability) { @@ -552,36 +562,10 @@ TEST_F(PresentationServiceImplTest, ListenForSessionMessages) { TEST_F(PresentationServiceImplTest, ListenForSessionMessagesWithEmptyMsg) { std::string text_msg(""); - std::vector<uint8_t> binary_data{}; + std::vector<uint8_t> binary_data; RunListenForSessionMessages(text_msg, binary_data); } -TEST_F(PresentationServiceImplTest, ReceiveSessionMessagesAfterReset) { - std::string text_msg("123"); - expected_msgs_ = mojo::Array<presentation::SessionMessagePtr>(); - service_ptr_->ListenForSessionMessages( - base::Bind(&PresentationServiceImplTest::ExpectSessionMessages, - base::Unretained(this))); - - base::RunLoop run_loop; - base::Callback<void(scoped_ptr<ScopedVector<PresentationSessionMessage>>)> - message_cb; - EXPECT_CALL(mock_delegate_, ListenForSessionMessages(_, _, _)) - .WillOnce(DoAll(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit), - SaveArg<2>(&message_cb))); - run_loop.Run(); - - scoped_ptr<ScopedVector<PresentationSessionMessage>> messages( - new ScopedVector<PresentationSessionMessage>()); - messages->push_back(content::PresentationSessionMessage::CreateStringMessage( - kPresentationUrl, kPresentationId, - scoped_ptr<std::string>(new std::string(text_msg)))); - ExpectReset(); - service_impl_->Reset(); - message_cb.Run(messages.Pass()); - SaveQuitClosureAndRunLoop(); -} - TEST_F(PresentationServiceImplTest, StartSessionInProgress) { std::string presentation_url1("http://fooUrl"); std::string presentation_url2("http://barUrl"); @@ -653,39 +637,35 @@ TEST_F(PresentationServiceImplTest, DefaultSessionStartReset) { TEST_F(PresentationServiceImplTest, SendStringMessage) { std::string message("Test presentation session message"); + presentation::PresentationSessionInfoPtr session( + presentation::PresentationSessionInfo::New()); + session->url = kPresentationUrl; + session->id = kPresentationId; presentation::SessionMessagePtr message_request( presentation::SessionMessage::New()); - message_request->presentation_url = kPresentationUrl; - message_request->presentation_id = kPresentationId; message_request->type = presentation::PresentationMessageType:: PRESENTATION_MESSAGE_TYPE_TEXT; message_request->message = message; service_ptr_->SendSessionMessage( - message_request.Pass(), - base::Bind( - &PresentationServiceImplTest::ExpectSendMessageMojoCallback, - base::Unretained(this))); + session.Pass(), message_request.Pass(), + base::Bind(&PresentationServiceImplTest::ExpectSendMessageMojoCallback, + base::Unretained(this))); base::RunLoop run_loop; base::Callback<void(bool)> send_message_cb; PresentationSessionMessage* test_message = nullptr; - EXPECT_CALL(mock_delegate_, SendMessageRawPtr( - _, _, _, _)) - .WillOnce(DoAll( - InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit), - SaveArg<2>(&test_message), - SaveArg<3>(&send_message_cb))); + EXPECT_CALL(mock_delegate_, SendMessageRawPtr(_, _, _, _, _)) + .WillOnce(DoAll(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit), + SaveArg<3>(&test_message), SaveArg<4>(&send_message_cb))); run_loop.Run(); + // Make sure |test_message| gets deleted. + scoped_ptr<PresentationSessionMessage> scoped_test_message(test_message); EXPECT_TRUE(test_message); - EXPECT_EQ(kPresentationUrl, test_message->presentation_url); - EXPECT_EQ(kPresentationId, test_message->presentation_id); EXPECT_FALSE(test_message->is_binary()); - EXPECT_TRUE(test_message->message.get()->size() <= - kMaxPresentationSessionMessageSize); - EXPECT_EQ(message, *(test_message->message.get())); - EXPECT_FALSE(test_message->data); - delete test_message; + EXPECT_LE(test_message->message.size(), kMaxPresentationSessionMessageSize); + EXPECT_EQ(message, test_message->message); + ASSERT_FALSE(test_message->data); send_message_cb.Run(true); SaveQuitClosureAndRunLoop(); } @@ -696,41 +676,38 @@ TEST_F(PresentationServiceImplTest, SendArrayBuffer) { std::vector<uint8> data; data.assign(buffer, buffer + sizeof(buffer)); + presentation::PresentationSessionInfoPtr session( + presentation::PresentationSessionInfo::New()); + session->url = kPresentationUrl; + session->id = kPresentationId; presentation::SessionMessagePtr message_request( presentation::SessionMessage::New()); - message_request->presentation_url = kPresentationUrl; - message_request->presentation_id = kPresentationId; message_request->type = presentation::PresentationMessageType:: PRESENTATION_MESSAGE_TYPE_ARRAY_BUFFER; message_request->data = mojo::Array<uint8>::From(data); service_ptr_->SendSessionMessage( - message_request.Pass(), - base::Bind( - &PresentationServiceImplTest::ExpectSendMessageMojoCallback, - base::Unretained(this))); + session.Pass(), message_request.Pass(), + base::Bind(&PresentationServiceImplTest::ExpectSendMessageMojoCallback, + base::Unretained(this))); base::RunLoop run_loop; base::Callback<void(bool)> send_message_cb; PresentationSessionMessage* test_message = nullptr; - EXPECT_CALL(mock_delegate_, SendMessageRawPtr( - _, _, _, _)) - .WillOnce(DoAll( - InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit), - SaveArg<2>(&test_message), - SaveArg<3>(&send_message_cb))); + EXPECT_CALL(mock_delegate_, SendMessageRawPtr(_, _, _, _, _)) + .WillOnce(DoAll(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit), + SaveArg<3>(&test_message), SaveArg<4>(&send_message_cb))); run_loop.Run(); + // Make sure |test_message| gets deleted. + scoped_ptr<PresentationSessionMessage> scoped_test_message(test_message); EXPECT_TRUE(test_message); - EXPECT_EQ(kPresentationUrl, test_message->presentation_url); - EXPECT_EQ(kPresentationId, test_message->presentation_id); EXPECT_TRUE(test_message->is_binary()); EXPECT_EQ(PresentationMessageType::ARRAY_BUFFER, test_message->type); - EXPECT_FALSE(test_message->message); - EXPECT_EQ(data.size(), test_message->data.get()->size()); - EXPECT_TRUE(test_message->data.get()->size() <= - kMaxPresentationSessionMessageSize); - EXPECT_EQ(0, memcmp(buffer, &(*test_message->data.get())[0], sizeof(buffer))); - delete test_message; + EXPECT_TRUE(test_message->message.empty()); + ASSERT_TRUE(test_message->data); + EXPECT_EQ(data.size(), test_message->data->size()); + EXPECT_LE(test_message->data->size(), kMaxPresentationSessionMessageSize); + EXPECT_EQ(0, memcmp(buffer, &(*test_message->data)[0], sizeof(buffer))); send_message_cb.Run(true); SaveQuitClosureAndRunLoop(); } @@ -744,28 +721,26 @@ TEST_F(PresentationServiceImplTest, SendArrayBufferWithExceedingLimit) { std::vector<uint8> data; data.assign(buffer, buffer + sizeof(buffer)); + presentation::PresentationSessionInfoPtr session( + presentation::PresentationSessionInfo::New()); + session->url = kPresentationUrl; + session->id = kPresentationId; presentation::SessionMessagePtr message_request( presentation::SessionMessage::New()); - message_request->presentation_url = kPresentationUrl; - message_request->presentation_id = kPresentationId; message_request->type = presentation::PresentationMessageType:: PRESENTATION_MESSAGE_TYPE_ARRAY_BUFFER; message_request->data = mojo::Array<uint8>::From(data); service_ptr_->SendSessionMessage( - message_request.Pass(), - base::Bind( - &PresentationServiceImplTest::ExpectSendMessageMojoCallback, - base::Unretained(this))); + session.Pass(), message_request.Pass(), + base::Bind(&PresentationServiceImplTest::ExpectSendMessageMojoCallback, + base::Unretained(this))); base::RunLoop run_loop; base::Callback<void(bool)> send_message_cb; PresentationSessionMessage* test_message = nullptr; - EXPECT_CALL(mock_delegate_, SendMessageRawPtr( - _, _, _, _)) - .WillOnce(DoAll( - InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit), - SaveArg<2>(&test_message), - SaveArg<3>(&send_message_cb))); + EXPECT_CALL(mock_delegate_, SendMessageRawPtr(_, _, _, _, _)) + .WillOnce(DoAll(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit), + SaveArg<3>(&test_message), SaveArg<4>(&send_message_cb))); run_loop.Run(); EXPECT_FALSE(test_message); @@ -778,39 +753,38 @@ TEST_F(PresentationServiceImplTest, SendBlobData) { std::vector<uint8> data; data.assign(buffer, buffer + sizeof(buffer)); + presentation::PresentationSessionInfoPtr session( + presentation::PresentationSessionInfo::New()); + session->url = kPresentationUrl; + session->id = kPresentationId; presentation::SessionMessagePtr message_request( presentation::SessionMessage::New()); - message_request->presentation_url = kPresentationUrl; - message_request->presentation_id = kPresentationId; message_request->type = presentation::PresentationMessageType::PRESENTATION_MESSAGE_TYPE_BLOB; message_request->data = mojo::Array<uint8>::From(data); service_ptr_->SendSessionMessage( - message_request.Pass(), + session.Pass(), message_request.Pass(), base::Bind(&PresentationServiceImplTest::ExpectSendMessageMojoCallback, base::Unretained(this))); base::RunLoop run_loop; base::Callback<void(bool)> send_message_cb; PresentationSessionMessage* test_message = nullptr; - EXPECT_CALL(mock_delegate_, SendMessageRawPtr(_, _, _, _)) - .WillOnce(DoAll( - InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit), - SaveArg<2>(&test_message), - SaveArg<3>(&send_message_cb))); + EXPECT_CALL(mock_delegate_, SendMessageRawPtr(_, _, _, _, _)) + .WillOnce(DoAll(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit), + SaveArg<3>(&test_message), SaveArg<4>(&send_message_cb))); run_loop.Run(); + // Make sure |test_message| gets deleted. + scoped_ptr<PresentationSessionMessage> scoped_test_message(test_message); EXPECT_TRUE(test_message); - EXPECT_EQ(kPresentationUrl, test_message->presentation_url); - EXPECT_EQ(kPresentationId, test_message->presentation_id); EXPECT_TRUE(test_message->is_binary()); EXPECT_EQ(PresentationMessageType::BLOB, test_message->type); - EXPECT_FALSE(test_message->message); - EXPECT_EQ(data.size(), test_message->data.get()->size()); - EXPECT_TRUE(test_message->data.get()->size() <= - kMaxPresentationSessionMessageSize); - EXPECT_EQ(0, memcmp(buffer, &(*test_message->data.get())[0], sizeof(buffer))); - delete test_message; + EXPECT_TRUE(test_message->message.empty()); + ASSERT_TRUE(test_message->data); + EXPECT_EQ(data.size(), test_message->data->size()); + EXPECT_LE(test_message->data->size(), kMaxPresentationSessionMessageSize); + EXPECT_EQ(0, memcmp(buffer, &(*test_message->data)[0], sizeof(buffer))); send_message_cb.Run(true); SaveQuitClosureAndRunLoop(); } diff --git a/content/browser/presentation/presentation_type_converters.h b/content/browser/presentation/presentation_type_converters.h index 38a8524..f66c323 100644 --- a/content/browser/presentation/presentation_type_converters.h +++ b/content/browser/presentation/presentation_type_converters.h @@ -35,6 +35,15 @@ struct TypeConverter<presentation::PresentationSessionInfoPtr, }; template <> +struct TypeConverter<content::PresentationSessionInfo, + presentation::PresentationSessionInfoPtr> { + static content::PresentationSessionInfo Convert( + const presentation::PresentationSessionInfoPtr& input) { + return content::PresentationSessionInfo(input->url, input->id); + } +}; + +template <> struct TypeConverter<presentation::PresentationErrorPtr, content::PresentationError> { static presentation::PresentationErrorPtr Convert( diff --git a/content/common/presentation/presentation_service.mojom b/content/common/presentation/presentation_service.mojom index 18cb24c..ae0126d 100644 --- a/content/common/presentation/presentation_service.mojom +++ b/content/common/presentation/presentation_service.mojom @@ -33,8 +33,6 @@ enum PresentationMessageType { }; struct SessionMessage { - string presentation_url; - string presentation_id; PresentationMessageType type; // Used when message type is TEXT. string? message; @@ -90,7 +88,7 @@ interface PresentationService { // The false in the result callback notifies the renderer to stop sending // the send requests and invalidate all pending requests. This occurs // for eg., when frame is deleted or navigated away. - SendSessionMessage(SessionMessage message_request) => (bool success); + SendSessionMessage(PresentationSessionInfo sessionInfo, SessionMessage message_request) => (bool success); // Called when closeSession() is called by the frame. CloseSession(string presentation_url, string presentation_id); @@ -98,13 +96,14 @@ interface PresentationService { // Starts listening for state changes for sessions created on this frame. // When state change occurs, PresentationServiceClient::OnSessionStateChanged // will be invoked with the session and its new state. + // This is called after a presentation session is created. ListenForSessionStateChange(); - // Called when the frame is ready to process the next batch of messages. - // When the callback carries null messages, there is an error - // at the presentation service side. - ListenForSessionMessages() - => (array<SessionMessage>? messages); + // Starts listening for messages for session with |sessionInfo|. + // Messages will be received in + // PresentationServiceClient::OnSessionMessagesReceived. + // This is called after a presentation session is created. + ListenForSessionMessages(PresentationSessionInfo sessionInfo); }; interface PresentationServiceClient { @@ -123,4 +122,7 @@ interface PresentationServiceClient { // See PresentationService::ListenForSessionStateChange. OnSessionStateChanged(PresentationSessionInfo sessionInfo, PresentationSessionState newState); + + // See PresentationService::ListenForSessionMessages. + OnSessionMessagesReceived(PresentationSessionInfo sessionInfo, array<SessionMessage> messages); }; diff --git a/content/public/browser/presentation_service_delegate.h b/content/public/browser/presentation_service_delegate.h index 01b0fda..90d0ce4 100644 --- a/content/public/browser/presentation_service_delegate.h +++ b/content/public/browser/presentation_service_delegate.h @@ -21,6 +21,8 @@ class PresentationScreenAvailabilityListener; using SessionStateChangedCallback = base::Callback<void(const PresentationSessionInfo&, PresentationSessionState)>; +using PresentationSessionMessageCallback = base::Callback<void( + const ScopedVector<content::PresentationSessionMessage>&)>; // An interface implemented by embedders to handle presentation API calls // forwarded from PresentationServiceImpl. @@ -47,8 +49,6 @@ class CONTENT_EXPORT PresentationServiceDelegate { base::Callback<void(const PresentationSessionInfo&)>; using PresentationSessionErrorCallback = base::Callback<void(const PresentationError&)>; - using PresentationSessionMessageCallback = base::Callback<void( - scoped_ptr<ScopedVector<PresentationSessionMessage>>)>; using SendMessageCallback = base::Callback<void(bool)>; virtual ~PresentationServiceDelegate() {} @@ -144,24 +144,28 @@ class CONTENT_EXPORT PresentationServiceDelegate { int render_frame_id, const std::string& presentation_id) = 0; - // Gets the next batch of messages from all presentation sessions in the frame + // Listen for messages for a presentation session. // |render_process_id|, |render_frame_id|: ID for originating frame. - // |message_cb|: Invoked with a non-empty list of messages. + // |session|: URL and ID of presentation session to listen for messages. + // |message_cb|: Invoked with a non-empty list of messages whenever there are + // messages. virtual void ListenForSessionMessages( int render_process_id, int render_frame_id, + const content::PresentationSessionInfo& session, const PresentationSessionMessageCallback& message_cb) = 0; // Sends a message (string or binary data) to a presentation session. // |render_process_id|, |render_frame_id|: ID of originating frame. - // |message_request|: Contains Presentation URL, ID and message to be sent - // and delegate is responsible for deallocating the message_request. + // |session|: The presentation session to send the message to. + // |message|: The message to send. The embedder takes ownership of |message|. + // Must not be null. // |send_message_cb|: Invoked after handling the send message request. - virtual void SendMessage( - int render_process_id, - int render_frame_id, - scoped_ptr<PresentationSessionMessage> message_request, - const SendMessageCallback& send_message_cb) = 0; + virtual void SendMessage(int render_process_id, + int render_frame_id, + const content::PresentationSessionInfo& session, + scoped_ptr<PresentationSessionMessage> message, + const SendMessageCallback& send_message_cb) = 0; // Continuously listen for presentation session state changes for a frame. // |render_process_id|, |render_frame_id|: ID of frame. diff --git a/content/public/browser/presentation_session_message.cc b/content/public/browser/presentation_session_message.cc index 4d7bfd5..24c78d7 100644 --- a/content/public/browser/presentation_session_message.cc +++ b/content/public/browser/presentation_session_message.cc @@ -7,65 +7,13 @@ namespace content { PresentationSessionMessage::PresentationSessionMessage( - const std::string& presentation_url, - const std::string& presentation_id, - scoped_ptr<std::string> message) - : presentation_url(presentation_url), - presentation_id(presentation_id), - type(PresentationMessageType::TEXT), - message(message.Pass()), - data(nullptr) { -} - -PresentationSessionMessage::PresentationSessionMessage( - const std::string& presentation_url, - const std::string& presentation_id, - PresentationMessageType type, - scoped_ptr<std::vector<uint8_t>> data) - : presentation_url(presentation_url), - presentation_id(presentation_id), - type(type), - message(nullptr), - data(data.Pass()) { -} + PresentationMessageType type) + : type(type) {} -// static -scoped_ptr<PresentationSessionMessage> -PresentationSessionMessage::CreateStringMessage( - const std::string& presentation_url, - const std::string& presentation_id, - scoped_ptr<std::string> message) { - return scoped_ptr<PresentationSessionMessage>(new PresentationSessionMessage( - presentation_url, presentation_id, message.Pass())); -} - -// static -scoped_ptr<PresentationSessionMessage> -PresentationSessionMessage::CreateArrayBufferMessage( - const std::string& presentation_url, - const std::string& presentation_id, - scoped_ptr<std::vector<uint8_t>> data) { - return scoped_ptr<PresentationSessionMessage>(new PresentationSessionMessage( - presentation_url, presentation_id, PresentationMessageType::ARRAY_BUFFER, - data.Pass())); -} - -// static -scoped_ptr<PresentationSessionMessage> -PresentationSessionMessage::CreateBlobMessage( - const std::string& presentation_url, - const std::string& presentation_id, - scoped_ptr<std::vector<uint8_t>> data) { - return scoped_ptr<PresentationSessionMessage>(new PresentationSessionMessage( - presentation_url, presentation_id, PresentationMessageType::BLOB, - data.Pass())); -} +PresentationSessionMessage::~PresentationSessionMessage() {} bool PresentationSessionMessage::is_binary() const { return data != nullptr; } -PresentationSessionMessage::~PresentationSessionMessage() { -} - } // namespace content diff --git a/content/public/browser/presentation_session_message.h b/content/public/browser/presentation_session_message.h index 80e8050..d3192c0 100644 --- a/content/public/browser/presentation_session_message.h +++ b/content/public/browser/presentation_session_message.h @@ -24,41 +24,13 @@ enum PresentationMessageType { // Empty messages are allowed. struct CONTENT_EXPORT PresentationSessionMessage { public: + explicit PresentationSessionMessage(PresentationMessageType type); ~PresentationSessionMessage(); - // Creates string message, which takes the ownership of |message|. - static scoped_ptr<PresentationSessionMessage> CreateStringMessage( - const std::string& presentation_url, - const std::string& presentation_id, - scoped_ptr<std::string> message); - - // Creates array buffer message, which takes the ownership of |data|. - static scoped_ptr<PresentationSessionMessage> CreateArrayBufferMessage( - const std::string& presentation_url, - const std::string& presentation_id, - scoped_ptr<std::vector<uint8_t>> data); - - // Creates blob message, which takes the ownership of |data|. - static scoped_ptr<PresentationSessionMessage> CreateBlobMessage( - const std::string& presentation_url, - const std::string& presentation_id, - scoped_ptr<std::vector<uint8_t>> data); - bool is_binary() const; - std::string presentation_url; - std::string presentation_id; - PresentationMessageType type; - scoped_ptr<std::string> message; + const PresentationMessageType type; + std::string message; scoped_ptr<std::vector<uint8_t>> data; - - private: - PresentationSessionMessage(const std::string& presentation_url, - const std::string& presentation_id, - scoped_ptr<std::string> message); - PresentationSessionMessage(const std::string& presentation_url, - const std::string& presentation_id, - PresentationMessageType type, - scoped_ptr<std::vector<uint8_t>> data); }; } // namespace content diff --git a/content/renderer/presentation/presentation_dispatcher.cc b/content/renderer/presentation/presentation_dispatcher.cc index 1538883..74b4dca 100644 --- a/content/renderer/presentation/presentation_dispatcher.cc +++ b/content/renderer/presentation/presentation_dispatcher.cc @@ -50,22 +50,6 @@ blink::WebPresentationSessionState GetWebPresentationSessionStateFromMojo( return blink::WebPresentationSessionState::Disconnected; } -presentation::SessionMessage* GetMojoSessionMessage( - const blink::WebString& presentationUrl, - const blink::WebString& presentationId, - presentation::PresentationMessageType type, - const uint8* data, - size_t length) { - presentation::SessionMessage* session_message = - new presentation::SessionMessage(); - session_message->presentation_url = presentationUrl.utf8(); - session_message->presentation_id = presentationId.utf8(); - session_message->type = type; - const std::vector<uint8> vector(data, data + length); - session_message->data = mojo::Array<uint8>::From(vector); - return session_message; -} - } // namespace namespace content { @@ -75,9 +59,7 @@ PresentationDispatcher::PresentationDispatcher(RenderFrame* render_frame) controller_(nullptr), binding_(this), listening_state_(ListeningState::Inactive), - last_known_availability_(false), - listening_for_messages_(false) { -} + last_known_availability_(false) {} PresentationDispatcher::~PresentationDispatcher() { // Controller should be destroyed before the dispatcher when frame is @@ -149,21 +131,11 @@ void PresentationDispatcher::sendString( return; } - presentation::SessionMessage* session_message = - new presentation::SessionMessage(); - session_message->presentation_url = presentationUrl.utf8(); - session_message->presentation_id = presentationId.utf8(); - session_message->type = presentation::PresentationMessageType:: - PRESENTATION_MESSAGE_TYPE_TEXT; - session_message->message = message.utf8(); - - message_request_queue_.push(make_linked_ptr(session_message)); + message_request_queue_.push(make_linked_ptr( + CreateSendTextMessageRequest(presentationUrl, presentationId, message))); // Start processing request if only one in the queue. - if (message_request_queue_.size() == 1) { - const linked_ptr<presentation::SessionMessage>& request = - message_request_queue_.front(); - DoSendMessage(*request); - } + if (message_request_queue_.size() == 1) + DoSendMessage(message_request_queue_.front().get()); } void PresentationDispatcher::sendArrayBuffer( @@ -178,18 +150,14 @@ void PresentationDispatcher::sendArrayBuffer( return; } - presentation::SessionMessage* session_message = - GetMojoSessionMessage(presentationUrl, presentationId, - presentation::PresentationMessageType:: - PRESENTATION_MESSAGE_TYPE_ARRAY_BUFFER, - data, length); - message_request_queue_.push(make_linked_ptr(session_message)); + message_request_queue_.push(make_linked_ptr( + CreateSendBinaryMessageRequest(presentationUrl, presentationId, + presentation::PresentationMessageType:: + PRESENTATION_MESSAGE_TYPE_ARRAY_BUFFER, + data, length))); // Start processing request if only one in the queue. - if (message_request_queue_.size() == 1) { - const linked_ptr<presentation::SessionMessage>& request = - message_request_queue_.front(); - DoSendMessage(*request); - } + if (message_request_queue_.size() == 1) + DoSendMessage(message_request_queue_.front().get()); } void PresentationDispatcher::sendBlobData( @@ -204,49 +172,20 @@ void PresentationDispatcher::sendBlobData( return; } - presentation::SessionMessage* session_message = GetMojoSessionMessage( + message_request_queue_.push(make_linked_ptr(CreateSendBinaryMessageRequest( presentationUrl, presentationId, presentation::PresentationMessageType::PRESENTATION_MESSAGE_TYPE_BLOB, - data, length); - message_request_queue_.push(make_linked_ptr(session_message)); - if (message_request_queue_.size() == 1) { - const linked_ptr<presentation::SessionMessage>& request = - message_request_queue_.front(); - DoSendMessage(*request); - } + data, length))); + // Start processing request if only one in the queue. + if (message_request_queue_.size() == 1) + DoSendMessage(message_request_queue_.front().get()); } -void PresentationDispatcher::DoSendMessage( - const presentation::SessionMessage& session_message) { +void PresentationDispatcher::DoSendMessage(SendMessageRequest* request) { ConnectToPresentationServiceIfNeeded(); - presentation::SessionMessagePtr message_request( - presentation::SessionMessage::New()); - message_request->presentation_url = session_message.presentation_url; - message_request->presentation_id = session_message.presentation_id; - message_request->type = session_message.type; - switch (session_message.type) { - case presentation::PresentationMessageType:: - PRESENTATION_MESSAGE_TYPE_TEXT: { - message_request->message = session_message.message; - break; - } - case presentation::PresentationMessageType:: - PRESENTATION_MESSAGE_TYPE_ARRAY_BUFFER: - case presentation::PresentationMessageType:: - PRESENTATION_MESSAGE_TYPE_BLOB: { - message_request->data = - mojo::Array<uint8>::From(session_message.data.storage()); - break; - } - default: { - NOTREACHED() << "Invalid presentation message type " - << session_message.type; - break; - } - } presentation_service_->SendSessionMessage( - message_request.Pass(), + request->session_info.Pass(), request->message.Pass(), base::Bind(&PresentationDispatcher::HandleSendMessageRequests, base::Unretained(this))); } @@ -268,9 +207,7 @@ void PresentationDispatcher::HandleSendMessageRequests(bool success) { message_request_queue_.pop(); if (!message_request_queue_.empty()) { - const linked_ptr<presentation::SessionMessage>& request = - message_request_queue_.front(); - DoSendMessage(*request); + DoSendMessage(message_request_queue_.front().get()); } } @@ -327,8 +264,6 @@ void PresentationDispatcher::DidCommitProvisionalLoad( // Remove all pending send message requests. MessageRequestQueue empty; std::swap(message_request_queue_, empty); - - listening_for_messages_ = false; } void PresentationDispatcher::OnScreenAvailabilityUpdated(bool available) { @@ -378,8 +313,8 @@ void PresentationDispatcher::OnDefaultSessionStarted( if (!session_info.is_null()) { controller_->didStartDefaultSession( - new PresentationSessionClient(session_info.Pass())); - StartListenForMessages(); + new PresentationSessionClient(session_info.Clone())); + presentation_service_->ListenForSessionMessages(session_info.Pass()); } } @@ -397,18 +332,8 @@ void PresentationDispatcher::OnSessionCreated( } DCHECK(!session_info.is_null()); - callback->onSuccess(new PresentationSessionClient(session_info.Pass())); - StartListenForMessages(); -} - -void PresentationDispatcher::StartListenForMessages() { - if (listening_for_messages_) - return; - - listening_for_messages_ = true; - presentation_service_->ListenForSessionMessages( - base::Bind(&PresentationDispatcher::OnSessionMessagesReceived, - base::Unretained(this))); + callback->onSuccess(new PresentationSessionClient(session_info.Clone())); + presentation_service_->ListenForSessionMessages(session_info.Pass()); } void PresentationDispatcher::OnSessionStateChanged( @@ -424,22 +349,16 @@ void PresentationDispatcher::OnSessionStateChanged( } void PresentationDispatcher::OnSessionMessagesReceived( + presentation::PresentationSessionInfoPtr session_info, mojo::Array<presentation::SessionMessagePtr> messages) { - if (!listening_for_messages_) - return; // messages may come after the frame navigated. - - // When messages is null, there is an error at presentation service side. - if (!controller_ || messages.is_null()) { - listening_for_messages_ = false; + if (!controller_) return; - } for (size_t i = 0; i < messages.size(); ++i) { // Note: Passing batches of messages to the Blink layer would be more // efficient. scoped_ptr<PresentationSessionClient> session_client( - new PresentationSessionClient(messages[i]->presentation_url, - messages[i]->presentation_id)); + new PresentationSessionClient(session_info->url, session_info->id)); switch (messages[i]->type) { case presentation::PresentationMessageType:: PRESENTATION_MESSAGE_TYPE_TEXT: { @@ -463,10 +382,6 @@ void PresentationDispatcher::OnSessionMessagesReceived( } } } - - presentation_service_->ListenForSessionMessages( - base::Bind(&PresentationDispatcher::OnSessionMessagesReceived, - base::Unretained(this))); } void PresentationDispatcher::ConnectToPresentationServiceIfNeeded() { @@ -503,4 +418,51 @@ void PresentationDispatcher::UpdateListeningState() { } } +PresentationDispatcher::SendMessageRequest::SendMessageRequest( + presentation::PresentationSessionInfoPtr session_info, + presentation::SessionMessagePtr message) + : session_info(session_info.Pass()), message(message.Pass()) {} + +PresentationDispatcher::SendMessageRequest::~SendMessageRequest() {} + +// static +PresentationDispatcher::SendMessageRequest* +PresentationDispatcher::CreateSendTextMessageRequest( + const blink::WebString& presentationUrl, + const blink::WebString& presentationId, + const blink::WebString& message) { + presentation::PresentationSessionInfoPtr session_info = + presentation::PresentationSessionInfo::New(); + session_info->url = presentationUrl.utf8(); + session_info->id = presentationId.utf8(); + + presentation::SessionMessagePtr session_message = + presentation::SessionMessage::New(); + session_message->type = + presentation::PresentationMessageType::PRESENTATION_MESSAGE_TYPE_TEXT; + session_message->message = message.utf8(); + return new SendMessageRequest(session_info.Pass(), session_message.Pass()); +} + +// static +PresentationDispatcher::SendMessageRequest* +PresentationDispatcher::CreateSendBinaryMessageRequest( + const blink::WebString& presentationUrl, + const blink::WebString& presentationId, + presentation::PresentationMessageType type, + const uint8* data, + size_t length) { + presentation::PresentationSessionInfoPtr session_info = + presentation::PresentationSessionInfo::New(); + session_info->url = presentationUrl.utf8(); + session_info->id = presentationId.utf8(); + + presentation::SessionMessagePtr session_message = + presentation::SessionMessage::New(); + session_message->type = type; + std::vector<uint8> tmp_data_vector(data, data + length); + session_message->data.Swap(&tmp_data_vector); + return new SendMessageRequest(session_info.Pass(), session_message.Pass()); +} + } // namespace content diff --git a/content/renderer/presentation/presentation_dispatcher.h b/content/renderer/presentation/presentation_dispatcher.h index 1f8887a..6ef3874 100644 --- a/content/renderer/presentation/presentation_dispatcher.h +++ b/content/renderer/presentation/presentation_dispatcher.h @@ -31,6 +31,26 @@ class CONTENT_EXPORT PresentationDispatcher ~PresentationDispatcher() override; private: + struct SendMessageRequest { + SendMessageRequest(presentation::PresentationSessionInfoPtr session_info, + presentation::SessionMessagePtr message); + ~SendMessageRequest(); + + presentation::PresentationSessionInfoPtr session_info; + presentation::SessionMessagePtr message; + }; + + static SendMessageRequest* CreateSendTextMessageRequest( + const blink::WebString& presentationUrl, + const blink::WebString& presentationId, + const blink::WebString& message); + static SendMessageRequest* CreateSendBinaryMessageRequest( + const blink::WebString& presentationUrl, + const blink::WebString& presentationId, + presentation::PresentationMessageType type, + const uint8* data, + size_t length); + // WebPresentationClient implementation. virtual void setController( blink::WebPresentationController* controller); @@ -77,6 +97,9 @@ class CONTENT_EXPORT PresentationDispatcher presentation::PresentationSessionInfoPtr session_info, presentation::PresentationSessionState new_state) override; void OnScreenAvailabilityNotSupported() override; + void OnSessionMessagesReceived( + presentation::PresentationSessionInfoPtr session_info, + mojo::Array<presentation::SessionMessagePtr> messages) override; void OnSessionCreated( blink::WebPresentationSessionClientCallbacks* callback, @@ -84,17 +107,17 @@ class CONTENT_EXPORT PresentationDispatcher presentation::PresentationErrorPtr error); void OnDefaultSessionStarted( presentation::PresentationSessionInfoPtr session_info); - void OnSessionMessagesReceived( - mojo::Array<presentation::SessionMessagePtr> messages); - void DoSendMessage(const presentation::SessionMessage& session_message); + + // Call to PresentationService to send the message in |request|. + // |session_info| and |message| of |reuqest| will be consumed. + // |HandleSendMessageRequests| will be invoked after the send is attempted. + void DoSendMessage(SendMessageRequest* request); void HandleSendMessageRequests(bool success); void ConnectToPresentationServiceIfNeeded(); void UpdateListeningState(); - void StartListenForMessages(); - // Used as a weak reference. Can be null since lifetime is bound to the frame. blink::WebPresentationController* controller_; presentation::PresentationServicePtr presentation_service_; @@ -102,8 +125,7 @@ class CONTENT_EXPORT PresentationDispatcher // Message requests are queued here and only one message at a time is sent // over mojo channel. - using MessageRequestQueue = - std::queue<linked_ptr<presentation::SessionMessage>>; + using MessageRequestQueue = std::queue<linked_ptr<SendMessageRequest>>; MessageRequestQueue message_request_queue_; enum class ListeningState { @@ -123,8 +145,6 @@ class CONTENT_EXPORT PresentationDispatcher std::set<blink::WebPresentationAvailabilityObserver*>; AvailabilityObserversSet availability_observers_; - bool listening_for_messages_; - DISALLOW_COPY_AND_ASSIGN(PresentationDispatcher); }; diff --git a/extensions/renderer/resources/media_router_bindings.js b/extensions/renderer/resources/media_router_bindings.js index 0c1dca2..95fdcb1 100644 --- a/extensions/renderer/resources/media_router_bindings.js +++ b/extensions/renderer/resources/media_router_bindings.js @@ -63,13 +63,11 @@ define('media_router_bindings', [ function messageToMojo_(message) { if ("string" == typeof message.message) { return new mediaRouterMojom.RouteMessage({ - 'route_id': message.routeId, 'type': mediaRouterMojom.RouteMessage.Type.TEXT, 'message': message.message, }); } else { return new mediaRouterMojom.RouteMessage({ - 'route_id': message.routeId, 'type': mediaRouterMojom.RouteMessage.Type.BINARY, 'data': message.message, }); @@ -322,7 +320,7 @@ define('media_router_bindings', [ this.sendRouteBinaryMessage = null; /** - * @type {function(Array.<string>): Promise.<Array.<RouteMessage>>} + * @type {function(string): Promise.<Array.<RouteMessage>>} */ this.listenForRouteMessages = null; @@ -504,12 +502,12 @@ define('media_router_bindings', [ /** * Listen for next batch of messages from one of the routeIds. - * @param {!Array.<string>} routeIds + * @param {!string} routeId * @return {!Promise.<Array.<RouteMessage>>} Resolved with a list of messages, * an empty list if an error occurred. */ - MediaRouteProvider.prototype.listenForRouteMessages = function(routeIds) { - return this.handlers_.listenForRouteMessages(routeIds) + MediaRouteProvider.prototype.listenForRouteMessages = function(routeId) { + return this.handlers_.listenForRouteMessages([routeId]) .then(function(messages) { return {'messages': messages.map(messageToMojo_)}; }, function() { |