Non-Local Join for Media Router and Presentation API

chrome/browser/media/router/media_router.mojom

 // 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.
 
 module media_router.interfaces;
 
 // Represents an output sink to which media can be routed.
 struct MediaSink {
   enum IconType {
     CAST,
@@ skipping 142 matching lines @@
   SendRouteBinaryMessage(string media_route_id, array<uint8> data)
       => (bool sent);
 
   // Starts querying for sinks capable of displaying |media_source|.
   StartObservingMediaSinks(string media_source);
 
   // Stops querying sinks for |media_source|.
   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|.
   // |messages| returned will contain the batch of messages.
   // |messages| will be empty if |StopListeningForRouteMessages| was invoked.
   // |error| indicates if a permanent error occurred. If true, then subsequent
   // calls will also return with |error| being true.
   ListenForRouteMessages(string route_id) =>
       (array<RouteMessage> messages, bool error);
 
@@ skipping 34 matching lines @@
   // process.
   RegisterMediaRouteProvider(MediaRouteProvider media_router_provider) =>
       (string instance_id);
 
   // Called when the Media Route Manager receives a new list of sinks.
   OnSinksReceived(string media_source, array<MediaSink> sinks);
 
   // 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);
 
   // Called when the state of presentation connected to route |route_id| has
   // changed to |state|.
   OnPresentationConnectionStateChanged(
       string route_id, PresentationConnectionState state);
 };