[Media Router] Add ProvideSinks() Mojo API

chrome/browser/media/router/media_router.h

 // 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_MEDIA_ROUTER_H_
 #define CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTER_H_
 
 #include <stdint.h>
 
 #include <string>
@@ skipping 41 matching lines @@
     const content::PresentationConnectionStateChangeInfo&)>::Subscription;
 
 // An interface for handling resources related to media routing.
 // Responsible for registering observers for receiving sink availability
 // updates, handling route requests/responses, and operating on routes (e.g.
 // posting messages or closing).
 // TODO(imcheng): Reduce number of parameters by putting them into structs.
 class MediaRouter : public KeyedService {
  public:
   using SendRouteMessageCallback = base::Callback<void(bool sent)>;
+  using MediaSinkList = std::vector<std::unique_ptr<MediaSink>>;

[mark a. foltz, 2017/02/10 01:23:55]

Should MediaSink be move-only? Or is there a use case for copying?  When do we
copy and when do we pass pointers?

[imcheng, 2017/02/10 22:45:18]

I think this just needs to be updated according to changes in
https://codereview.chromium.org/2675033002.

Previously we were exploring options with subtyping MediaSinks but found it did
not work well with downcasting / passing ptrs in the APIs.

After discussing with Bin, we came up with a wrapper for MediaSinks with a union
of DIAL/Cast specific data that needs to be passed to the extension. This seems
to work reasonably well without affecting consumers of MediaSinkObservers.

[zhaobin, 2017/02/16 22:56:35]

Done.

 
   ~MediaRouter() override = default;
 
   // Creates a media route from |source_id| to |sink_id|.
   // |origin| is the origin of requestor's page.
   // |web_contents| is the WebContents of the tab in which the request was made.
   // |origin| and |web_contents| are used for enforcing same-origin and/or
   // same-tab scope for JoinRoute() requests. (e.g., if enforced, the page
   // requesting JoinRoute() must have the same origin as the page that requested
   // CreateRoute()).
@@ skipping 89 matching lines @@
   // the user has no domain or is not signed in.  |sink_callback| will be called
   // either with the ID of the new sink when it is found or with an empty string
   // if no sink was found.
   virtual void SearchSinks(
       const MediaSink::Id& sink_id,
       const MediaSource::Id& source_id,
       const std::string& search_input,
       const std::string& domain,
       const MediaSinkSearchResponseCallback& sink_callback) = 0;
 
+  // Called when DIAL or CAST MediaSinkService finishes sink discovery.

[mark a. foltz, 2017/02/10 01:23:55]

Are there multiple services running at once?   Do they provide a combined list
or call this API separately?

[imcheng, 2017/02/10 22:45:18]

It seems this will be called directly by each service but I wonder if it makes
sense: without any additional info (such as provider name), it seems we don't
have the ability to tell an individual MRP that there are no sinks.

Also, I don't think sending a combined sink list to the extension makes sense,
as it means someone has to ask each service for current list of sinks before
pushing to the extension. So I would prefer this to be called per-service, but
we'd need to add back a provider name here.

[zhaobin, 2017/02/16 22:56:35]

Done.

+  // |sinks|: sinks discovered by MediaSinkService.
+  virtual void OnSinksDiscovered(std::unique_ptr<MediaSinkList> sinks) = 0;

[mark a. foltz, 2017/02/10 01:23:55]

Passing a std::vector by unique_ptr isn't common.  It's more common to pass by
const reference.  The implementation can cheaply copy the vector if it needs to
(it's just a list of pointers) or use std::move.

This would also avoid writing trampoline methods for mocking in unit tests.

[imcheng, 2017/02/10 22:45:18]

I don't think a vector<unique_ptr<T>> is copyable but I don't think the vector
itself needs to be wrapped in a unique_ptr (you can std::move the vector when
calling this method). But see comment in L62 above.

[zhaobin, 2017/02/16 22:56:35]

Done.

+
   // Adds |callback| to listen for state changes for presentation connected to
   // |route_id|. The returned Subscription object is owned by the caller.
   // |callback| will be invoked whenever there are state changes, until the
   // caller destroys the Subscription object.
   virtual std::unique_ptr<PresentationConnectionStateSubscription>
   AddPresentationConnectionStateChangedCallback(
       const MediaRoute::Id& route_id,
       const content::PresentationConnectionStateChangedCallback& callback) = 0;
 
   // Called when the incognito profile for this instance is being shut down.
@@ skipping 60 matching lines @@
 
   // Unregisters a previously registered RouteMessagesObserver. |observer| will
   // stop receiving further updates.
   virtual void UnregisterRouteMessageObserver(
       RouteMessageObserver* observer) = 0;
 };
 
 }  // namespace media_router
 
 #endif  // CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTER_H_