New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 632061 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 629566



Sign in to add a comment

mojo: allow Chromium and Blink variants share the same DataView

Project Member Reported by xlai@chromium.org, Jul 27 2016

Issue description

Currently, if Blink and Chromium want to access the same struct_traits.h to do
conversion between mojom types and user custom types, we can only templatize 
StructTraits so that the following function

   static bool Read(typename T::DataView data, UserCustomType* out);

can be applied to both Blink mojom type and Chromium mojom type.

This raises a concern: making all these struct_traits templated will cause code
bloating in headers, as many Read() functions are long and are better separated
in .cc implementation files.

A refactoring in mojo internals to let Chromium and Blink variants share the same
DataView would be helpful for mojo users from Blink. It would be good if the
StructTraits can specialize with DataView.

 

Comment 1 by xlai@chromium.org, Jul 27 2016

Blocking: 629566

Comment 2 by xlai@chromium.org, Aug 4 2016

Yuzhu, it looks like  crbug.com/597125  is somewhat related to this. Both have the same purpose of reducing code bloating due to templating (especially the latest CL landed to that bug).
Hi,

Yes. This will likely help to reduce code bloat and help 597125.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4cbffd67354d2c46f1e9517108328a56cc16ad9

commit a4cbffd67354d2c46f1e9517108328a56cc16ad9
Author: yzshen <yzshen@chromium.org>
Date: Fri Aug 12 18:23:41 2016

Mojo C++ bindings: support generating code shared by different variants.

The refacotring work of extracting shared code will be in follow-up CLs.

BUG= 632061 

Review-Url: https://codereview.chromium.org/2237913004
Cr-Commit-Position: refs/heads/master@{#411703}

[modify] https://crrev.com/a4cbffd67354d2c46f1e9517108328a56cc16ad9/mojo/public/tools/bindings/BUILD.gn
[add] https://crrev.com/a4cbffd67354d2c46f1e9517108328a56cc16ad9/mojo/public/tools/bindings/generators/cpp_templates/module-shared-internal.h.tmpl
[add] https://crrev.com/a4cbffd67354d2c46f1e9517108328a56cc16ad9/mojo/public/tools/bindings/generators/cpp_templates/module-shared.cc.tmpl
[add] https://crrev.com/a4cbffd67354d2c46f1e9517108328a56cc16ad9/mojo/public/tools/bindings/generators/cpp_templates/module-shared.h.tmpl
[modify] https://crrev.com/a4cbffd67354d2c46f1e9517108328a56cc16ad9/mojo/public/tools/bindings/generators/mojom_cpp_generator.py
[modify] https://crrev.com/a4cbffd67354d2c46f1e9517108328a56cc16ad9/mojo/public/tools/bindings/mojom.gni
[modify] https://crrev.com/a4cbffd67354d2c46f1e9517108328a56cc16ad9/mojo/public/tools/bindings/mojom_bindings_generator.py
[modify] https://crrev.com/a4cbffd67354d2c46f1e9517108328a56cc16ad9/mojo/public/tools/bindings/pylib/mojom/generate/generator.py

Comment 5 by xlai@chromium.org, Aug 12 2016

Glad to see the progress! 

How should the struct_traits look like now when it is mapping the chromium and blink mojom type to same custom type? I can use surface_id_struct_traits to experiment it.

Comment 6 by yzshen@chromium.org, Aug 12 2016

Hi, this is only part of the change. :) I have more CLs to come. I will let you know when it is ready for testing. It hopefully will be sometime next week.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1

commit f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1
Author: yzshen <yzshen@chromium.org>
Date: Mon Aug 22 07:47:52 2016

Mojo C++ bindings: extract code shared by different variants.

- enums, including those defined inside structs and interfaces.
- struct and union internal data types (i.e., the wire format definition).

BUG= 632061 

Review-Url: https://codereview.chromium.org/2247083003
Cr-Commit-Position: refs/heads/master@{#413415}

[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/BUILD.gn
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/enum_macros.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/enum_serialization_declaration.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl
[delete] https://crrev.com/5cbfbf15751cfaff33008bd0dd54699b7791158b/mojo/public/tools/bindings/generators/cpp_templates/module-internal.h.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/module-shared-internal.h.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/module-shared.cc.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/module-shared.h.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/module.cc.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/struct_declaration.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/validation_macros.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/generators/mojom_cpp_generator.py
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/mojo/public/tools/bindings/mojom.gni
[modify] https://crrev.com/f0a39ef09ff38dd10fe170c9bad90b7e3d0f4ed1/third_party/WebKit/Source/modules/notifications/NotificationManager.h

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1764bb964000947012673f05dfcebfcbe77a6a63

commit 1764bb964000947012673f05dfcebfcbe77a6a63
Author: yzshen <yzshen@chromium.org>
Date: Wed Aug 24 01:44:12 2016

Mojo C++ bindings: change the first template parameter of StructTraits and UnionTraits.

Previously the type is the auto-generated struct/union type. This CL changes it to the corresponding DataView type.

The reason of doing this? The DataView type is shared by the chromium and blink variants of generated bindings, while the generated struct/union type is not. By changing StructTraits/UnionTraits to be specialized by the DataView type, users will be able to use the same StructTraits specialization in both variants.

BUG= 632061 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2253293002
Cr-Commit-Position: refs/heads/master@{#413931}

[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/begin_frame_args_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/compositor_frame_metadata_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/compositor_frame_metadata_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/compositor_frame_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/compositor_frame_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/filter_operation_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/filter_operations_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/quads_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/quads_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/render_pass_id_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/render_pass_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/render_pass_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/returned_resource_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/selection_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/shared_quad_state_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/surface_id_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/surface_sequence_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/transferable_resource_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/cc/ipc/transferable_resource_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/chrome/browser/media/router/mojo/media_router_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/components/arc/common/OWNERS
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/components/arc/common/app_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/components/arc/common/app_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/components/autofill/content/public/cpp/autofill_types_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/components/autofill/content/public/cpp/autofill_types_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/components/metrics/public/cpp/call_stack_profile_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/components/password_manager/content/public/cpp/credential_manager_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/components/password_manager/content/public/cpp/credential_manager_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/components/translate/content/common/translate_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/components/translate/content/common/translate_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/content/common/bluetooth/web_bluetooth_device_id_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/device/bluetooth/public/interfaces/bluetooth_uuid_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/device/generic_sensor/public/interfaces/sensor_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/device/generic_sensor/public/interfaces/sensor_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/gpu/ipc/common/dx_diag_node_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/gpu/ipc/common/dx_diag_node_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/gpu/ipc/common/gpu_info_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/gpu/ipc/common/gpu_info_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/gpu/ipc/common/mailbox_holder_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/gpu/ipc/common/mailbox_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/gpu/ipc/common/mailbox_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/gpu/ipc/common/sync_token_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/media/mojo/common/pipeline_statistics_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/common/common_custom_types_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/common/common_custom_types_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/cpp/bindings/struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/cpp/bindings/tests/rect_blink_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/cpp/bindings/tests/rect_chromium_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/cpp/bindings/tests/struct_with_traits_impl_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/cpp/bindings/union_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/tools/bindings/generators/cpp_templates/struct_data_view_declaration.tmpl
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_declaration.tmpl
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/tools/bindings/generators/cpp_templates/union_serialization_declaration.tmpl
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/mojo/public/tools/bindings/generators/cpp_templates/union_serialization_definition.tmpl
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/services/shell/public/cpp/capabilities_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/services/shell/public/cpp/identity_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/services/ui/public/interfaces/display/display_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/services/ui/public/interfaces/display/display_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/skia/public/interfaces/bitmap_array_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/skia/public/interfaces/bitmap_skbitmap_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/skia/public/interfaces/bitmap_skbitmap_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/skia/public/interfaces/image_filter_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/third_party/WebKit/Source/platform/mojo/KURLStructTraits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/third_party/WebKit/Source/platform/mojo/SecurityOriginStructTraits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/display/display.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/events/devices/mojo/input_device_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/events/devices/mojo/input_device_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/events/latency_info.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/events/mojo/OWNERS
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/events/mojo/event_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/events/mojo/event_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/events/mojo/latency_info_struct_traits.cc
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/events/mojo/latency_info_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/gfx/geometry/mojo/geometry_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/gfx/mojo/accelerated_widget_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/gfx/mojo/selection_bound_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/ui/gfx/mojo/transform_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/url/mojo/origin_struct_traits.h
[modify] https://crrev.com/1764bb964000947012673f05dfcebfcbe77a6a63/url/mojo/url_gurl_struct_traits.h

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6dc903529d337fbc824b20347be2c4f69197ca8b

commit 6dc903529d337fbc824b20347be2c4f69197ca8b
Author: yzshen <yzshen@chromium.org>
Date: Wed Aug 24 09:12:57 2016

Mojo C++ bindings: share DataView class between chromium and blink variants.

In order to do that, this CL changes all the serialization code to use DataView
classes to indicate the mojom definitions, rather than using the wrapper
classes.

BUG= 632061 

Review-Url: https://codereview.chromium.org/2259283003
Cr-Commit-Position: refs/heads/master@{#414028}

[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/BUILD.gn
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/array_data_view.h
[add] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/interface_data_view.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/array_internal.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/array_serialization.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/bindings_internal.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/control_message_handler.cc
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/control_message_proxy.cc
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/handle_interface_serialization.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/map_serialization.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/native_struct_serialization.cc
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/native_struct_serialization.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/pipe_control_message_handler.cc
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/pipe_control_message_proxy.cc
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/serialization.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/lib/string_serialization.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/map_data_view.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/array_common_test.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/data_view_unittest.cc
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/map_common_test.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/rect_blink.typemap
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/rect_blink_traits.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/rect_chromium.typemap
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/serialization_warning_unittest.cc
[add] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/shared_rect.h
[add] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/shared_rect_traits.h
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/struct_traits_unittest.cc
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/struct_unittest.cc
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/union_unittest.cc
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/cpp/bindings/tests/wtf_types_unittest.cc
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/interfaces/bindings/tests/rect.mojom
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/interfaces/bindings/tests/test_native_types.mojom
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/BUILD.gn
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/module-shared.h.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/module.cc.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/struct_data_view_declaration.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/struct_macros.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_declaration.tmpl
[add] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/struct_traits_declaration.tmpl
[rename] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/struct_traits_definition.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/union_data_view_declaration.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/union_serialization_declaration.tmpl
[add] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/union_traits_declaration.tmpl
[rename] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/union_traits_definition.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/java_templates/data_types_definition.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/js_templates/validation_macros.tmpl
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/mojom_cpp_generator.py
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/generators/mojom_js_generator.py
[modify] https://crrev.com/6dc903529d337fbc824b20347be2c4f69197ca8b/mojo/public/tools/bindings/pylib/mojom/generate/module.py

Status: Fixed (was: Started)
I compared the binary size difference before/after the three CLs listed in comment #7,8,9 above.

Settings: =============================================
Compare (stripped) build of mojo_public_bindings_unittests

Official build with the following flags:
"""is_component_build = false
is_debug = false
use_goma = true
enable_nacl = false
is_official_build = true"""

The test case added in https://codereview.chromium.org/2259283003 is removed to make sure the comparison is fair.

The base chromium git commit is 5cbfbf15751cfaff33008bd0dd54699b7791158b

Results: ===============================================
windows:
before: 2854400
after: 2846208

after - before = -8192

linux:
before: 3466080
after: 3470176

after - before = 4096

I looked at the explain_binary_size_delta result. It seems the difference is because some interface parameter data classes are previously declared/defined directly in the same .cc file in an anonymous namespace. After this change, the declaration is moved to some .h file and definition to the corresponding .cc file.

Because it produces a positive result on Windows and the difference is not too big. I am considering this change mostly neutral binary-size-wise.
Cc: tibell@chromium.org
+CC tibell in case you are interested in the data listed in comment #11.

Sign in to add a comment