New issue
Advanced search Search tips

Issue 905533 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Dec 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 912327



Sign in to add a comment

Mojo JS lite bindings should compile with more warnings enabled

Project Member Reported by rockot@google.com, Nov 15

Issue description

We compile the bindings library with ADVANCED_OPTIMIZATIONS but we unintentionally dropped all the default compiler options. This means we're triggering a large number of warnings & errors with the proper options enable.

We should be able to add these options back, i.e. on the js_binary target, make sure |default_closure_args| are added to the closure_flags list.

 
Blocking: 912327
Cc: oksamyt@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 6

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

commit 59b68d8136b3526cf41a42228344bcb52cd73275
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Dec 06 03:01:51 2018

[mojo] Fix JS lite bindings compilation

Adds the default compilation options to the js_binary target which
builds the bindings_lite binary. Fixes all of the miscellaneous resulting
compilation errors. Some major resulting changes:

 - Interface support (stuff which has to depend on generated control
   message bindings) is moved into a separate module to avoid a circular
   dependency between it and the rest of mojo.internal.
 - Enums, Structs, and Unions have had their type specs lifted into a
   $ property. So e.g. an enum e defining FOO, BAR, BAZ will have
   properties e.FOO, e.BAR, e.BAZ, and then e.$ for internal bindings
   use.
 - A few missing core Mojo externs have been filled out.
 - JS lite support library code is now written as if it will always be
   Closure-compiled in a real Closure environment. A simple Python script
   is added to concatenate files and rewrite Closure-style imports and
   exports for environments which do not compile JS.
 - The mojom GN template now emits two new targets, $foo_js_library and
   $foo_js_library_for_compile; the former is a js_library with no
   Closure directives while the latter uses goog.require and goog.provide
   to manage dependencies. They are otherwise equivalent.
 - JS bindings now define everything in a Closure-friendly way within the
   global namespace.

It is still not feasible to use fully compiled (i.e. with
ADVANCED_OPTIMIZATION) generated bindings from non-complied JS due to
unavoidable symbol munging, but for fully compiled JS binaries, they
work just fine.

Bug:  905533 
Change-Id: I0d915188f34dd94b7380a20a80b3d450c367640b
Reviewed-on: https://chromium-review.googlesource.com/c/1340914
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614248}
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/js/BUILD.gn
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/js/bindings_lite.js
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/js/compile_preamble.js
[add] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/js/interface_support.js
[add] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/tools/bindings/concatenate_and_replace_closure_exports.py
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/tools/bindings/generators/js_templates/lite/enum_definition.tmpl
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/tools/bindings/generators/js_templates/lite/interface_definition.tmpl
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/tools/bindings/generators/js_templates/lite/module_definition.tmpl
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/tools/bindings/generators/js_templates/lite/mojom-lite.js.tmpl
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/tools/bindings/generators/js_templates/lite/struct_definition.tmpl
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/tools/bindings/generators/js_templates/lite/union_definition.tmpl
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/tools/bindings/generators/mojom_js_generator.py
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/mojo/public/tools/bindings/mojom.gni
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/third_party/closure_compiler/closure_args.gni
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/third_party/closure_compiler/compile_js.gni
[modify] https://crrev.com/59b68d8136b3526cf41a42228344bcb52cd73275/third_party/closure_compiler/externs/mojo_core.js

Status: Fixed (was: Available)

Sign in to add a comment