mojo: Move meta-API InterfaceProxy methods to a member of InterfaceProxy

Add a "$" member to InterfaceProxies and move all meta-API methods like flushForTesting(), close(), etc. to it. This avoids collisions with interface method names. Bug: 849993 Change-Id: Ic311422419ba51b859fc0bbd1debcfbd5389762e Reviewed-on: https://chromium-review.googlesource.com/c/1468851 Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org> Reviewed-by: Ken Rockot <rockot@google.com> Reviewed-by: calamity <calamity@chromium.org> Cr-Commit-Position: refs/heads/master@{#633394}
qweri0p · Feb 19, 2019 · c34a32d · c34a32d
1 parent 90a07d3
commit c34a32d
Show file tree

Hide file tree

Showing 11 changed files with 94 additions and 57 deletions.
diff --git a/chrome/browser/resources/app_management/browser_proxy.js b/chrome/browser/resources/app_management/browser_proxy.js
@@ -39,7 +39,7 @@ cr.define('app_management', function() {
         this.handler = new appManagement.mojom.PageHandlerProxy();
         const factory = appManagement.mojom.PageHandlerFactory.getProxy();
         factory.createPageHandler(
-            this.callbackRouter.createProxy(), this.handler.createRequest());
+            this.callbackRouter.createProxy(), this.handler.$.createRequest());
       }
     }
   }

diff --git a/chrome/browser/resources/app_management/fake_page_handler.js b/chrome/browser/resources/app_management/fake_page_handler.js
@@ -101,6 +101,12 @@ cr.define('app_management', function() {
 
       /** @type {!Array<App>} */
       this.apps_ = [];
+
+      this.$ = {
+        flushForTesting: async () => {
+          await this.page.$.flushForTesting();
+        }
+      };
     }
 
     async getApps() {
@@ -169,7 +175,7 @@ cr.define('app_management', function() {
      */
     async addApp(id, optConfig) {
       this.page.onAppAdded(FakePageHandler.createApp(id, optConfig));
-      await this.flushForTesting();
+      await this.$.flushForTesting();
     }
 
     /**
@@ -181,11 +187,7 @@ cr.define('app_management', function() {
      */
     async changeApp(id, changes) {
       this.page.onAppChanged(FakePageHandler.createApp(id, changes));
-      await this.flushForTesting();
-    }
-
-    async flushForTesting() {
-      await this.page.flushForTesting();
+      await this.$.flushForTesting();
     }
   }
 

diff --git a/chrome/browser/resources/downloads/browser_proxy.js b/chrome/browser/resources/downloads/browser_proxy.js
@@ -13,7 +13,7 @@ cr.define('downloads', function() {
 
       const factory = downloads.mojom.PageHandlerFactory.getProxy();
       factory.createPageHandler(
-          this.callbackRouter.createProxy(), this.handler.createRequest());
+          this.callbackRouter.createProxy(), this.handler.$.createRequest());
     }
   }
 

diff --git a/chrome/test/data/webui/app_management/metadata_view_test.js b/chrome/test/data/webui/app_management/metadata_view_test.js
@@ -49,7 +49,7 @@ suite('<app-management-metadata-view>', function() {
 
         // Toggle from false to true.
         toggle.click();
-        await fakeHandler.flushForTesting();
+        await fakeHandler.$.flushForTesting();
 
         // Check that the isPinned field of the app has changed.
         expectEquals(OptionalBool.kTrue, metadataView.app_.isPinned);
@@ -59,7 +59,7 @@ suite('<app-management-metadata-view>', function() {
 
         // Toggle from true to false.
         toggle.click();
-        await fakeHandler.flushForTesting();
+        await fakeHandler.$.flushForTesting();
 
         // Check that the isPinned field of the app has changed.
         expectEquals(OptionalBool.kFalse, metadataView.app_.isPinned);

diff --git a/chrome/test/data/webui/app_management/pwa_permission_view_test.js b/chrome/test/data/webui/app_management/pwa_permission_view_test.js
@@ -23,7 +23,7 @@ suite('<app-management-pwa-permission-view>', function() {
 
   async function clickToggle(permissionItemId) {
     getToggleFromPermissionItem(permissionItemId).click();
-    await fakeHandler.flushForTesting();
+    await fakeHandler.$.flushForTesting();
   }
 
   setup(async function() {

diff --git a/chrome/test/data/webui/downloads/manager_tests.js b/chrome/test/data/webui/downloads/manager_tests.js
@@ -31,7 +31,7 @@ suite('manager tests', function() {
                                   sinceString: 'Today',
                                   url: 'a'.repeat(1000),
                                 })]);
-    return pageRouterProxy.flushForTesting().then(() => {
+    return pageRouterProxy.$.flushForTesting().then(() => {
       Polymer.dom.flush();
 
       const item = manager.$$('downloads-item');
@@ -52,20 +52,20 @@ suite('manager tests', function() {
     let download2 = createDownload();
 
     pageRouterProxy.insertItems(0, [download1, download2]);
-    return pageRouterProxy.flushForTesting()
+    return pageRouterProxy.$.flushForTesting()
         .then(() => {
           Polymer.dom.flush();
           assertEquals(1, countDates());
 
           pageRouterProxy.removeItem(0);
-          return pageRouterProxy.flushForTesting();
+          return pageRouterProxy.$.flushForTesting();
         })
         .then(() => {
           Polymer.dom.flush();
           assertEquals(1, countDates());
 
           pageRouterProxy.insertItems(0, [download1]);
-          return pageRouterProxy.flushForTesting();
+          return pageRouterProxy.$.flushForTesting();
         })
         .then(() => {
           Polymer.dom.flush();
@@ -79,7 +79,7 @@ suite('manager tests', function() {
       state: downloads.States.DANGEROUS,
     });
     pageRouterProxy.insertItems(0, [dangerousDownload]);
-    return pageRouterProxy.flushForTesting()
+    return pageRouterProxy.$.flushForTesting()
         .then(() => {
           Polymer.dom.flush();
           assertTrue(!!manager.$$('downloads-item').$$('.dangerous'));
@@ -89,7 +89,7 @@ suite('manager tests', function() {
             state: downloads.States.COMPLETE,
           });
           pageRouterProxy.updateItem(0, safeDownload);
-          return pageRouterProxy.flushForTesting();
+          return pageRouterProxy.$.flushForTesting();
         })
         .then(() => {
           Polymer.dom.flush();
@@ -104,7 +104,7 @@ suite('manager tests', function() {
                                   sinceString: 'Today',
                                   url: 'a'.repeat(1000),
                                 })]);
-    return pageRouterProxy.flushForTesting()
+    return pageRouterProxy.$.flushForTesting()
         .then(() => {
           Polymer.dom.flush();
           const item = manager.$$('downloads-item');

diff --git a/chrome/test/data/webui/downloads/test_support.js b/chrome/test/data/webui/downloads/test_support.js
@@ -37,7 +37,7 @@ class TestDownloadsMojoHandler {
   /** @override */
   async remove(id) {
     this.pageRouterProxy_.removeItem(id);
-    await this.pageRouterProxy_.flushForTesting();
+    await this.pageRouterProxy_.$.flushForTesting();
     this.callTracker_.methodCalled('remove', id);
   }
 

diff --git a/mojo/public/js/interface_support.js b/mojo/public/js/interface_support.js
@@ -151,19 +151,24 @@ mojo.internal.interfaceSupport.ConnectionErrorEventRouter = class {
  * Generic helper used to implement all generated proxy classes. Knows how to
  * serialize requests and deserialize their replies, both according to
  * declarative message structure specs.
+ * @template T
  * @export
  */
 mojo.internal.interfaceSupport.InterfaceProxyBase = class {
   /**
+   * @param {!function(new:T, !MojoHandle)} requestType
    * @param {MojoHandle=} opt_handle The message pipe handle to use as a proxy
    *     endpoint. If null, this object must be bound with bindHandle before
    *     it can be used to send any messages.
    * @public
    */
-  constructor(opt_handle) {
+  constructor(requestType, opt_handle) {
     /** @public {?MojoHandle} */
     this.handle = null;
 
+    /** @private {!function(new:T, !MojoHandle)} */
+    this.requestType_ = requestType;
+
     /** @private {?mojo.internal.interfaceSupport.HandleReader} */
     this.reader_ = null;
 
@@ -186,6 +191,15 @@ mojo.internal.interfaceSupport.InterfaceProxyBase = class {
       this.bindHandle(opt_handle);
   }
 
+  /**
+   * @return {!T}
+   */
+  createRequest() {
+    let {handle0, handle1} = Mojo.createMessagePipe();
+    this.bindHandle(handle0);
+    return new this.requestType_(handle1);
+  }
+
   /**
    * @param {!MojoHandle} handle
    * @export
@@ -232,7 +246,7 @@ mojo.internal.interfaceSupport.InterfaceProxyBase = class {
   sendMessage(ordinal, paramStruct, responseStruct, args) {
     if (!this.handle) {
       throw new Error(
-          'Attempting to use an unbound proxy. Try createRequest() first.')
+          'Attempting to use an unbound proxy. Try $.createRequest() first.')
     }
 
     // The pipe has already been closed, so just drop the message.
@@ -314,6 +328,40 @@ mojo.internal.interfaceSupport.InterfaceProxyBase = class {
   }
 };
 
+/**
+ * Wrapper around mojo.internal.interfaceSupport.InterfaceProxyBase that
+ * exposes the subset of InterfaceProxyBase's method that users are allowed
+ * to use.
+ * @template T
+ * @export
+ */
+mojo.internal.interfaceSupport.InterfaceProxyBaseWrapper = class {
+  /**
+   * @param {!mojo.internal.interfaceSupport.InterfaceProxyBase<T>} proxy
+   * @public
+   */
+  constructor(proxy) {
+    /** @private {!mojo.internal.interfaceSupport.InterfaceProxyBase<T>} */
+    this.proxy_ = proxy;
+  }
+
+  /**
+   * @return {!T}
+   * @export
+   */
+  createRequest() {
+    return this.proxy_.createRequest();
+  }
+
+  /**
+   * @return {!Promise}
+   * @export
+   */
+  flushForTesting() {
+    return this.proxy_.flushForTesting();
+  }
+}
+
 /**
  * Helper used by generated EventRouter types to dispatch incoming interface
  * messages as Event-like things.

diff --git a/mojo/public/tools/bindings/generators/js_templates/lite/interface_definition.tmpl b/mojo/public/tools/bindings/generators/js_templates/lite/interface_definition.tmpl
@@ -32,24 +32,23 @@
 {{module.namespace}}.{{interface.name}}Proxy = class {
   /** @param {MojoHandle=} opt_handle */
   constructor(opt_handle) {
-    /** @private {!mojo.internal.interfaceSupport.InterfaceProxyBase} */
+    /**
+     * @private {!mojo.internal.interfaceSupport.InterfaceProxyBase<!{{module.namespace}}.{{interface.name}}Request>}
+     */
     this.proxy =
-        new mojo.internal.interfaceSupport.InterfaceProxyBase(opt_handle);
+        new mojo.internal.interfaceSupport.InterfaceProxyBase(
+          {{module.namespace}}.{{interface.name}}Request,
+          opt_handle);
+
+    /**
+     * @public {!mojo.internal.interfaceSupport.InterfaceProxyBaseWrapper<!{{module.namespace}}.{{interface.name}}Request>>
+     */
+    this.$ = new mojo.internal.interfaceSupport.InterfaceProxyBaseWrapper(this.proxy);
 
     /** @public {!mojo.internal.interfaceSupport.ConnectionErrorEventRouter} */
     this.onConnectionError = this.proxy.getConnectionErrorEventRouter();
   }
 
-  /**
-   * @return {!{{module.namespace}}.{{interface.name}}Request}
-   * @export
-   */
-  createRequest() {
-    let {handle0, handle1} = Mojo.createMessagePipe();
-    this.proxy.bindHandle(handle0);
-    return new {{module.namespace}}.{{interface.name}}Request(handle1);
-  }
-
 {%- for method in interface.methods -%}
 {%-   set interface_message_id =
           interface.mojom_name ~ "_" ~ method.mojom_name %}
@@ -86,15 +85,6 @@
         ]);
   }
 {%-  endfor %}
-
-
-  /**
-   * @return {!Promise}
-   * @export
-   */
-  flushForTesting() {
-    return this.proxy.flushForTesting();
-  }
 };
 
 /**
@@ -147,7 +137,7 @@
   static getProxy() {
     let proxy = new {{module.namespace}}.{{interface.name}}Proxy;
     Mojo.bindInterface('{{mojom_namespace}}.{{interface.name}}',
-                       proxy.createRequest().handle);
+                       proxy.$.createRequest().handle);
     return proxy;
   }
 
@@ -160,7 +150,7 @@
    */
   createProxy() {
     let proxy = new {{module.namespace}}.{{interface.name}}Proxy;
-    this.target_.bindHandle(proxy.createRequest().handle);
+    this.target_.bindHandle(proxy.$.createRequest().handle);
     return proxy;
   }
 };
@@ -239,7 +229,7 @@
    */
   createProxy() {
     let proxy = new {{module.namespace}}.{{interface.name}}Proxy;
-    this.target_.bindHandle(proxy.createRequest().handle);
+    this.target_.bindHandle(proxy.$.createRequest().handle);
     return proxy;
   }
 

diff --git a/mojo/public/tools/bindings/generators/js_templates/lite/interface_externs.tmpl b/mojo/public/tools/bindings/generators/js_templates/lite/interface_externs.tmpl
@@ -45,20 +45,17 @@ goog.provide('{{module.namespace}}.{{interface.name}}CallbackRouter');
 /** @implements { {{module.namespace}}.{{interface.name}}Interface } */
 {{module.namespace}}.{{interface.name}}Proxy = class {
   constructor() {
+    /** @public {!{
+     *    createRequest: function(): !{{module.namespace}}.{{interface.name}}Request,
+     *    flushForTesting: function(): !Promise
+     *  }}
+     */
+    this.$;
+
     /** @public {!mojo.internal.ConnectionErrorEventRouter} */
     this.onConnectionError;
   }
 {{ generateInterfaceClassBody() }}
-
-  /**
-   * @return {!{{module.namespace}}.{{interface.name}}Request}
-   */
-  createRequest() {}
-
-  /**
-   * @return {Promise}
-   */
-  flushForTesting() {}
 };
 
 {{module.namespace}}.{{interface.name}} = class {

diff --git a/third_party/blink/web_tests/mojo/bindings-lite.html b/third_party/blink/web_tests/mojo/bindings-lite.html
@@ -109,7 +109,7 @@
   let clientRouter = new liteJsTest.mojom.SubinterfaceClientCallbackRouter;
   let subinterfaceProxy = new liteJsTest.mojom.SubinterfaceProxy;
   targetProxy.requestSubinterface(
-      subinterfaceProxy.createRequest(), clientRouter.createProxy());
+      subinterfaceProxy.$.createRequest(), clientRouter.createProxy());
   return new Promise(resolve => {
     clientRouter.didFlush.addListener(values => {
       assert_array_equals(values, kTestNumbers);
@@ -156,7 +156,7 @@
   const kNumPokes = 100;
   for (let i = 0; i < kNumPokes; ++i)
     proxy.poke();
-  return proxy.flushForTesting().then(() => {
+  return proxy.$.flushForTesting().then(() => {
     assert_equals(impl.numPokes, kNumPokes);
   });
 }, 'can use generated flushForTesting API for synchronization in tests');