Added message interface between chrome://media-app and chrome://media-app-guest

Todate, we've only passed a single, simple message to open a file from media-app
to media-app-guest. To better support our use-cases we need a bidirectional pipe,
and some abstractions around the dispatching of messages.

This CL introduces a MessagePipe class to encapsulate the communication channel
and manage dispatch of requests and responses between the privileged and
unprivileged JS contexts in chrome://media-app. It still requires explicit
definitions of what messages either side can send and receive but allows for
these messages to be defined and used with less friction.

Bug: 996088, 1040328
Change-Id: I6179a5d176d74f0f37fa1809363e87a0ed2976e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2049404
Commit-Queue: Zain Afzal <zafzal@google.com>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746214}

chromeos/components/media_app_ui/BUILD.gn

@@ -69,6 +69,7 @@ source_set("browser_test_support") {
     "test/driver.js",
     "test/driver_api.js",
     "test/guest_query_receiver.js",
+    "test/receiver_api.js",
   ]
 }
 
@@ -98,6 +99,7 @@ js_type_check("closure_compile_tests") {
     ":test_driver_api_js",
     ":test_driver_js",
     ":test_guest_query_receiver_js",
+    ":test_receiver_api_js",
   ]
 }
 
@@ -106,10 +108,19 @@ js_library("test_driver_api_js") {
   sources = [ "test/driver_api.js" ]
 }
 
+js_library("test_receiver_api_js") {
+  testonly = true
+  sources = [ "test/receiver_api.js" ]
+}
+
 js_library("test_guest_query_receiver_js") {
   testonly = true
   sources = [ "test/guest_query_receiver.js" ]
-  deps = [ ":test_driver_api_js" ]
+  deps = [
+    ":test_driver_api_js",
+    ":test_receiver_api_js",
+    "//chromeos/components/media_app_ui/resources/js:message_pipe",
+  ]
 }
 
 js_library("test_driver_js") {
@@ -117,6 +128,7 @@ js_library("test_driver_js") {
   sources = [ "test/driver.js" ]
   deps = [
     ":test_driver_api_js",
+    "//chromeos/components/media_app_ui/resources/js:message_pipe",
     "//ui/webui/resources/js:assert",
   ]
 }

chromeos/components/media_app_ui/media_app_guest_ui.cc

@@ -21,6 +21,7 @@ content::WebUIDataSource* MediaAppGuestUI::CreateDataSource() {
   // Add resources from chromeos_media_app_resources.pak.
   source->AddResourcePath("app.html", IDR_MEDIA_APP_APP_HTML);
   source->AddResourcePath("receiver.js", IDR_MEDIA_APP_RECEIVER_JS);
+  source->AddResourcePath("message_pipe.js", IDR_MEDIA_APP_MESSAGE_PIPE_JS);
 
   // Add resources from chromeos_media_app_bundle_resources.pak that are also
   // needed for mocks. If enable_cros_media_app = true, then these calls will

chromeos/components/media_app_ui/media_app_ui.cc

@@ -31,6 +31,7 @@ content::WebUIDataSource* CreateHostDataSource() {
                           IDR_MEDIA_APP_MOJO_API_BOOTSTRAP_JS);
   source->AddResourcePath("media_app.mojom-lite.js",
                           IDR_MEDIA_APP_MEDIA_APP_MOJOM_JS);
+  source->AddResourcePath("message_pipe.js", IDR_MEDIA_APP_MESSAGE_PIPE_JS);
 
   // Add resources from chromeos_media_app_bundle_resources.pak.
   source->AddResourcePath("system_assets/app_icon_256.png",

chromeos/components/media_app_ui/resources/app.html

@@ -10,4 +10,5 @@
   }
 </style>
 <script src="/js/app_main.js"></script>
+<script src="/message_pipe.js"></script>
 <script src="/receiver.js"></script>

chromeos/components/media_app_ui/resources/index.html

@@ -61,4 +61,5 @@
   <div id="prev-container" class="navigation-button"></div>
   <div id="next-container" class="navigation-button"></div>
 </div>
+<script src="/message_pipe.js"></script>
 <script src="/launch.js"></script>

chromeos/components/media_app_ui/resources/js/BUILD.gn

@@ -19,6 +19,7 @@ js_type_check("closure_compile") {
                   ]
   deps = [
     ":launch",
+    ":message_pipe",
     ":mojo_api_bootstrap",
     ":receiver",
   ]
@@ -29,10 +30,15 @@ js_library("launch") {
     "web_app_file_handling.externs.js",
     "dom_draft.externs.js",
   ]
+  deps = [ ":message_pipe" ]
+}
+
+js_library("message_pipe") {
 }
 
 js_library("receiver") {
   externs_list = [ "media_app.externs.js" ]
+  deps = [ ":message_pipe" ]
 }
 
 js_library("mojo_api_bootstrap") {

chromeos/components/media_app_ui/resources/js/launch.js

@@ -16,27 +16,16 @@ const currentFiles = [];
  */
 let entryIndex = -1;
 
-/**
- * Helper to call postMessage on the guest frame.
- *
- * @param {Object} message
- */
-function postToGuestWindow(message) {
-  const guest =
-      /** @type{HTMLIFrameElement} */ (document.querySelector('iframe'));
-  // The next line should probably be passing a transfer argument, but that
-  // causes Chrome to send a "null" message. The transfer seems to work without
-  // the third argument (but inefficiently, perhaps).
-  guest.contentWindow.postMessage(message, 'chrome://media-app-guest');
-}
+/** A pipe through which we can send messages to the guest frame. */
+const guestMessagePipe = new MessagePipe('chrome://media-app-guest');
 
 /**
  * Loads a file in the guest.
  *
  * @param {File} file
  */
-async function loadFile(file) {
-  postToGuestWindow({'file': file});
+function loadFile(file) {
+  guestMessagePipe.sendMessage('file', {'file': file});
 }
 
 /**

chromeos/components/media_app_ui/resources/js/receiver.js

@@ -15,6 +15,16 @@ class SingleArrayBufferFileList {
   }
 }
 
+/** A pipe through which we can send messages to the parent frame. */
+const parentMessagePipe = new MessagePipe('chrome://media-app', window.parent);
+
+parentMessagePipe.registerHandler('file', (message) => {
+  const fileMessage = /** @type mediaApp.MessageEventData */ (message);
+  if (fileMessage.file) {
+    loadFile(fileMessage.file);
+  }
+})
+
 /**
  * Loads files associated with a message received from the host.
  * @param {!File} file
@@ -30,32 +40,12 @@ async function loadFile(file) {
   const app = /** @type {?mediaApp.ClientApi} */ (
       document.querySelector('backlight-app'));
   if (app) {
-    app.loadFiles(fileList);
+    await app.loadFiles(fileList);
   } else {
     window.customLaunchData = {files: fileList};
   }
 }
 
-function receiveMessage(/** Event */ e) {
-  const event = /** @type{MessageEvent<Object>} */ (e);
-  if (event.origin !== 'chrome://media-app') {
-    return;
-  }
-
-  // First ensure the message is our MessageEventData type, then act on it
-  // appropriately. Note test messages won't have a file (and are not handled by
-  // this listener), so it's currently sufficient to just check for `file`.
-  if ('file' in event.data) {
-    const message =
-        /** @type{MessageEvent<mediaApp.MessageEventData>}*/ (event);
-    if (message.data.file) {
-      loadFile(message.data.file);
-    } else {
-      console.error('Unknown message:', message);
-    }
-  }
-}
-
 // Attempting to execute chooseFileSystemEntries is guaranteed to result in a
 // SecurityError due to the fact that we are running in a unprivileged iframe.
 // Note, we can not do window.chooseFileSystemEntries due to the fact that
@@ -63,5 +53,3 @@ function receiveMessage(/** Event */ e) {
 // TODO(crbug/1040328): Remove this when we have a polyfill that allows us to
 // talk to the privileged frame.
 window['chooseFileSystemEntries'] = null;
-
-window.addEventListener('message', receiveMessage, false);

chromeos/components/media_app_ui/resources/media_app_resources.grd

@@ -19,6 +19,9 @@
       <!-- Unprivileged guest contents. -->
       <include name="IDR_MEDIA_APP_APP_HTML" file="app.html" type="BINDATA" />
       <include name="IDR_MEDIA_APP_RECEIVER_JS" file="js/receiver.js" type="BINDATA" />
+
+      <!-- Common contents. -->
+      <include name="IDR_MEDIA_APP_MESSAGE_PIPE_JS" file="js/message_pipe.js" type="BINDATA" />
     </includes>
   </release>
 </grit>

chromeos/components/media_app_ui/test/driver.js

@@ -4,51 +4,6 @@
 
 /** Host-side of web-driver like controller for sandboxed guest frames. */
 class GuestDriver {
-  /** @param {string} origin */
-  constructor(origin) {
-    this.origin = origin;
-
-    /** @type {Array<MessageEvent<TestMessageResponseData>>} */
-    this.testMessageQueue = [];
-
-    /** @type {?function(MessageEvent<TestMessageResponseData>)} */
-    this.testMessageWaiter = null;
-
-    this.messageListener = (/** Event */ event) => {
-      const testEvent =
-          /** @type{MessageEvent<TestMessageResponseData>} */ (event);
-
-      console.log('Event from guest: ' + JSON.stringify(testEvent.data));
-      if (this.testMessageWaiter) {
-        this.testMessageWaiter(testEvent);
-        this.testMessageWaiter = null;
-      } else {
-        this.testMessageQueue.push(testEvent);
-      }
-    };
-
-    window.addEventListener('message', this.messageListener);
-  }
-
-  tearDown() {
-    window.removeEventListener('message', this.messageListener);
-  }
-
-  /**
-   * Returns the next message from the guest.
-   * @return {Promise<MessageEvent<TestMessageResponseData>>}
-   */
-  popTestMessage() {
-    if (this.testMessageQueue.length > 0) {
-      const front = this.testMessageQueue[0];
-      this.testMessageQueue.shift();
-      return Promise.resolve(front);
-    }
-    return new Promise(resolve => {
-      this.testMessageWaiter = resolve;
-    });
-  }
-
   /**
    * Sends a query to the guest that repeatedly runs a query selector until
    * it returns an element.
@@ -60,13 +15,12 @@ class GuestDriver {
    *   tagName if unspecified.
    */
   async waitForElementInGuest(query, opt_property, opt_commands = {}) {
-    const frame = assertInstanceof(
-        document.querySelector(`iframe[src^="${this.origin}"]`),
-        HTMLIFrameElement);
     /** @type{TestMessageQueryData} */
     const message = {testQuery: query, property: opt_property};
-    frame.contentWindow.postMessage({...message, ...opt_commands}, this.origin);
-    const event = await this.popTestMessage();
-    return event.data.testQueryResult;
+
+    const result = /** @type{TestMessageResponseData} */ (
+        await guestMessagePipe.sendMessage(
+            'test', {...message, ...opt_commands}));
+    return result.testQueryResult;
   }
 }

chromeos/components/media_app_ui/test/driver_api.js

@@ -13,3 +13,6 @@ var TestMessageResponseData;
  *     requestFullscreen: (boolean|undefined)}}
  */
 var TestMessageQueryData;
+
+/** @type {MessagePipe} */
+var guestMessagePipe;
\ No newline at end of file

chromeos/components/media_app_ui/test/guest_query_receiver.js

@@ -40,12 +40,13 @@ async function waitForNode(query, opt_path) {
 /**
  * Acts on received TestMessageQueryData.
  *
- * @param {MessageEvent<TestMessageQueryData>} event
+ * @param {TestMessageQueryData} data
+ * @return {!Promise<TestMessageResponseData>}
  */
-async function runTestQuery(event) {
-  const data = event.data;
+async function runTestQuery(data) {
   const element = await waitForNode(data.testQuery, data.pathToRoot || []);
   let result = element.tagName;
+
   if (data.property) {
     result = JSON.stringify(element[data.property]);
   } else if (data.requestFullscreen) {
@@ -57,17 +58,23 @@ async function runTestQuery(event) {
     }
   }
 
-  const response = {testQueryResult: result};
-  event.source.postMessage(response, event.origin);
-}
-
-function receiveTestMessage(/** Event */ e) {
-  const event = /** @type{MessageEvent<TestMessageQueryData>} */ (e);
-  if (event.data.testQuery) {
-    runTestQuery(event);
-  }
+  return {testQueryResult: result};
 }
 
-window.addEventListener('message', receiveTestMessage, false);
+// Wait until dom content has loaded to make sure receiver.js has been
+// parsed and executed.
+window.addEventListener('DOMContentLoaded', () => {
+  parentMessagePipe.registerHandler('test', (data) => {
+    return runTestQuery(/** @type{TestMessageQueryData} */ (data));
+  });
+  // Turn off error rethrowing for tests so the test runner doesn't mark
+  // our error handling tests as failed.
+  parentMessagePipe.rethrowErrors = false;
+  // Handler that will always error for helping to test the message pipe
+  // itself.
+  parentMessagePipe.registerHandler('bad-handler', () => {
+    throw Error('This is an error');
+  });
+});
 
-//# sourceURL=guest_query_reciever.js
+//# sourceURL=guest_query_receiver.js

chromeos/components/media_app_ui/test/media_app_guest_ui_browsertest.js

@@ -28,9 +28,14 @@ var MediaAppGuestUIBrowserTest = class extends testing.Test {
   preLoad() {
     document.addEventListener('DOMContentLoaded', () => {
       const mojoBindingsLite = document.createElement('script');
-      mojoBindingsLite.src = 'chrome://resources/mojo/mojo/public/js/mojo_bindings_lite.js';
+      mojoBindingsLite.src =
+          'chrome://resources/mojo/mojo/public/js/mojo_bindings_lite.js';
       document.head.appendChild(mojoBindingsLite);
     });
+    // The guest will try and create a message pipe to its parent, since there
+    // is no containing frame here, window.parent === parent. This line mocks
+    // a wrapping iframe so the message pipe can still instantiate correctly.
+    window.parent = document.createElement('iframe');
   }
 };
 
@@ -46,4 +51,4 @@ TEST_F('MediaAppGuestUIBrowserTest', 'GuestCanSpawnWorkers', () => {
   }
 
   assertEquals(error, null);
-});
+});
\ No newline at end of file

chromeos/components/media_app_ui/test/media_app_ui_browsertest.js

@@ -61,12 +61,7 @@ var MediaAppUIBrowserTest = class extends testing.Test {
   /** @override */
   setUp() {
     super.setUp();
-    driver = new GuestDriver(GUEST_ORIGIN);
-  }
-
-  /** @override */
-  tearDown() {
-    driver.tearDown();
+    driver = new GuestDriver();
   }
 };
 
@@ -144,3 +139,31 @@ TEST_F('MediaAppUIBrowserTest', 'CanFullscreenVideo', async () => {
 
   testDone();
 });
+
+// Tests that we receive an error if our message is unhandled.
+TEST_F('MediaAppUIBrowserTest', 'ReceivesNoHandlerError', async () => {
+  let errorMessage = '';
+  try {
+    await guestMessagePipe.sendMessage('unknown-message', null);
+  } catch (error) {
+    errorMessage = error.message;
+  }
+
+  assertEquals(
+      errorMessage,
+      'No handler registered for message type \'unknown-message\'');
+  testDone();
+});
+
+// Tests that we receive an error if the handler fails.
+TEST_F('MediaAppUIBrowserTest', 'ReceivesProxiedError', async () => {
+  let errorMessage = '';
+  try {
+    await guestMessagePipe.sendMessage('bad-handler', null);
+  } catch (error) {
+    errorMessage = error.message;
+  }
+
+  assertEquals(errorMessage, 'This is an error');
+  testDone();
+});

chromeos/components/media_app_ui/test/receiver_api.js

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/** @type {!MessagePipe} */
+var parentMessagePipe;
\ No newline at end of file