[Media Router] Add MediaSink subtypes

chrome/browser/media/router/discovery/media_sink_internal.h

+// Copyright 2017 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_DISCOVERY_MEDIA_SINK_INTERNAL_H_
+#define CHROME_BROWSER_MEDIA_ROUTER_DISCOVERY_MEDIA_SINK_INTERNAL_H_
+
+#include "chrome/browser/media/router/media_sink.h"
+#include "url/gurl.h"
+
+namespace media_router {
+
+// Extra data for dial media sink.

[mark a. foltz, 2017/02/28 00:05:29]

nit: DIAL

[zhaobin, 2017/02/28 18:46:29]

Done.

+struct DialSinkExtraData {
+  std::string ip_address;

[mark a. foltz, 2017/02/28 00:05:30]

net::IPAddress would be more accurate, type-wise, and allow you to enforce
validity of data passed from the MRP.

[zhaobin, 2017/02/28 18:46:28]

Done.

+
+  // Model name of the sink
+  std::string model_name;
+
+  // The base URL used for DIAL operations.
+  GURL app_url;
+
+  DialSinkExtraData();
+  DialSinkExtraData(const DialSinkExtraData& other);
+  ~DialSinkExtraData();
+};
+
+// Extra data for cast media sink.
+struct CastSinkExtraData {
+  std::string ip_address;

[mark a. foltz, 2017/02/28 00:05:29]

net::IPAddress

[zhaobin, 2017/02/28 18:46:29]

Done.

+
+  // Model name of the sink
+  std::string model_name;
+
+  // A bit vector representing the capabilities of the sink. The values are
+  // defined in media_router.mojom. The caller must set this value to a valid
+  // capacity set.

[mark a. foltz, 2017/02/28 00:05:29]

Is 0 a valid set?

[zhaobin, 2017/02/28 18:46:28]

Yes.

+  int capabilities = 0;
+
+  // ID of Cast channel opened for the sink. The caller must set this value to a
+  // valid cast_channel_id. The cast_channel_id may change over time as the

[mark a. foltz, 2017/02/28 00:05:29]

Is 0 a valid id?  IIRC the channel_ids start at 1, but I'm not sure.

[zhaobin, 2017/02/28 18:46:29]

No. It will be populated when we successfully open a cast channel. ChannelInfo
default ctor also init channel_id to 0.

+  // browser reconnects to a device.
+  int cast_channel_id = 0;
+
+  CastSinkExtraData();
+  CastSinkExtraData(const CastSinkExtraData& other);
+  ~CastSinkExtraData();
+};
+
+// Represents a media sink discovered by MediaSinkService. It is used by
+// MediaSinkService to push MediaSinks with extra data to the
+// MediaRouteProvider, and it is not exposed to users of MediaRouter.
+class MediaSinkInternal {
+ public:
+  explicit MediaSinkInternal(const MediaSink& sink);

[mark a. foltz, 2017/02/28 00:05:29]

Can you add a one-line comment explaining when you would use each of these
ctors?

[zhaobin, 2017/02/28 18:46:29]

Done.

+  MediaSinkInternal(const MediaSink& sink, const DialSinkExtraData& dial_data);
+  MediaSinkInternal(const MediaSink& sink, const CastSinkExtraData& cast_data);
+  MediaSinkInternal();

[mark a. foltz, 2017/02/28 00:05:29]

nit: Can you put this ctor first in the list?

[zhaobin, 2017/02/28 18:46:29]

Done.

+  MediaSinkInternal(const MediaSinkInternal& other);
+  ~MediaSinkInternal();
+
+  MediaSinkInternal& operator=(const MediaSinkInternal& other);
+
+  // Set |sink_| and invalidate extra data.
+  void set_sink(const MediaSink& sink);
+  const MediaSink& sink() const { return sink_; }
+
+  void set_dial_data(const DialSinkExtraData& dial_data);

[mark a. foltz, 2017/02/28 00:05:29]

Add comments explaining that the getters must only be called if the sink is a
DIAL/Cast sink.

[zhaobin, 2017/02/28 18:46:28]

Done.

+  const DialSinkExtraData& dial_data() const;
+  void set_cast_data(const CastSinkExtraData& cast_data);
+  const CastSinkExtraData& cast_data() const;
+
+  bool is_dial_sink() const { return dial_data_ && !cast_data_; }
+  bool is_cast_sink() const { return cast_data_ && !dial_data_; }
+
+  static bool IsValidSinkId(const std::string& sink_id);
+  static bool IsValidIPAddress(const std::string& ip_address);

[mark a. foltz, 2017/02/28 00:05:29]

See comment above about using net::IPAddress.

[zhaobin, 2017/02/28 18:46:28]

Done.

+
+ private:
+  MediaSink sink_;
+
+  // Set if sink is Dial sink.

[mark a. foltz, 2017/02/28 00:05:29]

nit: DIAL

[zhaobin, 2017/02/28 18:46:28]

Done.

+  base::Optional<DialSinkExtraData> dial_data_;
+
+  // Set if sink is Cast sink.
+  base::Optional<CastSinkExtraData> cast_data_;
+};
+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_MEDIA_ROUTER_DISCOVERY_MEDIA_SINK_INTERNAL_H_