Address mfoltz's feedback for the Media Router Mojo type converters.

R=mfoltz@chromium.org
BUG=464205

Review URL: https://codereview.chromium.org/1123243005

Cr-Commit-Position: refs/heads/master@{#329667}

chrome/browser/media/router/media_router_type_converters.cc

@@ -4,8 +4,53 @@
 
 #include "chrome/browser/media/router/media_router_type_converters.h"
 
+using media_router::interfaces::IssuePtr;
+using media_router::interfaces::MediaSinkPtr;
+using media_router::interfaces::MediaRoutePtr;
+
 namespace mojo {
 
+// static
+media_router::MediaSink
+TypeConverter<media_router::MediaSink, MediaSinkPtr>::Convert(
+    const MediaSinkPtr& input) {
+  return media_router::MediaSink(input->sink_id, input->name);
+}
+
+// static
+MediaSinkPtr TypeConverter<MediaSinkPtr, media_router::MediaSink>::Convert(
+    const media_router::MediaSink& input) {
+  MediaSinkPtr output(media_router::interfaces::MediaSink::New());
+  output->sink_id = input.id();
+  output->name = input.name();
+  return output.Pass();
+}
+
+// static
+media_router::MediaRoute
+TypeConverter<media_router::MediaRoute, MediaRoutePtr>::Convert(
+    const MediaRoutePtr& input) {
+  return media_router::MediaRoute(
+      input->media_route_id, media_router::MediaSource(input->media_source),
+      input->media_sink.To<media_router::MediaSink>(), input->description,
+      input->is_local);
+}
+
+// static
+MediaRoutePtr TypeConverter<MediaRoutePtr, media_router::MediaRoute>::Convert(
+    const media_router::MediaRoute& input) {
+  MediaRoutePtr output(media_router::interfaces::MediaRoute::New());
+  if (!input.media_source().Empty())
+    output->media_source = input.media_source().id();
+  output->media_route_id = input.media_route_id();
+  output->media_sink =
+      media_router::interfaces::MediaSink::From<media_router::MediaSink>(
+          input.media_sink());
+  output->description = input.description();
+  output->is_local = input.is_local();
+  return output.Pass();
+}
+
 media_router::Issue::Severity IssueSeverityFromMojo(
     media_router::interfaces::Issue::Severity severity) {
   switch (severity) {
@@ -38,4 +83,22 @@ media_router::IssueAction::Type IssueActionTypeFromMojo(
   }
 }
 
+// static
+media_router::Issue TypeConverter<media_router::Issue, IssuePtr>::Convert(
+    const IssuePtr& input) {
+  std::vector<media_router::IssueAction> actions;
+  if (input->secondary_actions) {
+    actions.reserve(input->secondary_actions.size());
+    for (size_t i = 0; i < input->secondary_actions.size(); ++i) {
+      actions.push_back(media_router::IssueAction(
+          IssueActionTypeFromMojo(input->secondary_actions[i])));
+    }
+  }
+  return media_router::Issue(
+      input->title, input->message,
+      media_router::IssueAction(IssueActionTypeFromMojo(input->default_action)),
+      actions, input->route_id, IssueSeverityFromMojo(input->severity),
+      input->is_blocking, input->help_url);
+}
+
 }  // namespace mojo

chrome/browser/media/router/media_router_type_converters.h

@@ -24,21 +24,14 @@ template <>
 struct TypeConverter<media_router::MediaSink,
                      media_router::interfaces::MediaSinkPtr> {
   static media_router::MediaSink Convert(
-      const media_router::interfaces::MediaSinkPtr& input) {
-    return media_router::MediaSink(input->sink_id, input->name);
-  }
+      const media_router::interfaces::MediaSinkPtr& input);
 };
+
 template <>
 struct TypeConverter<media_router::interfaces::MediaSinkPtr,
                      media_router::MediaSink> {
   static media_router::interfaces::MediaSinkPtr Convert(
-      const media_router::MediaSink& input) {
-    media_router::interfaces::MediaSinkPtr output(
-        media_router::interfaces::MediaSink::New());
-    output->sink_id = input.id();
-    output->name = input.name();
-    return output.Pass();
-  }
+      const media_router::MediaSink& input);
 };
 
 // MediaRoute conversion.
@@ -46,30 +39,14 @@ template <>
 struct TypeConverter<media_router::MediaRoute,
                      media_router::interfaces::MediaRoutePtr> {
   static media_router::MediaRoute Convert(
-      const media_router::interfaces::MediaRoutePtr& input) {
-    return media_router::MediaRoute(
-        input->media_route_id, media_router::MediaSource(input->media_source),
-        input->media_sink.To<media_router::MediaSink>(), input->description,
-        input->is_local);
-  }
+      const media_router::interfaces::MediaRoutePtr& input);
 };
+
 template <>
 struct TypeConverter<media_router::interfaces::MediaRoutePtr,
                      media_router::MediaRoute> {
   static media_router::interfaces::MediaRoutePtr Convert(
-      const media_router::MediaRoute& input) {
-    media_router::interfaces::MediaRoutePtr output(
-        media_router::interfaces::MediaRoute::New());
-    if (!input.media_source().Empty())
-      output->media_source = input.media_source().id();
-    output->media_route_id = input.media_route_id();
-    output->media_sink =
-        media_router::interfaces::MediaSink::From<media_router::MediaSink>(
-            input.media_sink());
-    output->description = input.description();
-    output->is_local = input.is_local();
-    return output.Pass();
-  }
+      const media_router::MediaRoute& input);
 };
 
 // Issue conversion.
@@ -82,22 +59,7 @@ media_router::IssueAction::Type IssueActionTypeFromMojo(
 template <>
 struct TypeConverter<media_router::Issue, media_router::interfaces::IssuePtr> {
   static media_router::Issue Convert(
-      const media_router::interfaces::IssuePtr& input) {
-    std::vector<media_router::IssueAction> actions;
-    if (input->secondary_actions) {
-      actions.reserve(input->secondary_actions.size());
-      for (size_t i = 0; i < input->secondary_actions.size(); ++i) {
-        actions.push_back(media_router::IssueAction(
-            IssueActionTypeFromMojo(input->secondary_actions[i])));
-      }
-    }
-    return media_router::Issue(
-        input->title, input->message,
-        media_router::IssueAction(
-            IssueActionTypeFromMojo(input->default_action)),
-        actions, input->route_id, IssueSeverityFromMojo(input->severity),
-        input->is_blocking, input->help_url);
-  }
+      const media_router::interfaces::IssuePtr& input);
 };
 
 }  // namespace mojo

chrome/browser/media/router/media_router_type_converters_unittest.cc

@@ -10,16 +10,7 @@
 
 namespace media_router {
 
-class MediaRouterTypeConvertersTest : public ::testing::Test {
- protected:
-  MediaRouterTypeConvertersTest() {}
-  ~MediaRouterTypeConvertersTest() override {}
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(MediaRouterTypeConvertersTest);
-};
-
-TEST_F(MediaRouterTypeConvertersTest, ConvertMediaSink) {
+TEST(MediaRouterTypeConvertersTest, ConvertMediaSink) {
   MediaSink expected_media_sink("sinkId1", "Sink 1");
   interfaces::MediaSinkPtr expected_mojo_sink(interfaces::MediaSink::New());
   expected_mojo_sink->sink_id = "sinkId1";
@@ -36,10 +27,12 @@ TEST_F(MediaRouterTypeConvertersTest, ConvertMediaSink) {
       media_router::interfaces::MediaSinkPtr>::Convert(mojo_sink);
 
   // Convert MediaSink and back should result in identical object.
+  EXPECT_EQ(expected_media_sink.name(), media_sink.name());
+  EXPECT_EQ(expected_media_sink.id(), media_sink.id());
   EXPECT_TRUE(expected_media_sink.Equals(media_sink));
 }
 
-TEST_F(MediaRouterTypeConvertersTest, ConvertMediaRoute) {
+TEST(MediaRouterTypeConvertersTest, ConvertMediaRoute) {
   MediaSource expected_source(ForTabMediaSource(123));
   MediaRoute expected_media_route("routeId1", expected_source,
                                   MediaSink("sinkId", "sinkName"),
@@ -60,10 +53,20 @@ TEST_F(MediaRouterTypeConvertersTest, ConvertMediaRoute) {
 
   MediaRoute media_route = mojo_route.To<MediaRoute>();
   EXPECT_TRUE(expected_media_route.Equals(media_route));
-  EXPECT_EQ("sinkName", media_route.media_sink().name());
+  EXPECT_EQ(expected_media_route.media_sink().name(),
+            media_route.media_sink().name());
+  EXPECT_EQ(expected_media_route.media_sink().id(),
+            media_route.media_sink().id());
+  EXPECT_EQ(expected_media_route.description(), media_route.description());
+  EXPECT_EQ(expected_media_route.state(), media_route.state());
+  EXPECT_TRUE(
+      expected_media_route.media_source().Equals(media_route.media_source()));
+  EXPECT_EQ(expected_media_route.media_source().id(),
+            media_route.media_source().id());
+  EXPECT_EQ(expected_media_route.is_local(), media_route.is_local());
 }
 
-TEST_F(MediaRouterTypeConvertersTest, ConvertMediaRouteWithoutOptionalFields) {
+TEST(MediaRouterTypeConvertersTest, ConvertMediaRouteWithoutOptionalFields) {
   MediaRoute expected_media_route("routeId1", MediaSource(),
                                   MediaSink("sinkId", "sinkName"),
                                   "Description", false);
@@ -86,7 +89,7 @@ TEST_F(MediaRouterTypeConvertersTest, ConvertMediaRouteWithoutOptionalFields) {
   EXPECT_EQ("sinkName", media_route.media_sink().name());
 }
 
-TEST_F(MediaRouterTypeConvertersTest, ConvertIssue) {
+TEST(MediaRouterTypeConvertersTest, ConvertIssue) {
   interfaces::IssuePtr mojoIssue;
   mojoIssue = interfaces::Issue::New();
   mojoIssue->title = "title";
@@ -133,7 +136,7 @@ TEST_F(MediaRouterTypeConvertersTest, ConvertIssue) {
   EXPECT_FALSE(converted_issue.Equals(expected_issue));
 }
 
-TEST_F(MediaRouterTypeConvertersTest, ConvertIssueWithoutOptionalFields) {
+TEST(MediaRouterTypeConvertersTest, ConvertIssueWithoutOptionalFields) {
   interfaces::IssuePtr mojoIssue;
   mojoIssue = interfaces::Issue::New();
   mojoIssue->title = "title";