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

Issue 652404 link

Starred by 2 users
Status: Fixed
Owner:
sa...@chromium.org
Closed: Mar 2017
Cc:
█ yzshen@chromium.org
█ tibell@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Mojo C++ Bindings: Typemap headers should be more narrowly scoped to relevant outputs

Project Member Reported by roc...@chromium.org, Oct 3 2016 
Today when a header is emitted by the C++ bindings generator, it contains #includes for the unified set of public_headers from all typemaps in the bindings configuration. This means a very large number unused headers are included by every mojom source module.

We should be able to deduce the transitive closure of typemap headers needed for any given generated file, and include only those ones.

Comment 1 by roc...@chromium.org, Oct 3 2016

Components: Internals>Mojo

Comment 2 by yzshen@chromium.org, Jan 9 2017

Components: -Internals>Mojo Internals>Mojo>Bindings

Comment 3 by sa...@chromium.org, Jan 24 2017

Cc: -sa...@chromium.org
Owner: sa...@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 7 2017 

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

commit 406822fa3d00ff5aeb7afa2099abff3d2672d3c9
Author: sammc <sammc@chromium.org>
Date: Tue Feb 07 22:22:03 2017

Remove unused typemap includes from generated C++ mojo bindings.

This restricts additional headers from typemaps to only types that are
used within the mojom.

BUG= 652404 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/chrome/browser/notifications/non_persistent_notification_handler.cc
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/browser/renderer_host/render_view_host_impl.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/child/child_thread_impl.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/child/indexed_db/webidbdatabase_impl.cc
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/common/indexed_db/indexed_db_struct_traits.cc
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/renderer/presentation/presentation_connection_proxy_unittest.cc
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/renderer/render_frame_impl.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/renderer/render_thread_impl.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/shell/browser/layout_test/layout_test_message_filter.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/shell/browser/layout_test/layout_test_notification_manager.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/content/test/run_all_unittests.cc
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/device/usb/mojo/type_converters.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/media/mojo/services/mojo_video_decoder_service.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/mojo/public/cpp/bindings/lib/wtf_clone_equals_util.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/mojo/public/cpp/bindings/tests/wtf_types_unittest.cc
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/mojo/public/tools/bindings/generators/mojom_cpp_generator.py
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/mojo/public/tools/bindings/pylib/mojom/generate/module.py
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/services/catalog/entry.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/services/service_manager/background/tests/background_service_manager_unittest.cc
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/services/service_manager/public/cpp/lib/service_test.cc
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/services/service_manager/public/cpp/test/run_all_service_tests.cc
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/services/ui/ws/drag_controller.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/services/ui/ws/event_dispatcher.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/third_party/WebKit/Source/platform/mojo/Geometry.typemap
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/third_party/WebKit/Source/platform/mojo/OWNERS
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/third_party/WebKit/public/blink_typemaps.gni
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/third_party/WebKit/public/platform/scheduler/test/mock_renderer_scheduler.h
[modify] https://crrev.com/406822fa3d00ff5aeb7afa2099abff3d2672d3c9/ui/views/mus/mus_client.h
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 13 2017 

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

commit 8e4878fdb12d295c90bbfe58adf3d87016561695
Author: sammc <sammc@chromium.org>
Date: Mon Mar 13 22:45:19 2017

Use depfiles for mojom generation.

Currently, if foo.mojom imports bar.mojom and bar.mojom changes,
foo.mojom will not be regenerated. If the change to bar.mojom changes
which traits headers should be included by foo.mojom.cc by adding or
removing a typemapped type, this can cause compile failures. This CL
fixes this by generating a depfile for each mojom generation step so
changing any mojoms imported by a particular mojom cause the bindings
for that mojom to be regenerated.

BUG= 652404 

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

[modify] https://crrev.com/8e4878fdb12d295c90bbfe58adf3d87016561695/mojo/public/tools/bindings/mojom.gni
[modify] https://crrev.com/8e4878fdb12d295c90bbfe58adf3d87016561695/mojo/public/tools/bindings/mojom_bindings_generator.py

Comment 6 by sa...@chromium.org, Mar 28 2017

Status: Fixed (was: Started)
►

Sign in to add a comment