[Media Router] Add ProvideSinks() Mojo API

chrome/browser/media/router/media_router.h

Index: chrome/browser/media/router/media_router.h
diff --git a/chrome/browser/media/router/media_router.h b/chrome/browser/media/router/media_router.h
index 92d6d068729e7e475d55f696e57c6cd04a6179ed..1084ef7e568cee410ca063fc6384b57a5c163058 100644
--- a/chrome/browser/media/router/media_router.h
+++ b/chrome/browser/media/router/media_router.h
@@ -59,6 +59,7 @@ using PresentationConnectionStateSubscription = base::CallbackList<void(
 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;
 
@@ -168,6 +169,10 @@ class MediaRouter : public KeyedService {
       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