New issue
Advanced search Search tips

Issue 894376 link

Starred by 2 users
Status: Fixed
Owner:
rockot@google.com
Closed: Dec 11
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Update JS mojom bindings

Project Member Reported by roc...@chromium.org, Oct 11 
There has been near universal agreement among Chrome-team web developers that JS mojom bindings should not be modeled after the C++ bindings. Apart from this concern, they're also -quite large-, and load time is not an insignificant factor in some scenarios.

As such there is a desire to update the JS bindings to be more ergonomic and less expensive to load. See original discussion doc here[1] and proposal of detailed changes here[2].

For a first pass, the new bindings will omit support for versioning and associated interfaces.

This bug tracks the work to land the new bindings and to eventually land support for aforementioned additional features.

[1] https://docs.google.com/document/d/1WQCzPRSjGLX-7Q9JcC42mCRKAWGPybAm6PT1pVfkBmc/edit
[2] https://docs.google.com/document/d/16OOLpycCQouc6ilGZPBXitkJF0owLXBLEwtCUhWTgss/edit

Comment 1 by roc...@chromium.org, Oct 11 

Add unions to the list of things that need to be done too.

Comment 2 by roc...@chromium.org, Oct 17

Owner: rockot@google.com
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 24 

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

commit 86d36046b14227b3da84850dfec9dc8fae34440a
Author: Ken Rockot <rockot@google.com>
Date: Wed Oct 24 02:50:09 2018

[mojo] Add lite JS mojom bindings

Introduces generation of smaller JS mojom bindings. This
implementation is incomplete, lacking support for unions,
associated interfaces, and versioning. It's still sufficient
for many existing WebUI use cases.

The new bindings also present more ergonomic/webby interfaces for
things. See bug and documents linked therein for details.

The old concatenated mojo_bindings.js sits at around 155 kB, and the
new mojo_bindings_lite.js is around 47 kB uncompiled. With compilation
enabled it's 21 kB, a net reduction of ~86%.

On platforms where the build supports Closure compilation, the common
bindings library is always compiled.

Bug:  894376 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I47dafbca77dccd6a45a829d21c33679440da0a2f
Reviewed-on: https://chromium-review.googlesource.com/c/1278470
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#602229}
[modify] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/BUILD.gn
[modify] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/content/test/BUILD.gn
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/content/test/data/lite_js_test.mojom
[modify] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/js/BUILD.gn
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/js/bindings_lite.js
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/js/compile_preamble.js
[modify] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/js/mojo_bindings_resources.grd
[modify] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/BUILD.gn
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/generators/js_templates/lite/enum_definition.tmpl
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/generators/js_templates/lite/interface_definition.tmpl
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/generators/js_templates/lite/interface_externs.tmpl
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/generators/js_templates/lite/module.externs.tmpl
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/generators/js_templates/lite/module_definition.tmpl
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/generators/js_templates/lite/mojom-lite.js.tmpl
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/generators/js_templates/lite/struct_definition.tmpl
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/generators/js_templates/lite/struct_externs.tmpl
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/generators/js_templates/lite/union_definition.tmpl
[modify] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/generators/mojom_js_generator.py
[modify] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/mojom.gni
[modify] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/mojo/public/tools/bindings/mojom_bindings_generator.py
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/third_party/WebKit/LayoutTests/mojo/bindings-lite.html
[modify] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/third_party/closure_compiler/externs/mojo.js
[add] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/third_party/closure_compiler/externs/mojo_core.js
[modify] https://crrev.com/86d36046b14227b3da84850dfec9dc8fae34440a/third_party/closure_compiler/externs/pending.js
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 1 

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

commit 68638d404b911ac4628124c83d9614e037a5e3b4
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Nov 01 03:30:13 2018

Port omnibox WebUI to new JS mojom bindings

Usage of the old bindings required loading about 185 kB of JS,
including 155 kB from the common bindings library (mojo_bindings.js)
as well as another ~30 kB from the JS generated by omnibox.mojom.

With the new bindings, the common library is only 21 kB compiled
and the generated JS for omnibox.mojom 16 kB (uncompiled, with lots
of Closure annotations). Net mojom-supporting code size reduction of
80%, with room to improve once we compile generated code too.

Also fixes a long-standing bug in the omnibox JS where the selected
page classification value was not being used.

Bug:  894376 
Change-Id: Ie6c5450b7df3b7e7ac29cf68d9a1532c9365046c
Reviewed-on: https://chromium-review.googlesource.com/c/1289701
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604509}
[modify] https://crrev.com/68638d404b911ac4628124c83d9614e037a5e3b4/chrome/browser/browser_resources.grd
[modify] https://crrev.com/68638d404b911ac4628124c83d9614e037a5e3b4/chrome/browser/resources/omnibox/BUILD.gn
[modify] https://crrev.com/68638d404b911ac4628124c83d9614e037a5e3b4/chrome/browser/resources/omnibox/omnibox.html
[modify] https://crrev.com/68638d404b911ac4628124c83d9614e037a5e3b4/chrome/browser/resources/omnibox/omnibox.js
[modify] https://crrev.com/68638d404b911ac4628124c83d9614e037a5e3b4/chrome/browser/resources/omnibox/omnibox_inputs.js
[modify] https://crrev.com/68638d404b911ac4628124c83d9614e037a5e3b4/chrome/browser/ui/webui/omnibox/omnibox_ui.cc
[modify] https://crrev.com/68638d404b911ac4628124c83d9614e037a5e3b4/content/browser/webui/shared_resources_data_source.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 13 

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

commit ebfee307edc9517da8ed9516f655bf994fb00999
Author: Christopher Lam <calamity@chromium.org>
Date: Tue Nov 13 07:09:17 2018

[Mojo WebUI] Move removeListener to CallbackRouters.

This CL makes the CallbackRouters responsible for removing listeners.
This improves ergonomics of removing a list of listeners for different
InterfaceCallbackTargets.

Bug:  894376 
Change-Id: I051857e463257100d667cf78ca8cc2c1d53bcd83
Reviewed-on: https://chromium-review.googlesource.com/c/1301034
Commit-Queue: calamity <calamity@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#607512}
[modify] https://crrev.com/ebfee307edc9517da8ed9516f655bf994fb00999/mojo/public/js/bindings_lite.js
[modify] https://crrev.com/ebfee307edc9517da8ed9516f655bf994fb00999/mojo/public/tools/bindings/generators/js_templates/lite/interface_definition.tmpl
[modify] https://crrev.com/ebfee307edc9517da8ed9516f655bf994fb00999/mojo/public/tools/bindings/generators/js_templates/lite/interface_externs.tmpl
[modify] https://crrev.com/ebfee307edc9517da8ed9516f655bf994fb00999/third_party/closure_compiler/externs/mojo.js

Comment 6 by rockot@google.com, Dec 11

Status: Fixed (was: Started)
Calling this fixed. We'll file separate bugs for additional features, fixes, conversions, etc.
Project Member

Comment 7 by bugdroid1@chromium.org, Today (14 hours ago) 

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

commit 70caea9d2a4aed46d6c75430328bbeb5a28ff377
Author: Ken Rockot <rockot@chromium.org>
Date: Tue Jan 22 18:13:16 2019

Adapt bluetooth-internals new mojom JS bindings

Bug:  894376 
Change-Id: Ia51e42d6c06f0f6f10fa3d4d2f5322a454905902
Reviewed-on: https://chromium-review.googlesource.com/c/1311115
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624832}
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/browser_resources.grd
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/BUILD.gn
[add] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/BUILD.gn
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/adapter_broker.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/bluetooth_internals.html
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/bluetooth_internals.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/characteristic_list.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/descriptor_list.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/device_broker.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/device_collection.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/device_details_page.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/device_table.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/expandable_list.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/object_fieldset.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/service_list.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/sidebar.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/snackbar.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/resources/bluetooth_internals/value_control.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/browser/ui/webui/bluetooth_internals/bluetooth_internals_ui.cc
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/test/data/webui/BUILD.gn
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/chrome/test/data/webui/bluetooth_internals_browsertest.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/mojo/public/js/interface_support.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/mojo/public/tools/bindings/generators/js_templates/lite/interface_definition.tmpl
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/mojo/public/tools/bindings/generators/js_templates/lite/interface_externs.tmpl
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/third_party/closure_compiler/externs/mojo.js
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/ui/webui/resources/js/cr/ui/BUILD.gn
[add] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/ui/webui/resources/js/cr/ui/page_manager/BUILD.gn
[modify] https://crrev.com/70caea9d2a4aed46d6c75430328bbeb5a28ff377/ui/webui/resources/js/cr/ui/page_manager/page_manager.js
►

Sign in to add a comment