diff options
author | imcheng <imcheng@google.com> | 2015-08-06 18:00:14 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-08-07 01:01:39 +0000 |
commit | a0b6dc9205a195a4fa8c52a71fa1cb2ae8abb591 (patch) | |
tree | d14fc535a1efb5d7667b496920e6a82926f42497 | |
parent | bb1fa29f61337a14332e6a038b6b09f23d8098b5 (diff) | |
download | chromium_src-a0b6dc9205a195a4fa8c52a71fa1cb2ae8abb591.zip chromium_src-a0b6dc9205a195a4fa8c52a71fa1cb2ae8abb591.tar.gz chromium_src-a0b6dc9205a195a4fa8c52a71fa1cb2ae8abb591.tar.bz2 |
[Presentation API] Change ListenForSessionMessages API to observers.
ListenForSessionMessages is now observer style and listens for a single
presentation as opposed to all sessions in that frame.
This change also separate presentation url / id from the
various SessionMessage structs from presentation mojom / content.
Now SendSessionMessage / ListenForSessionMessages must pass in an
explicit PresentationSessionInfo. Doing this allows us to avoid parsing
presenation url/id from route id for presentation session - MediaRoute
association, because previously we would have to fill out the url/id
field of content::PresentationSessionMessage struct and there was no
good way to do it without parsing. Now we simply bind the
PresentationSessionInfo to the callback that receives the messages.
Removing the pres url/id from the message struct also avoids
duplication when sending a batch of messages for the same session.
Added PresentationSessionMessagesObserver class and MediaRouter API
to implement the observer style API. MediaRouterMojoImpl contains
logic to adapt the observer stlye API to the getNext style API for
listening for route messages from the Media Route Provider.
Another change that were needed to completely remove the route id
parsing logic is to invoke MediaRouteResponseCallbacks with the
resulting presentation id generated by MediaRouter. This is needed
due to browser-initiated presentations where the presentation id is
not known in advance.
BUG=510297,510267
Review URL: https://codereview.chromium.org/1259073004
Cr-Commit-Position: refs/heads/master@{#342249}
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() { |