[Media Router] Add MediaSink subtypes

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

Index: chrome/browser/media/router/discovery/media_sink_internal.cc
diff --git a/chrome/browser/media/router/discovery/media_sink_internal.cc b/chrome/browser/media/router/discovery/media_sink_internal.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a0a84977bae2bfc6077ecabb1b5f0db974b7cf32
--- /dev/null
+++ b/chrome/browser/media/router/discovery/media_sink_internal.cc
@@ -0,0 +1,175 @@
+// 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.
+
+#include "chrome/browser/media/router/discovery/media_sink_internal.h"
+
+#include "base/logging.h"
+#include "base/strings/string_util.h"
+
+namespace media_router {
+
+MediaSinkInternal::MediaSinkInternal(const MediaSink& sink)
+    : sink_(sink), sink_type_(SinkType::GENERIC) {}
+
+MediaSinkInternal::MediaSinkInternal(const MediaSink& sink,
+                                     const DialSinkExtraData& dial_data)
+    : sink_(sink), sink_type_(SinkType::DIAL) {
+  dial_data_.Init(dial_data);
+}
+
+MediaSinkInternal::MediaSinkInternal(const MediaSink& sink,
+                                     const CastSinkExtraData& cast_data)
+    : sink_(sink), sink_type_(SinkType::CAST) {
+  cast_data_.Init(cast_data);
+}
+
+MediaSinkInternal::MediaSinkInternal() : sink_type_(SinkType::GENERIC) {}
+
+MediaSinkInternal::MediaSinkInternal(const MediaSinkInternal& other) {
+  InternalCopyConstructFrom(other);
+}
+
+MediaSinkInternal::~MediaSinkInternal() {
+  InternalCleanup();
+}
+
+MediaSinkInternal& MediaSinkInternal::operator=(
+    const MediaSinkInternal& other) {
+  if (this != &other) {

[mark a. foltz, 2017/03/13 17:47:24]

The conversion to union seems to require all this extra code to do copy and
assignment.  What is the benefit?

[zhaobin, 2017/03/14 01:23:23]

dcheng@ suggests that .mojom should be consistent with c++ object (if we use
union in media_router.mojom, we need to use union in c++ object). Maybe we can
change extra data to optional instead of union in mojom?

+    if (sink_type_ == other.sink_type_) {
+      InternalCopyAssignFrom(other);
+    } else {
+      InternalCleanup();
+      InternalCopyConstructFrom(other);
+    }
+  }
+
+  return *this;
+}
+
+bool MediaSinkInternal::operator==(const MediaSinkInternal& other) const {
+  if (sink_type_ != other.sink_type_)
+    return false;
+
+  if (!(sink_ == other.sink_))

[imcheng, 2017/03/11 00:33:08]

Can this just use MediaSink::Equals?

[mark a. foltz, 2017/03/13 17:47:24]

Equals() methods are discouraged vs. operator==.

https://google.github.io/styleguide/cppguide.html#Operator_Overloading

[zhaobin, 2017/03/14 01:23:23]

Acknowledged.

[zhaobin, 2017/03/14 01:23:23]

MediaSink::Equals only compares IDs. Here we want to compare all fields, use !=
instead...

+    return false;
+
+  switch (sink_type_) {
+    case SinkType::DIAL:
+      return *dial_data_ == *(other.dial_data_);
+    case SinkType::CAST:
+      return *cast_data_ == *(other.cast_data_);
+    case SinkType::GENERIC:
+      return true;
+  }
+  NOTREACHED();

[imcheng, 2017/03/11 00:33:08]

Hmm.. I thought you'd still have to have a return statement following a
NOTREACHED()?

[zhaobin, 2017/03/14 01:23:23]

Done.

+}
+
+void MediaSinkInternal::set_sink(const MediaSink& sink) {
+  sink_ = sink;
+}
+
+void MediaSinkInternal::set_dial_data(const DialSinkExtraData& dial_data) {

[imcheng, 2017/03/11 00:33:08]

Is there a case set_*_data would be called more than once? Maybe we can just
DCHECK_EQ(SinkType::GENERIC, sink_type_) here?

[zhaobin, 2017/03/14 01:23:23]

I think it is valid to call set_dial_data() multiple times: e.g.

MediaSinkInternal media_sink(sink, dial_extra_data);
media_sink.set_dial_data(dial_extra_data2); // DCHECK and crash this does not
seem right.

We may DCHECK sink_type_ is GENERIC or DIAL.

+  InternalCleanup();
+
+  sink_type_ = SinkType::DIAL;
+  dial_data_.Init(dial_data);
+}
+
+const DialSinkExtraData& MediaSinkInternal::dial_data() const {
+  DCHECK(is_dial_sink());
+  return *dial_data_;
+}
+
+void MediaSinkInternal::set_cast_data(const CastSinkExtraData& cast_data) {
+  InternalCleanup();
+
+  sink_type_ = SinkType::CAST;
+  cast_data_.Init(cast_data);
+}
+
+const CastSinkExtraData& MediaSinkInternal::cast_data() const {
+  DCHECK(is_cast_sink());
+  return *cast_data_;
+}
+
+// static
+bool MediaSinkInternal::IsValidSinkId(const std::string& sink_id) {
+  if (sink_id.empty() || !base::IsStringASCII(sink_id)) {
+    DLOG(WARNING) << "Invalid [sink_id]: " << sink_id;
+    return false;
+  }
+
+  return true;
+}
+
+void MediaSinkInternal::InternalCopyAssignFrom(const MediaSinkInternal& other) {
+  sink_ = other.sink_;
+  sink_type_ = other.sink_type_;
+
+  switch (sink_type_) {
+    case SinkType::DIAL:
+      *dial_data_ = *other.dial_data_;
+      return;
+    case SinkType::CAST:
+      *cast_data_ = *other.cast_data_;
+      return;
+    case SinkType::GENERIC:
+      return;
+  }
+  NOTREACHED();
+}
+
+void MediaSinkInternal::InternalCopyConstructFrom(
+    const MediaSinkInternal& other) {
+  sink_ = other.sink_;
+  sink_type_ = other.sink_type_;
+
+  switch (sink_type_) {
+    case SinkType::DIAL:
+      dial_data_.Init(*other.dial_data_);
+      return;
+    case SinkType::CAST:
+      cast_data_.Init(*other.cast_data_);
+      return;
+    case SinkType::GENERIC:
+      return;
+  }
+  NOTREACHED();
+}
+
+void MediaSinkInternal::InternalCleanup() {
+  switch (sink_type_) {
+    case SinkType::DIAL:
+      dial_data_.Destroy();
+      return;
+    case SinkType::CAST:
+      cast_data_.Destroy();
+      return;
+    case SinkType::GENERIC:
+      return;
+  }
+  NOTREACHED();
+}
+
+DialSinkExtraData::DialSinkExtraData() = default;
+DialSinkExtraData::DialSinkExtraData(const DialSinkExtraData& other) = default;
+DialSinkExtraData::~DialSinkExtraData() = default;
+
+bool DialSinkExtraData::operator==(const DialSinkExtraData& other) const {
+  return ip_address == other.ip_address && model_name == other.model_name &&
+         app_url == other.app_url;
+}
+
+CastSinkExtraData::CastSinkExtraData() = default;
+CastSinkExtraData::CastSinkExtraData(const CastSinkExtraData& other) = default;
+CastSinkExtraData::~CastSinkExtraData() = default;
+
+bool CastSinkExtraData::operator==(const CastSinkExtraData& other) const {
+  return ip_address == other.ip_address && model_name == other.model_name &&
+         capabilities == other.capabilities &&
+         cast_channel_id == other.cast_channel_id;
+}
+
+}  // namespace media_router