[Media Router] Clean up issues related code.

chrome/browser/media/router/mojo/media_router_struct_traits.h

Index: chrome/browser/media/router/mojo/media_router_struct_traits.h
diff --git a/chrome/browser/media/router/mojo/media_router_struct_traits.h b/chrome/browser/media/router/mojo/media_router_struct_traits.h
index 4fe10239cad005abec59e312780d0f09f10ba55d..a1dd128dc53532302c656f8bce475c77a55e692e 100644
--- a/chrome/browser/media/router/mojo/media_router_struct_traits.h
+++ b/chrome/browser/media/router/mojo/media_router_struct_traits.h
@@ -7,12 +7,81 @@
 
 #include <string>
 
+#include "chrome/browser/media/router/issue.h"
 #include "chrome/browser/media/router/mojo/media_router.mojom.h"
 #include "mojo/common/common_custom_types_struct_traits.h"
 
 namespace mojo {
 
 template <>
+struct EnumTraits<media_router::mojom::Issue::ActionType,
+                  media_router::IssueInfo::Action> {
+  static media_router::mojom::Issue::ActionType ToMojom(
+      media_router::IssueInfo::Action action) {
+    switch (action) {
+      case media_router::IssueInfo::Action::DISMISS:
+        return media_router::mojom::Issue::ActionType::DISMISS;
+      case media_router::IssueInfo::Action::LEARN_MORE:
+        return media_router::mojom::Issue::ActionType::LEARN_MORE;
+      default:

[dcheng, 2016/12/13 01:13:33]

One final nit: remove the default, and move the NOTREACHED() / return outside
the switch. That way, the compiler will warn if we add new, unhandled enum
values.

[imcheng, 2016/12/13 17:57:16]

Done. We do have a dummy value that denotes the boundary for the enum (used in
converting an int value coming from WebUI) so I will go ahead and include it in
the switch case as well?

[dcheng, 2016/12/13 18:00:49]

If it's defined to be the same integer value as another enumerator, it's not
necessary to include it (and usually it's omitted, to avoid noise), e.g.
something like:

enum class Foo {
  Bar,
  Baz,
  Max = Baz,
}

[imcheng, 2016/12/13 19:22:47]

You're right. I forgot about that. Done.

+        NOTREACHED() << "Unknown issue action type "
+                     << static_cast<int>(action);
+        return media_router::mojom::Issue::ActionType::DISMISS;
+    }
+  }
+
+  static bool FromMojom(media_router::mojom::Issue::ActionType input,
+                        media_router::IssueInfo::Action* output) {
+    switch (input) {
+      case media_router::mojom::Issue::ActionType::DISMISS:
+        *output = media_router::IssueInfo::Action::DISMISS;
+        return true;
+      case media_router::mojom::Issue::ActionType::LEARN_MORE:
+        *output = media_router::IssueInfo::Action::LEARN_MORE;
+        return true;
+      default:
+        return false;
+    }
+  }
+};
+
+template <>
+struct EnumTraits<media_router::mojom::Issue::Severity,
+                  media_router::IssueInfo::Severity> {
+  static media_router::mojom::Issue::Severity ToMojom(
+      media_router::IssueInfo::Severity severity) {
+    switch (severity) {
+      case media_router::IssueInfo::Severity::FATAL:
+        return media_router::mojom::Issue::Severity::FATAL;
+      case media_router::IssueInfo::Severity::WARNING:
+        return media_router::mojom::Issue::Severity::WARNING;
+      case media_router::IssueInfo::Severity::NOTIFICATION:
+        return media_router::mojom::Issue::Severity::NOTIFICATION;
+      default:
+        NOTREACHED() << "Unknown issue severity " << static_cast<int>(severity);
+        return media_router::mojom::Issue::Severity::WARNING;
+    }
+  }
+
+  static bool FromMojom(media_router::mojom::Issue::Severity input,
+                        media_router::IssueInfo::Severity* output) {
+    switch (input) {
+      case media_router::mojom::Issue::Severity::FATAL:
+        *output = media_router::IssueInfo::Severity::FATAL;
+        return true;
+      case media_router::mojom::Issue::Severity::WARNING:
+        *output = media_router::IssueInfo::Severity::WARNING;
+        return true;
+      case media_router::mojom::Issue::Severity::NOTIFICATION:
+        *output = media_router::IssueInfo::Severity::NOTIFICATION;
+        return true;
+      default:
+        return false;
+    }
+  }
+};
+
+template <>
 struct StructTraits<media_router::mojom::RouteMessageDataView,
                     media_router::RouteMessage> {
   static media_router::mojom::RouteMessage::Type type(
@@ -66,6 +135,52 @@ struct StructTraits<media_router::mojom::RouteMessageDataView,
   }
 };
 
+template <>
+struct StructTraits<media_router::mojom::IssueDataView,
+                    media_router::IssueInfo> {
+  static bool Read(media_router::mojom::IssueDataView data,
+                   media_router::IssueInfo* out);
+
+  static base::Optional<std::string> route_id(
+      const media_router::IssueInfo& issue) {
+    return issue.route_id.empty() ? base::Optional<std::string>()

[dcheng, 2016/12/13 01:13:33]

Let's annotate these getters (since they're unused) with NOTREACHED() for now
(base::Optional isn't /that/ expensive to construct, but it's also not free)

[imcheng, 2016/12/13 17:57:15]

Turns out we do use these in tests when we invoke a mojo proxy with an C++ Issue
object. So I will leave them as is.

[dcheng, 2016/12/13 18:00:49]

I see. Can we just make these Optional in the C++ struct as well? Then we can
just passthrough, and future reviews won't have to worry about this potentially
becoming more expensive.

(It is a fairly minor issue, in the scheme of things, but it would be better to
make this idiomatic)

[imcheng, 2016/12/13 19:22:47]

Ok. Will do in another patch.

+                                  : base::make_optional(issue.route_id);
+  }
+
+  static media_router::IssueInfo::Severity severity(
+      const media_router::IssueInfo& issue) {
+    return issue.severity;
+  }
+
+  static bool is_blocking(const media_router::IssueInfo& issue) {
+    return issue.is_blocking;
+  }
+
+  static std::string title(const media_router::IssueInfo& issue) {
+    return issue.title;
+  }
+
+  static base::Optional<std::string> message(
+      const media_router::IssueInfo& issue) {
+    return issue.message.empty() ? base::Optional<std::string>()
+                                 : base::make_optional(issue.message);
+  }
+
+  static media_router::IssueInfo::Action default_action(
+      const media_router::IssueInfo& issue) {
+    return issue.default_action;
+  }
+
+  static base::Optional<std::vector<media_router::IssueInfo::Action>>
+  secondary_actions(const media_router::IssueInfo& issue) {
+    return issue.secondary_actions;
+  }
+
+  static int32_t help_page_id(const media_router::IssueInfo& issue) {
+    return issue.help_page_id;
+  }
+};
+
 }  // namespace mojo
 
 #endif  // CHROME_BROWSER_MEDIA_ROUTER_MOJO_MEDIA_ROUTER_STRUCT_TRAITS_H_