Non-Local Join for Media Router and Presentation API

chrome/browser/media/router/media_router.mojom

Index: chrome/browser/media/router/media_router.mojom
diff --git a/chrome/browser/media/router/media_router.mojom b/chrome/browser/media/router/media_router.mojom
index 6289a2d0736ee46d3ed8a3510546fc35377021d8..af5e0750b566b3df66c3d71c442e6f549e933a83 100644
--- a/chrome/browser/media/router/media_router.mojom
+++ b/chrome/browser/media/router/media_router.mojom
@@ -160,12 +160,15 @@ interface MediaRouteProvider {
   StopObservingMediaSinks(string media_source);
 
   // Starts reporting the state of active media routes via
-  // OnRoutesUpdated(). Querying will continue until
-  // StopObservingMediaRoutes() is called.
-  StartObservingMediaRoutes();
+  // OnRoutesUpdated() in the context of the |media_source|.
+  // The |media_source| should be considered when returning joinable
+  // routes in the OnRoutesUpdated() call.
+  // Querying will continue until StopObservingMediaRoutes() is called.

[mark a. foltz, 2015/12/09 00:48:16]

The media_source should be optional correct?
Please document this fact.

[matt.boetger, 2015/12/15 19:21:20]

It was not my intention to make this optional.  I will make it optional however.
 

With the concern for making it possible to observe routes without a media
source, do we need to also create an OnRoutesUpdated that does not take a media
source?  Should we make a separate overloaded call?  Or perhaps a new method
call completely.  

[mark a. foltz, 2015/12/16 07:17:34]

I don't think so.  Since we're passing back the media_source with the route
update the client can match it up with the observer object on its side.

+  StartObservingMediaRoutes(string media_source);
 
-  // Stops querying the state of all media routes.
-  StopObservingMediaRoutes();
+  // Stops querying the state of all media routes in the context of
+  // the |media_source|.
+  StopObservingMediaRoutes(string media_source);

[mark a. foltz, 2015/12/09 00:48:16]

What happens if the client calls

Start(foo)
Stop()

or

Start()
Stop(foo)

or

Start(foo)
Stop(bar)

?

It's important that we be able to reliably turn off route updates from the MRPM.

[matt.boetger, 2015/12/15 19:21:20]

It was intended that if you call Start(foo) you must call Stop(foo) - exactly
how the Start/Stop of the media sink queries work.  I'm not sure why you'd
expect any other behavior, but I will document it here.  Should I add similar
documentation to the Media Sink queries?

[mark a. foltz, 2015/12/16 07:17:34]

Sure, it would be great to make this more explicit where it's not.

 
   // Called when the MediaRouter is ready to get the next batch of messages
   // associated with |route_id|.
@@ -220,8 +223,11 @@ interface MediaRouter {
   // Called when issues are reported for media routes.
   OnIssue(Issue issue);
 
-  // Called when list of routes has been updated.
-  OnRoutesUpdated(array<MediaRoute> routes);
+  // Called when list of routes has been updated in the context of the
+  // calling |media_source|.  The array |joinable_route_ids| should
+  // contain route IDs of joinable routes found in the |routes| array.
+  OnRoutesUpdated(string media_source, array<MediaRoute> routes,
+      array<string> joinable_route_ids);

[mark a. foltz, 2015/12/03 19:24:23]

I would much rather add an is_joinable boolean flag to MediaRoute instead of
passing a separate array.

[mark a. foltz, 2015/12/09 00:48:16]

You can ignore this comment.

[matt.boetger, 2015/12/15 19:21:20]

Done.

[matt.boetger, 2015/12/15 19:21:20]

Done.

 
   // Called when the overall availability of media sinks has been updated.
   OnSinkAvailabilityUpdated(SinkAvailability availability);