Mojo JS Bindings: Eliminate foo$ Stub and Proxy class members

Mojo JS Bindings: Eliminate foo$ and Stub and Proxy class members

Warning: this change will require Chrome changes. See
https://codereview.chromium.org/803173009/#msg6.

Added ProxyBindings and StubBindings methods that expose
bindings-related properties for generated Proxy and Stub classes
without risking name collisions.

Outgoing "in/out" interface valued parameters are now specified as
functions that are applied to the out value.

Note: the leading capital in the Proxy,StubBindings() methods is
unconventional. It seemed OK given their unusual role: they can be
viewed as transformers from "Foo" objects to "FooBindings" objects.

BUG=
R=esprehn@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/8628fc37e38c1986a2fd8ca97f99ce3906b2cc30

[hansmuller1, 2014-12-23 19:22:15]

hansmuller@google.com changed reviewers:
+ esprehn@chromium.org

[hansmuller1, 2014-12-23 19:30:56]

hansmuller@google.com changed reviewers:
+ hansmuller@google.com

[hansmuller1, 2014-12-23 19:30:56]

This change had a bigger impact that I'd anticipated at first. In addition to
eliminating the foo$ properties from the generated classes, I had to stop adding
remote$ and connection$ properties to (user provided) stub implementation
delegate objects. I'm using function valued parameters instead. This has worked
out reasonably nicely.

Completing a comprehensive description of how interface valued parameters, stubs
and proxies, are to be used is still TBD.  The examples/js/README.md has been
updated a little and there's some useful information in the comments for the
Stub,ProxyBindings() functions.

I don't plan to commit this change until after the holidays. Although it
shouldn't impact existing Chromium Mojo code, it would be best to test that
assertion when we're all in.

[esprehn, 2014-12-29 22:00:44]

lgtm, this looks like a great simplification to me.

https://codereview.chromium.org/803173009/diff/1/mojo/public/js/bindings.js
File mojo/public/js/bindings.js (right):

https://codereview.chromium.org/803173009/diff/1/mojo/public/js/bindings.js#n...
mojo/public/js/bindings.js:52: // Mojo interface name collisions.Unless
otherwise specified, the initial
missing space

https://codereview.chromium.org/803173009/diff/1/mojo/public/tools/bindings/g...
File
mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl
(right):

https://codereview.chromium.org/803173009/diff/1/mojo/public/tools/bindings/g...
mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl:5:
function {{interface.name}}Proxy(receiver) {
class {{interface.name}}Proxy extends bindings.ProxyBase {
  constructor(receiver) {
    super(receiver);
  }
}

https://codereview.chromium.org/803173009/diff/1/mojo/public/tools/bindings/g...
mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl:8:
{{interface.name}}Proxy.prototype = Object.create(bindings.ProxyBase.prototype);
Use class, then you don't need this at all.

https://codereview.chromium.org/803173009/diff/1/mojo/public/tools/bindings/g...
mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl:49:
function {{interface.name}}Stub(delegate) {
class {{interface.name}}Stub extends StubBase {
  constructor(delegate) {
   super(delegate);
   this.delegaete$ = delegate;
  }
}

also why do you still need the $ suffix here?

https://codereview.chromium.org/803173009/diff/1/mojo/public/tools/bindings/g...
mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl:53:
{{interface.name}}Stub.prototype = Object.create(bindings.StubBase.prototype);
don't need this with class.

[hansmuller1, 2014-12-29 23:14:11]

Thanks for suggesting this change.

https://codereview.chromium.org/803173009/diff/1/mojo/public/js/bindings.js
File mojo/public/js/bindings.js (right):

https://codereview.chromium.org/803173009/diff/1/mojo/public/js/bindings.js#n...
mojo/public/js/bindings.js:52: // Mojo interface name collisions.Unless
otherwise specified, the initial
On 2014/12/29 22:00:44, esprehn wrote:
> missing space

Done.

https://codereview.chromium.org/803173009/diff/1/mojo/public/tools/bindings/g...
File
mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl
(right):

https://codereview.chromium.org/803173009/diff/1/mojo/public/tools/bindings/g...
mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl:5:
function {{interface.name}}Proxy(receiver) {
On 2014/12/29 22:00:44, esprehn wrote:
> class {{interface.name}}Proxy extends bindings.ProxyBase {
>   constructor(receiver) {
>     super(receiver);
>   }
> }

I hadn't been using classes in the generated code because it needs to run in
Chromium (the application framework classes are only intended to work with the
JS content handler; the Mojo SDK). [per our side discussion ...] It looks like
https://codereview.chromium.org/808433002 ushers in ES6 classes for Chrome, by
default (NICE). I will submit a follow-up CL that uses classes in all of the
Mojo JS bindings.

https://codereview.chromium.org/803173009/diff/1/mojo/public/tools/bindings/g...
mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl:8:
{{interface.name}}Proxy.prototype = Object.create(bindings.ProxyBase.prototype);
On 2014/12/29 22:00:44, esprehn wrote:
> Use class, then you don't need this at all.

Acknowledged.

https://codereview.chromium.org/803173009/diff/1/mojo/public/tools/bindings/g...
mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl:49:
function {{interface.name}}Stub(delegate) {
On 2014/12/29 22:00:44, esprehn wrote:
> class {{interface.name}}Stub extends StubBase {
>   constructor(delegate) {
>    super(delegate);
>    this.delegaete$ = delegate;
>   }
> }
> 
> also why do you still need the $ suffix here?

Oops, that's a mistake.The super class does the delegate initialization, I
should have deleted that line.

https://codereview.chromium.org/803173009/diff/1/mojo/public/tools/bindings/g...
mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl:53:
{{interface.name}}Stub.prototype = Object.create(bindings.StubBase.prototype);
On 2014/12/29 22:00:44, esprehn wrote:
> don't need this with class.

Acknowledged.

[hansmuller1, 2015-01-05 20:25:17]

I've tested this CL with Chrome. Some minor Chrome changes are needed. A diff
follows.

diff --git a/content/browser/webui/web_ui_data_source_impl.cc
b/content/browser/webui/web_ui_data_source_impl.cc
index caa8f9a..bbe54f6 100644
--- a/content/browser/webui/web_ui_data_source_impl.cc
+++ b/content/browser/webui/web_ui_data_source_impl.cc
@@ -31,6 +31,7 @@ WebUIDataSource* WebUIDataSource::AddMojoDataSource(
     const char* path;
     int id;
   } resources[] = {
+    { mojo::kBindingsModuleName, IDR_MOJO_BINDINGS_JS },
     { mojo::kBufferModuleName, IDR_MOJO_BUFFER_JS },
     { mojo::kCodecModuleName, IDR_MOJO_CODEC_JS },
     { mojo::kConnectionModuleName, IDR_MOJO_CONNECTION_JS },
diff --git a/content/browser/webui/web_ui_mojo_browsertest.cc
b/content/browser/webui/web_ui_mojo_browsertest.cc
index d967230..722f2a5 100644
--- a/content/browser/webui/web_ui_mojo_browsertest.cc
+++ b/content/browser/webui/web_ui_mojo_browsertest.cc
@@ -40,7 +40,8 @@ bool got_message = false;
 bool GetResource(const std::string& id,
                  const WebUIDataSource::GotDataCallback& callback) {
   // These are handled by the WebUIDataSource that AddMojoDataSource() creates.
-  if (id == mojo::kBufferModuleName ||
+  if (id == mojo::kBindingsModuleName ||
+      id == mojo::kBufferModuleName ||
       id == mojo::kCodecModuleName ||
       id == mojo::kConnectionModuleName ||
       id == mojo::kConnectorModuleName ||
diff --git a/content/content_resources.grd b/content/content_resources.grd
index 445cc8d..beddb0f 100644
--- a/content/content_resources.grd
+++ b/content/content_resources.grd
@@ -38,6 +38,7 @@
         <include name="IDR_UTILITY_SANDBOX_PROFILE" file="utility/utility.sb"
type="BINDATA" />
       </if>
       <if expr="not is_ios">
+        <include name="IDR_MOJO_BINDINGS_JS"
file="../mojo/public/js/bindings.js" flattenhtml="true" type="BINDATA" />
         <include name="IDR_MOJO_BUFFER_JS" file="../mojo/public/js/buffer.js"
flattenhtml="true" type="BINDATA" />
         <include name="IDR_MOJO_CODEC_JS" file="../mojo/public/js/codec.js"
flattenhtml="true" type="BINDATA" />
         <include name="IDR_MOJO_CONNECTION_JS"
file="../mojo/public/js/connection.js" flattenhtml="true" type="BINDATA" />
diff --git a/extensions/renderer/dispatcher.cc
b/extensions/renderer/dispatcher.cc
index 49c399f..e2b2c8e 100644
--- a/extensions/renderer/dispatcher.cc
+++ b/extensions/renderer/dispatcher.cc
@@ -568,6 +568,8 @@ std::vector<std::pair<std::string, int> >
Dispatcher::GetJsResources() {
                                     
IDR_WEB_VIEW_INTERNAL_CUSTOM_BINDINGS_JS));
   resources.push_back(std::make_pair("denyWebView", IDR_WEB_VIEW_DENY_JS));
   resources.push_back(
+      std::make_pair(mojo::kBindingsModuleName, IDR_MOJO_BINDINGS_JS));
+  resources.push_back(
       std::make_pair(mojo::kBufferModuleName, IDR_MOJO_BUFFER_JS));
   resources.push_back(
       std::make_pair(mojo::kCodecModuleName, IDR_MOJO_CODEC_JS));

[hansmuller, 2015-01-05 20:46:19]

Committed patchset #5 (id:80001) manually as
8628fc37e38c1986a2fd8ca97f99ce3906b2cc30 (presubmit successful).