From a1107b7873a0b930ff197da68c3eaacd751839d5 Mon Sep 17 00:00:00 2001 From: sammc Date: Wed, 23 Mar 2016 11:51:58 -0700 Subject: 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} --- .../mus/ws/window_manager_factory_service.cc | 1 + content/common/BUILD.gn | 2 +- content/content_common_mojo_bindings.gyp | 1 + gpu/command_buffer/common/mojo.typemap | 36 ++++-- mojo/mojo_edk_tests.gyp | 11 +- mojo/mojo_public.gyp | 50 ++++---- mojo/public/cpp/bindings/tests/BUILD.gn | 13 +- mojo/public/cpp/bindings/tests/rect_blink.h | 21 ---- mojo/public/cpp/bindings/tests/rect_blink_traits.h | 36 ++++++ mojo/public/cpp/bindings/tests/rect_chromium.h | 21 +--- .../cpp/bindings/tests/rect_chromium_traits.h | 35 ++++++ .../cpp/bindings/tests/struct_with_traits_impl.cc | 14 --- .../cpp/bindings/tests/struct_with_traits_impl.h | 26 ---- .../tests/struct_with_traits_impl_traits.cc | 21 ++++ .../tests/struct_with_traits_impl_traits.h | 45 +++++++ mojo/public/interfaces/bindings/tests/BUILD.gn | 61 ++++----- .../interfaces/bindings/tests/blink_test.typemap | 17 +-- .../bindings/tests/chromium_test.typemap | 17 +-- mojo/public/interfaces/bindings/tests/rect.mojom | 9 ++ .../bindings/tests/struct_with_traits.typemap | 5 +- .../bindings/tests/test_native_types.mojom | 11 +- .../cpp_templates/module-internal.h.tmpl | 3 - .../generators/cpp_templates/module.cc.tmpl | 4 + .../generators/cpp_templates/module.h.tmpl | 4 + .../struct_serialization_declaration.tmpl | 2 +- .../struct_serialization_definition.tmpl | 2 +- .../bindings/generators/mojom_cpp_generator.py | 136 ++++++++++++--------- mojo/public/tools/bindings/mojom.gni | 13 +- url/mojo/BUILD.gn | 15 +-- url/mojo/gurl.typemap | 5 +- url/mojo/origin.typemap | 5 +- url/url.gyp | 37 +----- 32 files changed, 378 insertions(+), 301 deletions(-) create mode 100644 mojo/public/cpp/bindings/tests/rect_blink_traits.h create mode 100644 mojo/public/cpp/bindings/tests/rect_chromium_traits.h create mode 100644 mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.cc create mode 100644 mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.h diff --git a/components/mus/ws/window_manager_factory_service.cc b/components/mus/ws/window_manager_factory_service.cc index 847df79..b424560 100644 --- a/components/mus/ws/window_manager_factory_service.cc +++ b/components/mus/ws/window_manager_factory_service.cc @@ -4,6 +4,7 @@ #include "components/mus/ws/window_manager_factory_service.h" +#include "base/bind.h" #include "components/mus/ws/window_manager_factory_registry.h" #include "components/mus/ws/window_tree.h" diff --git a/content/common/BUILD.gn b/content/common/BUILD.gn index e6f54e8..09552c6 100644 --- a/content/common/BUILD.gn +++ b/content/common/BUILD.gn @@ -538,7 +538,6 @@ mojom("mojo_bindings") { import_dirs = [ "//mojo/services" ] typemaps = [ "//url/mojo/origin.typemap" ] - typemap_deps = [ "//url/mojo:url_mojom" ] public_deps = [ "//components/leveldb/public/interfaces", @@ -548,5 +547,6 @@ mojom("mojo_bindings") { "//skia/public/interfaces", "//third_party/WebKit/public:mojo_bindings", "//ui/mojo/geometry:interfaces", + "//url/mojo:url_mojom_origin", ] } diff --git a/content/content_common_mojo_bindings.gyp b/content/content_common_mojo_bindings.gyp index 59f8a7e..5cc2835 100644 --- a/content/content_common_mojo_bindings.gyp +++ b/content/content_common_mojo_bindings.gyp @@ -11,6 +11,7 @@ 'variables': { 'mojom_extra_generator_args': [ '--typemap', '<(DEPTH)/url/mojo/origin.typemap', + '--typemap', '<(DEPTH)/url/mojo/gurl.typemap', ], 'mojom_files': [ # NOTE: Sources duplicated in //content/common/BUILD.gn:mojo_bindings. diff --git a/gpu/command_buffer/common/mojo.typemap b/gpu/command_buffer/common/mojo.typemap index cbc36c5..d412acd 100644 --- a/gpu/command_buffer/common/mojo.typemap +++ b/gpu/command_buffer/common/mojo.typemap @@ -6,43 +6,55 @@ "c++": { "gpu.mojom.Capabilities": { "typename": "gpu::Capabilities", - "headers": [ - "gpu/command_buffer/common/capabilities.h", + "public_headers": [ + "gpu/command_buffer/common/capabilities.h" + ], + "traits_headers": [ "gpu/ipc/common/gpu_command_buffer_traits.h" ] }, "gpu.mojom.CommandBufferState": { "typename": "gpu::CommandBuffer::State", - "headers": [ - "gpu/command_buffer/common/command_buffer.h", + "public_headers": [ + "gpu/command_buffer/common/command_buffer.h" + ], + "traits_headers": [ "gpu/ipc/common/gpu_command_buffer_traits.h" ] }, "gpu.mojom.Mailbox": { "typename": "gpu::Mailbox", - "headers": [ - "gpu/command_buffer/common/mailbox.h", + "public_headers": [ + "gpu/command_buffer/common/mailbox.h" + ], + "traits_headers": [ "gpu/ipc/common/gpu_command_buffer_traits.h" ] }, "gpu.mojom.MailboxHolder": { "typename": "gpu::MailboxHolder", - "headers": [ - "gpu/command_buffer/common/mailbox_holder.h", + "public_headers": [ + "gpu/command_buffer/common/mailbox_holder.h" + ], + "traits_headers": [ "gpu/ipc/common/gpu_command_buffer_traits.h" ] }, "gpu.mojom.SyncToken": { "typename": "gpu::SyncToken", - "headers": [ - "gpu/command_buffer/common/sync_token.h", + "public_headers": [ + "gpu/command_buffer/common/sync_token.h" + ], + "traits_headers": [ "gpu/ipc/common/gpu_command_buffer_traits.h" ] }, "gpu.mojom.ValueState": { "typename": "gpu::ValueState", - "headers": [ - "gpu/command_buffer/common/value_state.h", + "public_headers": [ + "gpu/command_buffer/common/value_state.h" + ], + "traits_headers": [ "gpu/ipc/common/gpu_command_buffer_traits.h" ] } 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 { - 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 { + 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 { - 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 { + 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::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 { - // 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::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 + +#include + +#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 { + // 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> 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" + 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" - 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})" ] } } diff --git a/url/mojo/BUILD.gn b/url/mojo/BUILD.gn index 965bb0e..8a93f29 100644 --- a/url/mojo/BUILD.gn +++ b/url/mojo/BUILD.gn @@ -4,24 +4,12 @@ import("//mojo/public/tools/bindings/mojom.gni") -mojom("url_mojom") { - sources = [ - "origin.mojom", - "url.mojom", - ] -} - mojom("url_mojom_gurl") { sources = [ "url.mojom", ] - variant = "chromium" typemaps = [ "gurl.typemap" ] - - public_deps = [ - ":url_mojom", - ] } mojom("url_mojom_origin") { @@ -29,11 +17,10 @@ mojom("url_mojom_origin") { "origin.mojom", ] - variant = "chromium" typemaps = [ "origin.typemap" ] public_deps = [ - ":url_mojom", + ":url_mojom_gurl", ] } diff --git a/url/mojo/gurl.typemap b/url/mojo/gurl.typemap index 93840c5..ddd755b 100644 --- a/url/mojo/gurl.typemap +++ b/url/mojo/gurl.typemap @@ -6,7 +6,10 @@ "c++": { "url.mojom.Url": { "typename": "GURL", - "headers": [ + "public_headers": [ + "url/gurl.h" + ], + "traits_headers": [ "url/mojo/url_gurl_struct_traits.h" ] } diff --git a/url/mojo/origin.typemap b/url/mojo/origin.typemap index d5e49c3..e785ae4 100644 --- a/url/mojo/origin.typemap +++ b/url/mojo/origin.typemap @@ -6,7 +6,10 @@ "c++": { "url.mojom.Origin": { "typename": "url::Origin", - "headers": [ + "public_headers": [ + "url/origin.h" + ], + "traits_headers": [ "url/mojo/origin_struct_traits.h" ] } diff --git a/url/url.gyp b/url/url.gyp index 08bb9ca..9ab87e1 100644 --- a/url/url.gyp +++ b/url/url.gyp @@ -72,46 +72,16 @@ 'target_name': 'url_interfaces_mojom', 'type': 'none', 'variables': { - 'mojom_files': [ - 'mojo/origin.mojom', - 'mojo/url.mojom', - ], - }, - 'includes': [ '../mojo/mojom_bindings_generator_explicit.gypi' ], - }, - { - 'target_name': 'url_mojom_chromium', - 'type': 'none', - 'variables': { - 'mojom_variant': 'chromium', - 'mojom_extra_generator_args': [ - '--typemap', '<(DEPTH)/url/mojo/gurl.typemap', - ], - 'mojom_files': [ - 'mojo/url.mojom', - ], - }, - 'includes': [ '../mojo/mojom_bindings_generator_explicit.gypi' ], - 'dependencies': [ - 'url_interfaces_mojom', - ], - }, - { - 'target_name': 'origin_mojom_chromium', - 'type': 'none', - 'variables': { - 'mojom_variant': 'chromium', 'mojom_extra_generator_args': [ '--typemap', '<(DEPTH)/url/mojo/origin.typemap', + '--typemap', '<(DEPTH)/url/mojo/gurl.typemap', ], 'mojom_files': [ 'mojo/origin.mojom', + 'mojo/url.mojom', ], }, 'includes': [ '../mojo/mojom_bindings_generator_explicit.gypi' ], - 'dependencies': [ - 'url_interfaces_mojom', - ], }, { 'target_name': 'url_mojom', @@ -139,8 +109,7 @@ 'includes': [ '../mojo/mojom_bindings_generator_explicit.gypi' ], 'dependencies': [ '../mojo/mojo_public.gyp:mojo_cpp_bindings', - 'origin_mojom_chromium', - 'url_mojom_chromium', + 'url_interfaces_mojom', ], }, { -- cgit v1.1