Mojo bindings: Fix typemap includes.

Previously, a header defining a StructTraits specialization could not
include its corresponding generated mojom header, due to the mojom
header including the StructTraits header. This required a brittle set of
forward declarations to correctly avoid this; in practice, this mostly
led to StructTraits that were nominally for a particular variant, but
included the generated mojom header from the default variant.

This CL fixes the issue by splitting typemap headers into
public_headers, which define the native type and are included by the
generated mojom header, and headers which define the StructTraits
specialization for the native type and are only included by the
generated mojom source file, allowing the StructTraits header to include
the generated mojom header.

BUG=596202
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

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

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

24 files changed, 339 insertions, 238 deletions

diff --git a/mojo/mojo_edk_tests.gyp b/mojo/mojo_edk_tests.gyp
index e7a396f..bf7dfd0 100644
--- a/mojo/mojo_edk_tests.gyp
+++ b/mojo/mojo_edk_tests.gyp
@@ -83,9 +83,10 @@
         'public/cpp/bindings/tests/pickled_struct_blink.h',
         'public/cpp/bindings/tests/pickled_struct_chromium.cc',
         'public/cpp/bindings/tests/pickled_struct_chromium.h',
-        # TODO: crbug.com/596202
-        # 'public/cpp/bindings/tests/rect_blink.h',
-        # 'public/cpp/bindings/tests/rect_chromium.h',
+        'public/cpp/bindings/tests/rect_blink.h',
+        'public/cpp/bindings/tests/rect_blink_traits.h',
+        'public/cpp/bindings/tests/rect_chromium.h',
+        'public/cpp/bindings/tests/rect_chromium_traits.h',
         'public/cpp/bindings/tests/request_response_unittest.cc',
         'public/cpp/bindings/tests/router_test_util.cc',
         'public/cpp/bindings/tests/router_test_util.h',
@@ -94,10 +95,12 @@
         'public/cpp/bindings/tests/serialization_warning_unittest.cc',
         'public/cpp/bindings/tests/stl_converters_unittest.cc',
         'public/cpp/bindings/tests/string_unittest.cc',
-        # 'public/cpp/bindings/tests/struct_traits_unittest.cc',
+        'public/cpp/bindings/tests/struct_traits_unittest.cc',
         'public/cpp/bindings/tests/struct_unittest.cc',
         'public/cpp/bindings/tests/struct_with_traits_impl.cc',
         'public/cpp/bindings/tests/struct_with_traits_impl.h',
+        'public/cpp/bindings/tests/struct_with_traits_impl_traits.cc',
+        'public/cpp/bindings/tests/struct_with_traits_impl_traits.h',
         'public/cpp/bindings/tests/sync_method_unittest.cc',
         'public/cpp/bindings/tests/type_conversion_unittest.cc',
         'public/cpp/bindings/tests/union_unittest.cc',
diff --git a/mojo/mojo_public.gyp b/mojo/mojo_public.gyp
index 800adfd..a3cde89 100644
--- a/mojo/mojo_public.gyp
+++ b/mojo/mojo_public.gyp
@@ -11,6 +11,28 @@
       '..',
     ],
   },
+  'variables': {
+    'mojo_public_test_interfaces_mojom_files': [
+      'public/interfaces/bindings/tests/math_calculator.mojom',
+      'public/interfaces/bindings/tests/no_module.mojom',
+      'public/interfaces/bindings/tests/ping_service.mojom',
+      'public/interfaces/bindings/tests/rect.mojom',
+      'public/interfaces/bindings/tests/regression_tests.mojom',
+      'public/interfaces/bindings/tests/sample_factory.mojom',
+      'public/interfaces/bindings/tests/sample_import.mojom',
+      'public/interfaces/bindings/tests/sample_import2.mojom',
+      'public/interfaces/bindings/tests/sample_interfaces.mojom',
+      'public/interfaces/bindings/tests/sample_service.mojom',
+      'public/interfaces/bindings/tests/scoping.mojom',
+      'public/interfaces/bindings/tests/serialization_test_structs.mojom',
+      'public/interfaces/bindings/tests/test_constants.mojom',
+      'public/interfaces/bindings/tests/test_native_types.mojom',
+      'public/interfaces/bindings/tests/test_structs.mojom',
+      'public/interfaces/bindings/tests/test_sync_methods.mojom',
+      'public/interfaces/bindings/tests/test_unions.mojom',
+      'public/interfaces/bindings/tests/validation_test_interfaces.mojom',
+    ]
+  },
   'targets': [
     {
       'target_name': 'mojo_public',
@@ -357,25 +379,7 @@
       'target_name': 'mojo_public_test_interfaces_mojom',
       'type': 'none',
       'variables': {
-        'mojom_files': [
-          'public/interfaces/bindings/tests/math_calculator.mojom',
-          'public/interfaces/bindings/tests/no_module.mojom',
-          'public/interfaces/bindings/tests/ping_service.mojom',
-          'public/interfaces/bindings/tests/rect.mojom',
-          'public/interfaces/bindings/tests/regression_tests.mojom',
-          'public/interfaces/bindings/tests/sample_factory.mojom',
-          'public/interfaces/bindings/tests/sample_import.mojom',
-          'public/interfaces/bindings/tests/sample_import2.mojom',
-          'public/interfaces/bindings/tests/sample_interfaces.mojom',
-          'public/interfaces/bindings/tests/sample_service.mojom',
-          'public/interfaces/bindings/tests/scoping.mojom',
-          'public/interfaces/bindings/tests/serialization_test_structs.mojom',
-          'public/interfaces/bindings/tests/test_constants.mojom',
-          'public/interfaces/bindings/tests/test_structs.mojom',
-          'public/interfaces/bindings/tests/test_sync_methods.mojom',
-          'public/interfaces/bindings/tests/test_unions.mojom',
-          'public/interfaces/bindings/tests/validation_test_interfaces.mojom',
-        ],
+        'mojom_files': '<(mojo_public_test_interfaces_mojom_files)',
       },
       'includes': [ 'mojom_bindings_generator_explicit.gypi' ],
     },
@@ -400,9 +404,7 @@
         'mojom_extra_generator_args': [
           '--typemap', '<(DEPTH)/mojo/public/interfaces/bindings/tests/blink_test.typemap',
         ],
-        'mojom_files': [
-          'public/interfaces/bindings/tests/test_native_types.mojom',
-        ],
+        'mojom_files': '<(mojo_public_test_interfaces_mojom_files)',
       },
       'includes': [ 'mojom_bindings_generator_explicit.gypi' ],
       'dependencies': [
@@ -417,9 +419,7 @@
         'mojom_extra_generator_args': [
           '--typemap', '<(DEPTH)/mojo/public/interfaces/bindings/tests/chromium_test.typemap',
         ],
-        'mojom_files': [
-          'public/interfaces/bindings/tests/test_native_types.mojom',
-        ],
+        'mojom_files': '<(mojo_public_test_interfaces_mojom_files)',
       },
       'includes': [ 'mojom_bindings_generator_explicit.gypi' ],
       'dependencies': [
diff --git a/mojo/public/cpp/bindings/tests/BUILD.gn b/mojo/public/cpp/bindings/tests/BUILD.gn
index 00693e0..29806cf 100644
--- a/mojo/public/cpp/bindings/tests/BUILD.gn
+++ b/mojo/public/cpp/bindings/tests/BUILD.gn
@@ -33,10 +33,10 @@ source_set("tests") {
     "pickled_struct_blink.h",
     "pickled_struct_chromium.cc",
     "pickled_struct_chromium.h",
-
-    # TODO: crbug.com/596202
-    #"rect_blink.h",
-    #"rect_chromium.h",
+    "rect_blink.h",
+    "rect_blink_traits.h",
+    "rect_chromium.h",
+    "rect_chromium_traits.h",
     "request_response_unittest.cc",
     "router_test_util.cc",
     "router_test_util.h",
@@ -45,11 +45,12 @@ source_set("tests") {
     "serialization_warning_unittest.cc",
     "stl_converters_unittest.cc",
     "string_unittest.cc",
-
-    #"struct_traits_unittest.cc",
+    "struct_traits_unittest.cc",
     "struct_unittest.cc",
     "struct_with_traits_impl.cc",
     "struct_with_traits_impl.h",
+    "struct_with_traits_impl_traits.cc",
+    "struct_with_traits_impl_traits.h",
     "sync_method_unittest.cc",
     "type_conversion_unittest.cc",
     "union_unittest.cc",
diff --git a/mojo/public/cpp/bindings/tests/rect_blink.h b/mojo/public/cpp/bindings/tests/rect_blink.h
index b246847..de7a792 100644
--- a/mojo/public/cpp/bindings/tests/rect_blink.h
+++ b/mojo/public/cpp/bindings/tests/rect_blink.h
@@ -6,7 +6,6 @@
 #define MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_BLINK_H_
 
 #include "base/logging.h"
-#include "mojo/public/interfaces/bindings/tests/rect.mojom.h"
 
 namespace mojo {
 namespace test {
@@ -60,26 +59,6 @@ class RectBlink {
 };
 
 }  // namespace test
-
-template <>
-struct StructTraits<test::Rect, test::RectBlink> {
-  static int x(const test::RectBlink& r) { return r.x(); }
-  static int y(const test::RectBlink& r) { return r.y(); }
-  static int width(const test::RectBlink& r) { return r.width(); }
-  static int height(const test::RectBlink& r) { return r.height(); }
-
-  static bool Read(test::Rect::Reader r, test::RectBlink* out) {
-    if (r.x() < 0 || r.y() < 0 || r.width() < 0 || r.height() < 0) {
-      return false;
-    }
-    out->setX(r.x());
-    out->setY(r.y());
-    out->setWidth(r.width());
-    out->setHeight(r.height());
-    return true;
-  }
-};
-
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_BLINK_H_
diff --git a/mojo/public/cpp/bindings/tests/rect_blink_traits.h b/mojo/public/cpp/bindings/tests/rect_blink_traits.h
new file mode 100644
index 0000000..c4e4014
--- /dev/null
+++ b/mojo/public/cpp/bindings/tests/rect_blink_traits.h
@@ -0,0 +1,36 @@
+// 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 MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_BLINK_TRAITS_H_
+#define MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_BLINK_TRAITS_H_
+
+#include "mojo/public/cpp/bindings/struct_traits.h"
+#include "mojo/public/cpp/bindings/tests/rect_blink.h"
+#include "mojo/public/interfaces/bindings/tests/rect.mojom-blink.h"
+
+namespace mojo {
+
+template <>
+struct StructTraits<test::blink::TypemappedRect, test::RectBlink> {
+  static int x(const test::RectBlink& r) { return r.x(); }
+  static int y(const test::RectBlink& r) { return r.y(); }
+  static int width(const test::RectBlink& r) { return r.width(); }
+  static int height(const test::RectBlink& r) { return r.height(); }
+
+  static bool Read(test::blink::TypemappedRect::Reader r,
+                   test::RectBlink* out) {
+    if (r.x() < 0 || r.y() < 0 || r.width() < 0 || r.height() < 0) {
+      return false;
+    }
+    out->setX(r.x());
+    out->setY(r.y());
+    out->setWidth(r.width());
+    out->setHeight(r.height());
+    return true;
+  }
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_BLINK_TRAITS_H_
diff --git a/mojo/public/cpp/bindings/tests/rect_chromium.h b/mojo/public/cpp/bindings/tests/rect_chromium.h
index 569780a..20c4362 100644
--- a/mojo/public/cpp/bindings/tests/rect_chromium.h
+++ b/mojo/public/cpp/bindings/tests/rect_chromium.h
@@ -5,7 +5,7 @@
 #ifndef MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_CHROMIUM_H_
 #define MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_CHROMIUM_H_
 
-#include "mojo/public/interfaces/bindings/tests/rect.mojom.h"
+#include "base/logging.h"
 
 namespace mojo {
 namespace test {
@@ -63,25 +63,6 @@ class RectChromium {
 };
 
 }  // namespace test
-
-template <>
-struct StructTraits<test::Rect, test::RectChromium> {
-  static int x(const test::RectChromium& r) { return r.x(); }
-  static int y(const test::RectChromium& r) { return r.y(); }
-  static int width(const test::RectChromium& r) { return r.width(); }
-  static int height(const test::RectChromium& r) { return r.height(); }
-
-  static bool Read(test::Rect::Reader r, test::RectChromium* out) {
-    if (r.width() < 0 || r.height() < 0)
-      return false;
-    out->set_x(r.x());
-    out->set_y(r.y());
-    out->set_width(r.width());
-    out->set_height(r.height());
-    return true;
-  }
-};
-
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_CHROMIUM_H_
diff --git a/mojo/public/cpp/bindings/tests/rect_chromium_traits.h b/mojo/public/cpp/bindings/tests/rect_chromium_traits.h
new file mode 100644
index 0000000..72c0838
--- /dev/null
+++ b/mojo/public/cpp/bindings/tests/rect_chromium_traits.h
@@ -0,0 +1,35 @@
+// Copyright 2015 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 MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_CHROMIUM_TRAITS_H_
+#define MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_CHROMIUM_TRAITS_H_
+
+#include "mojo/public/cpp/bindings/struct_traits.h"
+#include "mojo/public/cpp/bindings/tests/rect_chromium.h"
+#include "mojo/public/interfaces/bindings/tests/rect.mojom-chromium.h"
+
+namespace mojo {
+
+template <>
+struct StructTraits<test::chromium::TypemappedRect, test::RectChromium> {
+  static int x(const test::RectChromium& r) { return r.x(); }
+  static int y(const test::RectChromium& r) { return r.y(); }
+  static int width(const test::RectChromium& r) { return r.width(); }
+  static int height(const test::RectChromium& r) { return r.height(); }
+
+  static bool Read(test::chromium::TypemappedRect::Reader r,
+                   test::RectChromium* out) {
+    if (r.width() < 0 || r.height() < 0)
+      return false;
+    out->set_x(r.x());
+    out->set_y(r.y());
+    out->set_width(r.width());
+    out->set_height(r.height());
+    return true;
+  }
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_BINDINGS_TESTS_RECT_CHROMIUM_TRAITS_H_
diff --git a/mojo/public/cpp/bindings/tests/struct_with_traits_impl.cc b/mojo/public/cpp/bindings/tests/struct_with_traits_impl.cc
index ee08ecf..7e59d3b 100644
--- a/mojo/public/cpp/bindings/tests/struct_with_traits_impl.cc
+++ b/mojo/public/cpp/bindings/tests/struct_with_traits_impl.cc
@@ -4,8 +4,6 @@
 
 #include "mojo/public/cpp/bindings/tests/struct_with_traits_impl.h"
 
-#include "mojo/public/interfaces/bindings/tests/struct_with_traits.mojom.h"
-
 namespace mojo {
 namespace test {
 
@@ -14,16 +12,4 @@ StructWithTraitsImpl::StructWithTraitsImpl() {}
 StructWithTraitsImpl::~StructWithTraitsImpl() {}
 
 }  // namespace test
-
-// static
-bool StructTraits<test::StructWithTraits, test::StructWithTraitsImpl>::Read(
-    test::StructWithTraits_Reader r,
-    test::StructWithTraitsImpl* out) {
-  out->set_bool(r.f_bool());
-  out->set_uint32(r.f_uint32());
-  out->set_uint64(r.f_uint64());
-  out->set_string(r.f_string().as_string());
-  return true;
-}
-
 }  // namespace mojo
diff --git a/mojo/public/cpp/bindings/tests/struct_with_traits_impl.h b/mojo/public/cpp/bindings/tests/struct_with_traits_impl.h
index d56dae5..b10c8b3 100644
--- a/mojo/public/cpp/bindings/tests/struct_with_traits_impl.h
+++ b/mojo/public/cpp/bindings/tests/struct_with_traits_impl.h
@@ -47,32 +47,6 @@ class StructWithTraitsImpl {
 };
 
 }  // namespace test
-
-template <>
-struct StructTraits<test::StructWithTraits, test::StructWithTraitsImpl> {
-  // Deserialization to test::StructTraitsImpl.
-  static bool Read(test::StructWithTraits_Reader r,
-                   test::StructWithTraitsImpl* out);
-
-  // Fields in test::StructWithTraits.
-  // See src/mojo/public/interfaces/bindings/tests/test_native_types.mojom.
-  static bool f_bool(const test::StructWithTraitsImpl& value) {
-    return value.get_bool();
-  }
-
-  static uint32_t f_uint32(const test::StructWithTraitsImpl& value) {
-    return value.get_uint32();
-  }
-
-  static uint64_t f_uint64(const test::StructWithTraitsImpl& value) {
-    return value.get_uint64();
-  }
-
-  static base::StringPiece f_string(const test::StructWithTraitsImpl& value) {
-    return value.get_string();
-  }
-};
-
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_TESTS_STRUCT_WITH_TRAITS_IMPL_H_
diff --git a/mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.cc b/mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.cc
new file mode 100644
index 0000000..f58ce84
--- /dev/null
+++ b/mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.cc
@@ -0,0 +1,21 @@
+// 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.
+
+#include "mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.h"
+#include "mojo/public/interfaces/bindings/tests/struct_with_traits.mojom.h"
+
+namespace mojo {
+
+// static
+bool StructTraits<test::StructWithTraits, test::StructWithTraitsImpl>::Read(
+    test::StructWithTraits_Reader r,
+    test::StructWithTraitsImpl* out) {
+  out->set_bool(r.f_bool());
+  out->set_uint32(r.f_uint32());
+  out->set_uint64(r.f_uint64());
+  out->set_string(r.f_string().as_string());
+  return true;
+}
+
+}  // namespace mojo
diff --git a/mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.h b/mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.h
new file mode 100644
index 0000000..9433052
--- /dev/null
+++ b/mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.h
@@ -0,0 +1,45 @@
+// 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 MOJO_PUBLIC_CPP_BINDINGS_TESTS_STRUCT_WITH_TRAITS_IMPL_TRAITS_H_
+#define MOJO_PUBLIC_CPP_BINDINGS_TESTS_STRUCT_WITH_TRAITS_IMPL_TRAITS_H_
+
+#include <stdint.h>
+
+#include <string>
+
+#include "base/strings/string_piece.h"
+#include "mojo/public/cpp/bindings/struct_traits.h"
+#include "mojo/public/cpp/bindings/tests/struct_with_traits_impl.h"
+
+namespace mojo {
+
+template <>
+struct StructTraits<test::StructWithTraits, test::StructWithTraitsImpl> {
+  // Deserialization to test::StructTraitsImpl.
+  static bool Read(test::StructWithTraits_Reader r,
+                   test::StructWithTraitsImpl* out);
+
+  // Fields in test::StructWithTraits.
+  // See src/mojo/public/interfaces/bindings/tests/test_native_types.mojom.
+  static bool f_bool(const test::StructWithTraitsImpl& value) {
+    return value.get_bool();
+  }
+
+  static uint32_t f_uint32(const test::StructWithTraitsImpl& value) {
+    return value.get_uint32();
+  }
+
+  static uint64_t f_uint64(const test::StructWithTraitsImpl& value) {
+    return value.get_uint64();
+  }
+
+  static base::StringPiece f_string(const test::StructWithTraitsImpl& value) {
+    return value.get_string();
+  }
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_BINDINGS_TESTS_STRUCT_WITH_TRAITS_IMPL_TRAITS_H_
diff --git a/mojo/public/interfaces/bindings/tests/BUILD.gn b/mojo/public/interfaces/bindings/tests/BUILD.gn
index 6e6d24c..3e4bb64 100644
--- a/mojo/public/interfaces/bindings/tests/BUILD.gn
+++ b/mojo/public/interfaces/bindings/tests/BUILD.gn
@@ -4,26 +4,29 @@
 
 import("../../../tools/bindings/mojom.gni")
 
+test_interfaces_mojom = [
+  "math_calculator.mojom",
+  "no_module.mojom",
+  "ping_service.mojom",
+  "rect.mojom",
+  "regression_tests.mojom",
+  "sample_factory.mojom",
+  "sample_import.mojom",
+  "sample_import2.mojom",
+  "sample_interfaces.mojom",
+  "sample_service.mojom",
+  "scoping.mojom",
+  "serialization_test_structs.mojom",
+  "test_constants.mojom",
+  "test_native_types.mojom",
+  "test_structs.mojom",
+  "test_sync_methods.mojom",
+  "validation_test_interfaces.mojom",
+]
+
 mojom("test_interfaces") {
   testonly = true
-  sources = [
-    "math_calculator.mojom",
-    "no_module.mojom",
-    "ping_service.mojom",
-    "rect.mojom",
-    "regression_tests.mojom",
-    "sample_factory.mojom",
-    "sample_import.mojom",
-    "sample_import2.mojom",
-    "sample_interfaces.mojom",
-    "sample_service.mojom",
-    "scoping.mojom",
-    "serialization_test_structs.mojom",
-    "test_constants.mojom",
-    "test_structs.mojom",
-    "test_sync_methods.mojom",
-    "validation_test_interfaces.mojom",
-  ]
+  sources = test_interfaces_mojom
 }
 
 mojom("test_struct_traits_interfaces") {
@@ -53,16 +56,14 @@ mojom("test_associated_interfaces") {
 }
 
 mojom("versioning_test_service_interfaces") {
-  # FIXME: Dart packaged applications cannot depend on testonly mojoms.
-  # testonly = true
+  testonly = true
   sources = [
     "versioning_test_service.mojom",
   ]
 }
 
 mojom("versioning_test_client_interfaces") {
-  # FIXME: Dart packaged applications cannot depend on testonly mojoms.
-  # testonly = true
+  testonly = true
   sources = [
     "versioning_test_client.mojom",
   ]
@@ -71,31 +72,19 @@ mojom("versioning_test_client_interfaces") {
 mojom("test_interfaces_chromium") {
   testonly = true
 
-  sources = [
-    "test_native_types.mojom",
-  ]
+  sources = test_interfaces_mojom
 
   variant = "chromium"
   typemaps = [ "chromium_test.typemap" ]
-
-  public_deps = [
-    ":test_interfaces",
-  ]
 }
 
 mojom("test_interfaces_blink") {
   testonly = true
 
-  sources = [
-    "test_native_types.mojom",
-  ]
+  sources = test_interfaces_mojom
 
   variant = "blink"
   typemaps = [ "blink_test.typemap" ]
-
-  public_deps = [
-    ":test_interfaces",
-  ]
 }
 
 mojom("test_wtf_types") {
diff --git a/mojo/public/interfaces/bindings/tests/blink_test.typemap b/mojo/public/interfaces/bindings/tests/blink_test.typemap
index 5425ee1..94a7d68 100644
--- a/mojo/public/interfaces/bindings/tests/blink_test.typemap
+++ b/mojo/public/interfaces/bindings/tests/blink_test.typemap
@@ -6,15 +6,18 @@
   "c++": {
     "mojo.test.PickledStruct": {
       "typename": "mojo::test::PickledStructBlink",
-      "headers": [
+      "public_headers": [
         "mojo/public/cpp/bindings/tests/pickled_struct_blink.h"
       ]
+    },
+    "mojo.test.TypemappedRect": {
+      "typename": "mojo::test::RectBlink",
+      "public_headers": [
+        "mojo/public/cpp/bindings/tests/rect_blink.h"
+      ],
+      "traits_headers": [
+        "mojo/public/cpp/bindings/tests/rect_blink_traits.h"
+      ]
     }
-//  "mojo.test.Rect": {
-//    "typename": "mojo::test::RectBlink",
-//    "headers": [
-//      "mojo/public/cpp/bindings/tests/rect_blink.h"
-//    ]
-//  }
   }
 }
diff --git a/mojo/public/interfaces/bindings/tests/chromium_test.typemap b/mojo/public/interfaces/bindings/tests/chromium_test.typemap
index 260826e..57474b1 100644
--- a/mojo/public/interfaces/bindings/tests/chromium_test.typemap
+++ b/mojo/public/interfaces/bindings/tests/chromium_test.typemap
@@ -6,15 +6,18 @@
   "c++": {
     "mojo.test.PickledStruct": {
       "typename": "mojo::test::PickledStructChromium",
-      "headers": [
+      "public_headers": [
         "mojo/public/cpp/bindings/tests/pickled_struct_chromium.h"
       ]
+    },
+    "mojo.test.TypemappedRect": {
+      "typename": "mojo::test::RectChromium",
+      "public_headers": [
+        "mojo/public/cpp/bindings/tests/rect_chromium.h"
+      ],
+      "traits_headers": [
+        "mojo/public/cpp/bindings/tests/rect_chromium_traits.h"
+      ]
     }
-//  "mojo.test.Rect": {
-//    "typename": "mojo::test::RectChromium",
-//    "headers": [
-//      "mojo/public/cpp/bindings/tests/rect_chromium.h"
-//    ]
-//  }
   }
 }
diff --git a/mojo/public/interfaces/bindings/tests/rect.mojom b/mojo/public/interfaces/bindings/tests/rect.mojom
index b8cc977..833c76b 100644
--- a/mojo/public/interfaces/bindings/tests/rect.mojom
+++ b/mojo/public/interfaces/bindings/tests/rect.mojom
@@ -11,3 +11,12 @@ struct Rect {
   int32 width;
   int32 height;
 };
+
+// A copy of Rect that can be typemapped. Arrays of Rect are currently used,
+// which do not support typemapping.
+struct TypemappedRect {
+  int32 x;
+  int32 y;
+  int32 width;
+  int32 height;
+};
diff --git a/mojo/public/interfaces/bindings/tests/struct_with_traits.typemap b/mojo/public/interfaces/bindings/tests/struct_with_traits.typemap
index 244c471..0d6e7b2 100644
--- a/mojo/public/interfaces/bindings/tests/struct_with_traits.typemap
+++ b/mojo/public/interfaces/bindings/tests/struct_with_traits.typemap
@@ -6,8 +6,11 @@
   "c++": {
     "mojo.test.StructWithTraits": {
       "typename": "mojo::test::StructWithTraitsImpl",
-      "headers": [
+      "public_headers": [
         "mojo/public/cpp/bindings/tests/struct_with_traits_impl.h"
+      ],
+      "traits_headers": [
+        "mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.h"
       ]
     }
   }
diff --git a/mojo/public/interfaces/bindings/tests/test_native_types.mojom b/mojo/public/interfaces/bindings/tests/test_native_types.mojom
index 3fcedde..4bfeba1 100644
--- a/mojo/public/interfaces/bindings/tests/test_native_types.mojom
+++ b/mojo/public/interfaces/bindings/tests/test_native_types.mojom
@@ -4,7 +4,7 @@
 
 module mojo.test;
 
-// import "mojo/public/interfaces/bindings/tests/rect.mojom";
+import "mojo/public/interfaces/bindings/tests/rect.mojom";
 
 // Used to verify that structs can be declared with no body in mojom.
 
@@ -23,11 +23,10 @@ interface PicklePasser {
       => (array<array<PickledStruct>> passed);
 };
 
-// TODO: crbug.com/596202
 // Used to verify support for native serialization of mojom-defined structs
 // using StrucTraits with different variants of the Rect type from rect.mojom.
 
-// interface RectService {
-//   AddRect(Rect r);
-//   GetLargestRect() => (Rect largest);
-// };
+interface RectService {
+  AddRect(TypemappedRect r);
+  GetLargestRect() => (TypemappedRect largest);
+};
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/module-internal.h.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/module-internal.h.tmpl
index dfad049..84a0a8b 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/module-internal.h.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/module-internal.h.tmpl
@@ -21,9 +21,6 @@
 #include "mojo/public/cpp/bindings/lib/union_accessor.h"
 #include "mojo/public/cpp/bindings/lib/value_traits.h"
 #include "mojo/public/cpp/bindings/struct_ptr.h"
-{%- for header in extra_headers %}
-#include "{{header}}"
-{%- endfor %}
 
 {%- for import in imports %}
 {%-   if variant %}
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/module.cc.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/module.cc.tmpl
index 4d1dc90..9d40509 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/module.cc.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/module.cc.tmpl
@@ -40,6 +40,10 @@
 #include "mojo/public/cpp/bindings/lib/wtf_serialization.h"
 {%- endif %}
 
+{%- for header in extra_traits_headers %}
+#include "{{header}}"
+{%- endfor %}
+
 {%- for namespace in namespaces_as_array %}
 namespace {{namespace}} {
 {%- endfor %}
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl
index d506e6c..3d0ce78 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl
@@ -49,6 +49,10 @@
 #include "third_party/WebKit/Source/wtf/text/WTFString.h"
 {%- endif %}
 
+{%- for header in extra_public_headers %}
+#include "{{header}}"
+{%- endfor %}
+
 {%- for namespace in namespaces_as_array %}
 namespace {{namespace}} {
 {%- endfor %}
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_declaration.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_declaration.tmpl
index 2e88cb4..16e601d 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_declaration.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_declaration.tmpl
@@ -43,7 +43,7 @@ namespace {{variant}} {
 
 {%- endif %}
 
-{%- if not variant and not struct|is_native_only_kind %}
+{%- if not struct|is_native_only_kind %}
 
 {# NOTE: Generated Reader instances are intentionally cheap to copy and devoid
    of heap allocations. They should stay this way. #}
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
index 3c4c0ef..97c6521 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
@@ -32,7 +32,7 @@ bool Deserialize_(internal::{{struct.name}}_Data* input,
   return success;
 }
 
-{%- if not variant and not struct|is_native_only_kind %}
+{%- if not struct|is_native_only_kind %}
 
 {{struct.name}}_Reader::{{struct.name}}_Reader(
     internal::{{struct.name}}_Data*  data,
diff --git a/mojo/public/tools/bindings/generators/mojom_cpp_generator.py b/mojo/public/tools/bindings/generators/mojom_cpp_generator.py
index 0d28239..d5e3db1 100644
--- a/mojo/public/tools/bindings/generators/mojom_cpp_generator.py
+++ b/mojo/public/tools/bindings/generators/mojom_cpp_generator.py
@@ -51,6 +51,70 @@ _for_blink = False
 _variant = None
 
 
+class _NameFormatter(object):
+  """A formatter for the names of kinds or values."""
+
+  def __init__(self, token, variant):
+    self._token = token
+    self._variant = variant
+
+  def Format(self, separator, prefixed=False, internal=False,
+             include_variant=False, add_same_module_namespaces=False):
+    parts = []
+    if self._ShouldIncludeNamespace(add_same_module_namespaces):
+      if prefixed:
+        parts.append("")
+      parts.extend(self._GetNamespace())
+      if include_variant and self._variant:
+        parts.append(self._variant)
+    parts.extend(self._GetName(internal))
+    return separator.join(parts)
+
+  def FormatForCpp(self, add_same_module_namespaces=False, internal=False):
+    return self.Format(
+        "::", prefixed=True,
+        add_same_module_namespaces=add_same_module_namespaces,
+        internal=internal, include_variant=True)
+
+  def FormatForMojom(self):
+    return self.Format(".", add_same_module_namespaces=True)
+
+  def _MapKindName(self, token, internal):
+    if not internal:
+      return token.name
+    if mojom.IsStructKind(token) and token.native_only:
+      return "mojo::Array_Data<uint8_t>"
+    if (mojom.IsStructKind(token) or mojom.IsUnionKind(token) or
+        mojom.IsInterfaceKind(token) or mojom.IsEnumKind(token)):
+      return token.name + "_Data"
+    return token.name
+
+  def _GetName(self, internal):
+    name = []
+    if internal:
+      name.append("internal")
+    if self._token.parent_kind:
+      name.append(self._MapKindName(self._token.parent_kind, internal))
+    # Both variable and enum constants are constructed like:
+    # Namespace::Struct::CONSTANT_NAME
+    # For enums, CONSTANT_NAME is EnumName::ENUM_VALUE.
+    if isinstance(self._token, mojom.EnumValue):
+      name.extend([self._token.enum.name, self._token.name])
+    else:
+      name.append(self._MapKindName(self._token, internal))
+    return name
+
+  def _ShouldIncludeNamespace(self, add_same_module_namespaces):
+    return add_same_module_namespaces or self._token.imported_from
+
+  def _GetNamespace(self):
+    if self._token.imported_from:
+      return NamespaceToArray(self._token.imported_from["namespace"])
+    elif hasattr(self._token, "module"):
+      return NamespaceToArray(self._token.module.namespace)
+    return []
+
+
 def ConstantValue(constant):
   return ExpressionToText(constant.value, kind=constant.kind)
 
@@ -69,48 +133,15 @@ def DefaultValue(field):
 def NamespaceToArray(namespace):
   return namespace.split(".") if namespace else []
 
-def GetNamePartsForKind(kind, add_same_module_namespaces, add_variant,
-                        internal):
-  def MapKindName_(kind):
-    if not internal:
-      return kind.name
-    if mojom.IsStructKind(kind) and kind.native_only:
-      return "mojo::Array_Data<uint8_t>"
-    if (mojom.IsStructKind(kind) or mojom.IsUnionKind(kind) or
-        mojom.IsInterfaceKind(kind) or mojom.IsEnumKind(kind)):
-      return kind.name + "_Data"
-    return kind.name
-
-  parts = []
-  if kind.imported_from:
-    parts.extend(NamespaceToArray(kind.imported_from["namespace"]))
-    if _variant and add_variant:
-      parts.append(_variant)
-  elif add_same_module_namespaces:
-    if hasattr(kind, "module"):
-      parts.extend(NamespaceToArray(kind.module.namespace))
-    if _variant and add_variant:
-      parts.append(_variant)
-  if internal:
-    parts.append("internal")
-  if kind.parent_kind:
-    parts.append(MapKindName_(kind.parent_kind))
-  parts.append(MapKindName_(kind))
-  return parts
-
 def GetNameForKind(kind, internal=False):
-  parts = GetNamePartsForKind(kind, False, True, internal)
-  return "::".join(parts)
+  return _NameFormatter(kind, _variant).FormatForCpp(internal=internal)
 
 def GetQualifiedNameForKind(kind, internal=False):
-  # Always start with an empty part to force a leading "::" on output.
-  parts = [""]
-  parts.extend(GetNamePartsForKind(kind, True, True, internal))
-  return "::".join(parts)
+  return _NameFormatter(kind, _variant).FormatForCpp(
+      internal=internal, add_same_module_namespaces=True)
 
 def GetFullMojomNameForKind(kind):
-  parts = GetNamePartsForKind(kind, True, False, False)
-  return ".".join(parts)
+  return _NameFormatter(kind, _variant).FormatForMojom()
 
 def IsTypemappedKind(kind):
   return hasattr(kind, "name") and \
@@ -378,21 +409,7 @@ def GetUnionGetterReturnType(kind):
 
 def TranslateConstants(token, kind):
   if isinstance(token, mojom.NamedValue):
-    # Both variable and enum constants are constructed like:
-    # Namespace::Struct::CONSTANT_NAME
-    # For enums, CONSTANT_NAME is ENUM_NAME_ENUM_VALUE.
-    name = []
-    if token.imported_from:
-      name.extend(NamespaceToArray(token.namespace))
-    if _variant:
-      name.append(_variant)
-    if token.parent_kind:
-      name.append(token.parent_kind.name)
-    if isinstance(token, mojom.EnumValue):
-      name.extend([token.enum.name, token.name])
-    else:
-      name.append(token.name)
-    return "::".join(name)
+    return _NameFormatter(token, _variant).FormatForCpp()
 
   if isinstance(token, mojom.BuiltinValue):
     if token.value == "double.INFINITY" or token.value == "float.INFINITY":
@@ -523,10 +540,16 @@ class Generator(generator.Generator):
     "under_to_camel": generator.UnderToCamel,
   }
 
-  def GetExtraHeaders(self):
+  def GetExtraTraitsHeaders(self):
+    extra_headers = set()
+    for entry in self.typemap.itervalues():
+      extra_headers.update(entry.get("traits_headers", []))
+    return list(extra_headers)
+
+  def GetExtraPublicHeaders(self):
     extra_headers = set()
-    for name, entry in self.typemap.iteritems():
-      extra_headers.update(entry["headers"])
+    for entry in self.typemap.itervalues():
+      extra_headers.update(entry.get("public_headers", []))
     return list(extra_headers)
 
   def GetJinjaExports(self):
@@ -541,7 +564,8 @@ class Generator(generator.Generator):
       "unions": self.GetUnions(),
       "interfaces": self.GetInterfaces(),
       "variant": self.variant,
-      "extra_headers": self.GetExtraHeaders(),
+      "extra_traits_headers": self.GetExtraTraitsHeaders(),
+      "extra_public_headers": self.GetExtraPublicHeaders(),
       "for_blink": self.for_blink,
     }
 
diff --git a/mojo/public/tools/bindings/mojom.gni b/mojo/public/tools/bindings/mojom.gni
index fb31385..c510c30 100644
--- a/mojo/public/tools/bindings/mojom.gni
+++ b/mojo/public/tools/bindings/mojom.gni
@@ -209,17 +209,22 @@ template("mojom") {
     all_deps += invoker.public_deps
   }
 
-  group("${target_name}__is_mojom") {
+  if (defined(invoker.variant)) {
+    variant_suffix = "of_variant_${invoker.variant}"
+  } else {
+    variant_suffix = "of_no_variant"
+  }
+  group("${target_name}__is_mojom_${variant_suffix}") {
   }
 
   # Explicitly ensure that all dependencies (invoker.deps and
-  # invoker.public_deps) are mojom targets themselves.
-  group("${target_name}__check_deps_are_all_mojom") {
+  # invoker.public_deps) are mojom targets of the same variant themselves.
+  group("${target_name}__check_deps_are_all_mojom_${variant_suffix}") {
     deps = []
     foreach(d, all_deps) {
       name = get_label_info(d, "label_no_toolchain")
       toolchain = get_label_info(d, "toolchain")
-      deps += [ "${name}__is_mojom(${toolchain})" ]
+      deps += [ "${name}__is_mojom_${variant_suffix}(${toolchain})" ]
     }
   }