[Media Router] Add MediaSink subtypes

chrome/browser/media/router/mojo/media_router.mojom

Index: chrome/browser/media/router/mojo/media_router.mojom
diff --git a/chrome/browser/media/router/mojo/media_router.mojom b/chrome/browser/media/router/mojo/media_router.mojom
index 37fc064748de40d2a68f17ebec05798d2c3a4871..41bbede846da68241fd05e675a5fec90fe52cf0d 100644
--- a/chrome/browser/media/router/mojo/media_router.mojom
+++ b/chrome/browser/media/router/mojo/media_router.mojom
@@ -6,6 +6,7 @@ module media_router.mojom;
 
 import "mojo/common/time.mojom";
 import "url/mojo/origin.mojom";
+import "url/mojo/url.mojom";
 
 // Represents an output sink to which media can be routed.
 struct MediaSink {
@@ -29,6 +30,38 @@ struct MediaSink {
   IconType icon_type;
 };
 
+// Represents a sink discovered by Media Router, which is pushed to the
+// MediaRouteProvider. It wraps a MediaSink with additional data required by the
+// MediaRouteProvider to perform additional operations, such as route creation.
+struct TypedMediaSink {

[mark a. foltz, 2017/02/17 02:22:31]

Does this need to be distinct from MediaSink?

If they were merged, we could typemap the combined MediaSink to a C++ object
MediaSinkInternal (replacing TypedMediaSink) and retain the MediaSink struct for
the public Media Router API.  This would reduce the amount of typemapping code
that needs to be maintained.

The downside is that cloud media sinks passed from MRP to the MR would not be
able to fill all the fields.  But it seems like you will need to handle that
case in the MR anyway.

WDYT? 

[imcheng, 2017/02/17 19:56:16]

It seems doing this will weaken the validation provided by Mojo, since we would
have to make all of these fields optional, in order to accommodate the
OnSinksReceived use case.

IMO, the extra type map for TypedMediaSink isn't too bad since it mostly reuses
the MediaSink typemap.

[mark a. foltz, 2017/02/21 17:32:37]

I see.  However we could move all the additional fields introduced in
TypedMediaSink to the type-specific structs.

CastMediaSink
  ip_address (is this needed since the MRP gets the cast channel id?)
  model_name (for feedback?)
  capabilities

DialMediaSink
  ip_address (is this needed since the MRP gets the app_url for launch/stop?)
  model_name (for feedback?)

CloudMediaSink
  domain

This way the same container struct is passed to/from the MRP through both APIs,
fewer typemaps are written, and each sink type defines exactly the fields it
needs (albeit with a couple of duplicates).  I feel this is conceptually cleaner
and less complex in the long run.




 

[zhaobin, 2017/02/22 00:42:08]

Done.

+  MediaSink sink;
+
+  string ip_address;
+
+  // Model name of the sink, if it represents a physical device.
+  string? model_name;
+
+  MediaSinkExtraData extra_data;
+};
+
+union MediaSinkExtraData {
+  DialMediaSink dial_media_sink;
+  CastMediaSink cast_media_sink;
+};
+
+struct DialMediaSink {
+  // Used for DIAL launch
+  url.mojom.Url app_url;
+};
+
+struct CastMediaSink {
+  int32 capabilities;

[mark a. foltz, 2017/02/17 02:22:31]

Can this be defined further?  What do the bit positions mean?

[zhaobin, 2017/02/22 00:42:08]

Done.

+
+  // ID of cast channel opened by Media Router. The ID can be
+  // used to send call chrome.cast.channel.send.
+  int32 cast_channel_id;
+};
+
 // Should be kept in sync with media_route.h.
 struct MediaRoute {
   // The ID of this media route, e.g. "r_PR1O_blkC9dsKp-tb1ti8qurOo".