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

Issue 826070 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 607649



Sign in to add a comment

Mojo JS bindings have various issues with certain parameter names

Project Member Reported by och...@chromium.org, Mar 26 2018

Issue description

Example: https://cs.chromium.org/chromium/src/out/Debug/gen/components/metrics/public/interfaces/call_stack_profile_collector.mojom.js?q=call_stack_profile_collector.mojom.js&sq=package:chromium&dr&l=596

  CallStackProfileCollectorProxy.prototype.collect = function(params, startTimestamp, profiles) {
    var params = new CallStackProfileCollector_Collect_Params();
    params.params = params;
    ...


The collect method has an argument called "params", but the generated bindings also creates a "params" argument which hides it.
 

Comment 1 by och...@chromium.org, Mar 26 2018

Blocking: 607649

Comment 2 by och...@chromium.org, Mar 26 2018

Owner: roc...@chromium.org
Status: Assigned (was: Untriaged)
rockot, do you know who should own this? 

Comment 3 by och...@chromium.org, Mar 26 2018

Summary: Mojo JS bindings generator doesn't work when a method has an argument called "params" (was: Mojo JS bindings generator doesn't work when a method has an argument called "param")

Comment 4 by och...@chromium.org, Apr 10 2018

A couple more issues:

  CdmProxyProxy.prototype.process = function(function, cryptoSessionId, inputData, outputDataSize) {
                                             ^^^^^^^^
SyntaxError: Unexpected token function


  TestValueProxy.prototype.bounceDictionaryValue = function(in) {
                                                            ^^
SyntaxError: Unexpected token in
    at createScript (vm.js:80:10)


  RemoteObjectProxy.prototype.invokeMethod = function(name, arguments) {
                                                            ^^^^^^^^^
SyntaxError: Unexpected eval or arguments in strict mode



Comment 5 by och...@chromium.org, Apr 12 2018

Cc: roc...@chromium.org
Owner: och...@chromium.org
Status: Started (was: Assigned)
Summary: Mojo JS bindings have various issues with certain parameter names (was: Mojo JS bindings generator doesn't work when a method has an argument called "params")
Attempting a fix: https://chromium-review.googlesource.com/c/chromium/src/+/1009347
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 12 2018

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

commit 4443328b475395ac74c06782b14449901e988260
Author: Oliver Chang <ochang@chromium.org>
Date: Thu Apr 12 23:57:34 2018

Mojo JS Bindings: Prevent issues with method parameter names.

- Append '_' to parameter names which are also reserved keywords in
  JavaScript. Struct field names remain unchanged as such keywords
  still work there.

- Rename "params" variable in generated Proxy.prototype.{{method.name}}
  to prevent a parameter named "params" from breaking the bindings.

Bug:  826070 
Change-Id: I29046179929df18e14f63ff4d1cc3a74c1ef71e0
Reviewed-on: https://chromium-review.googlesource.com/1009347
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Oliver Chang <ochang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550420}
[modify] https://crrev.com/4443328b475395ac74c06782b14449901e988260/mojo/public/tools/bindings/generators/js_templates/externs/interface_definition.tmpl
[modify] https://crrev.com/4443328b475395ac74c06782b14449901e988260/mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl
[modify] https://crrev.com/4443328b475395ac74c06782b14449901e988260/mojo/public/tools/bindings/generators/mojom_js_generator.py

Comment 7 by och...@chromium.org, Apr 15 2018

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4443328b475395ac74c06782b14449901e988260

commit 4443328b475395ac74c06782b14449901e988260
Author: Oliver Chang <ochang@chromium.org>
Date: Thu Apr 12 23:57:34 2018

Mojo JS Bindings: Prevent issues with method parameter names.

- Append '_' to parameter names which are also reserved keywords in
  JavaScript. Struct field names remain unchanged as such keywords
  still work there.

- Rename "params" variable in generated Proxy.prototype.{{method.name}}
  to prevent a parameter named "params" from breaking the bindings.

Bug:  826070 
Change-Id: I29046179929df18e14f63ff4d1cc3a74c1ef71e0
Reviewed-on: https://chromium-review.googlesource.com/1009347
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Oliver Chang <ochang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550420}
[modify] https://crrev.com/4443328b475395ac74c06782b14449901e988260/mojo/public/tools/bindings/generators/js_templates/externs/interface_definition.tmpl
[modify] https://crrev.com/4443328b475395ac74c06782b14449901e988260/mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl
[modify] https://crrev.com/4443328b475395ac74c06782b14449901e988260/mojo/public/tools/bindings/generators/mojom_js_generator.py

Sign in to add a comment