[Media Router] Clean up issues related code.

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

 // Copyright 2016 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_MOJO_MEDIA_ROUTER_STRUCT_TRAITS_H_
 #define CHROME_BROWSER_MEDIA_ROUTER_MOJO_MEDIA_ROUTER_STRUCT_TRAITS_H_
 
 #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 {
 
+namespace {
+
+media_router::IssueInfo::Action IssueActionFromMojo(
+    media_router::mojom::Issue::ActionType action_type) {
+  switch (action_type) {
+    case media_router::mojom::Issue::ActionType::DISMISS:
+      return media_router::IssueInfo::Action::DISMISS;
+    case media_router::mojom::Issue::ActionType::LEARN_MORE:
+      return media_router::IssueInfo::Action::LEARN_MORE;
+    default:
+      NOTREACHED() << "Unknown issue action type " << action_type;
+      return media_router::IssueInfo::Action::DISMISS;

[mark a. foltz, 2016/11/29 21:27:08]

The Mojo enum reader should validate enum values.  Of course there could be a
missing value in IssueInfo::Action...

[imcheng, 2016/12/05 23:18:22]

IMO, it's good practice to always have default cases to catch the unexpected.

+  }
+}
+
+media_router::mojom::Issue::ActionType IssueActionToMojo(
+    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:
+      NOTREACHED() << "Unknown issue action type " << static_cast<int>(action);
+      return media_router::mojom::Issue::ActionType::DISMISS;
+  }
+}
+
+media_router::IssueInfo::Severity IssueSeverityFromMojo(
+    media_router::mojom::Issue::Severity severity) {
+  switch (severity) {
+    case media_router::mojom::Issue::Severity::FATAL:
+      return media_router::IssueInfo::Severity::FATAL;
+    case media_router::mojom::Issue::Severity::WARNING:
+      return media_router::IssueInfo::Severity::WARNING;
+    case media_router::mojom::Issue::Severity::NOTIFICATION:
+      return media_router::IssueInfo::Severity::NOTIFICATION;
+    default:
+      NOTREACHED() << "Unknown issue severity " << severity;
+      return media_router::IssueInfo::Severity::WARNING;
+  }
+}
+
+}  // namespace
+
 template <>
 struct StructTraits<media_router::mojom::RouteMessageDataView,
                     media_router::RouteMessage> {
   static media_router::mojom::RouteMessage::Type type(
       const media_router::RouteMessage& msg) {
     switch (msg.type) {
       case media_router::RouteMessage::TEXT:
         return media_router::mojom::RouteMessage::Type::TEXT;
       case media_router::RouteMessage::BINARY:
         return media_router::mojom::RouteMessage::Type::BINARY;
@@ skipping 34 matching lines @@
         out->binary = std::move(binary);
         break;
       }
       default:
         return false;
     }
     return true;
   }
 };
 
+template <>
+struct StructTraits<media_router::mojom::IssueDataView,
+                    media_router::IssueInfo> {
+  static bool Read(media_router::mojom::IssueDataView data,
+                   media_router::IssueInfo* out) {
+    if (!data.ReadTitle(&out->title))
+      return false;
+
+    media_router::mojom::Issue::ActionType default_action;
+    if (!data.ReadDefaultAction(&default_action))
+      return false;
+
+    out->default_action = IssueActionFromMojo(default_action);

[mark a. foltz, 2016/11/29 21:27:08]

Are Mojo enums zero indexed?  Can you just static cast here?

[imcheng, 2016/12/05 23:18:19]

I think so. But I would prefer a safer approach that doesn't assume how enum
values are generated in Mojo.

[mark a. foltz, 2016/12/05 23:24:09]

Mojo must provide some strong guarantees about its deserialization API. 
Otherwise how can the browser trust any of the data coming via Mojo?

[imcheng, 2016/12/09 23:28:41]

dcheng@ - can you advise on whether we can do a static_cast? The helper
functions (now EnumTraits class) seem a bit heavyweight just for converting
enums.

+
+    media_router::mojom::Issue::Severity severity;
+    if (!data.ReadSeverity(&severity))
+      return false;
+
+    out->severity = IssueSeverityFromMojo(severity);

[mark a. foltz, 2016/11/29 21:27:07]

Similar question as above.

[imcheng, 2016/12/05 23:18:20]

Acknowledged.

+
+    base::Optional<std::string> message;
+    if (!data.ReadMessage(&message))
+      return false;
+
+    out->message = message.value_or(std::string());
+
+    base::Optional<std::vector<media_router::mojom::Issue::ActionType>>
+        secondary_actions;
+    if (!data.ReadSecondaryActions(&secondary_actions))
+      return false;
+
+    if (secondary_actions.has_value()) {
+      for (const auto action : secondary_actions.value())
+        out->secondary_actions.push_back(IssueActionFromMojo(action));
+    }
+
+    base::Optional<std::string> route_id;
+    if (!data.ReadRouteId(&route_id))
+      return false;
+
+    out->route_id = route_id.value_or(std::string());
+
+    out->is_blocking = data.is_blocking();
+    out->help_page_id = data.help_page_id();
+
+    return true;
+  }
+
+  static base::Optional<std::string> route_id(
+      const media_router::IssueInfo& issue) {
+    return issue.route_id.empty() ? base::Optional<std::string>()
+                                  : base::make_optional(issue.route_id);
+  }
+
+  static media_router::mojom::Issue::Severity severity(
+      const media_router::IssueInfo& issue) {
+    switch (issue.severity) {
+      case media_router::IssueInfo::Severity::FATAL:

[mark a. foltz, 2016/11/29 21:27:08]

Can this be a static cast?

[imcheng, 2016/12/05 23:18:18]

See above.

+        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>(issue.severity);
+        return media_router::mojom::Issue::Severity::WARNING;
+    }
+  }
+
+  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::mojom::Issue::ActionType default_action(
+      const media_router::IssueInfo& issue) {
+    return IssueActionToMojo(issue.default_action);
+  }
+
+  static base::Optional<std::vector<media_router::mojom::Issue::ActionType>>
+  secondary_actions(const media_router::IssueInfo& issue) {
+    if (issue.secondary_actions.empty())
+      return base::Optional<
+          std::vector<media_router::mojom::Issue::ActionType>>();
+
+    std::vector<media_router::mojom::Issue::ActionType> mojo_action_types;
+    for (const auto& action : issue.secondary_actions)
+      mojo_action_types.push_back(IssueActionToMojo(action));
+
+    return base::make_optional(std::move(mojo_action_types));
+  }
+
+  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_