perf: Stop encoding messages in mobile (#3347)

The current approach for message passing in mobile encodes the JSON
payloads into a byte array before injecting into the WebView. This was
done by suggestion from @naugtur, but was known to be slow on mobile,
especially with larger messages.

More recently this caused performance degradations with Snaps that
MetaMask is actively trying to ship to production causing us to revisit
this. After discussion with @weizman we landed on the fact that simply
JSON stringifying should be safe enough and much more performant.

Author: FrederikBolding

packages/snaps-controllers/src/services/webview/WebViewMessageStream.test.ts

@@ -22,12 +22,11 @@ describe('WebViewMessageStream', () => {
     expect(await responsePromise).toBe(555);
 
     expect(mockWebViewA.injectJavaScript).toHaveBeenCalledWith(
-      `window.postMessage([123,34,116,97,114,103,101,116,34,58,34,98,34,44,34,100,97,116,97,34,58,34,83,89,78,34,125])`,
+      `window.postMessage("{\\"target\\":\\"b\\",\\"data\\":\\"SYN\\"}")`,
     );
 
-    // Inject { target: "foo", data: 111 }
     mockWebViewA.injectJavaScript(
-      `window.postMessage([123,34,116,97,114,103,101,116,34,58,34,102,111,111,34,44,34,100,97,116,97,34,58,49,49,49,125])`,
+      `window.postMessage("{\\"target\\":\\"foo\\",\\"data\\":111}")`,
     );
 
     const listener = jest.fn();

packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts

@@ -1,7 +1,7 @@
 import type { PostMessageEvent } from '@metamask/post-message-stream';
 import { BasePostMessageStream } from '@metamask/post-message-stream';
 import { isValidStreamMessage } from '@metamask/post-message-stream/dist/utils';
-import { assert, stringToBytes } from '@metamask/utils';
+import { assert } from '@metamask/utils';
 
 export type WebViewInterface = {
   injectJavaScript(js: string): void;
@@ -56,11 +56,9 @@ export class WebViewMessageStream extends BasePostMessageStream {
       data,
     });
 
-    // To prevent XSS, we encode the message before injecting it.
-    // This adds significant performance overhead for larger messages.
-    const bytes = new Uint8Array(stringToBytes(json));
+    const encoded = JSON.stringify(json);
 
-    this.#webView.injectJavaScript(`window.postMessage([${bytes.toString()}])`);
+    this.#webView.injectJavaScript(`window.postMessage(${encoded})`);
   }
 
   // TODO: Either fix this lint violation or explain why it's necessary to

packages/snaps-controllers/src/test-utils/webview.ts

@@ -1,5 +1,3 @@
-import { bytesToString } from '@metamask/utils';
-
 import { WebViewMessageStream } from '../services/webview/WebViewMessageStream';
 
 /**
@@ -9,9 +7,7 @@ import { WebViewMessageStream } from '../services/webview/WebViewMessageStream';
  * @returns The decoded JSON as a string.
  */
 export function parseInjectedJS(js: string) {
-  const byteString = js.slice(19, -1);
-  const bytes = new Uint8Array(JSON.parse(byteString));
-  return bytesToString(bytes);
+  return JSON.parse(js.slice(19, -1));
 }
 
 /**

packages/snaps-execution-environments/coverage.json

@@ -1,6 +1,6 @@
 {
   "branches": 90.74,
   "functions": 94.96,
-  "lines": 90.86,
-  "statements": 90.28
+  "lines": 90.85,
+  "statements": 90.27
 }

packages/snaps-execution-environments/src/webview/WebViewExecutorStream.test.ts

@@ -1,15 +1,13 @@
 import { sleep } from '@metamask/snaps-utils/test-utils';
-import { stringToBytes } from '@metamask/utils';
 
 import { WebViewExecutorStream } from './WebViewExecutorStream';
 
 describe('WebViewExecutorStream', () => {
   beforeEach(() => {
     const addEventListener = jest.fn();
     const postMessage = jest.fn().mockImplementation((message) => {
-      const bytes = stringToBytes(message);
       addEventListener.mock.calls.forEach(([_type, listener]) => {
-        setTimeout(() => listener({ data: Array.from(bytes) }));
+        setTimeout(() => listener({ data: message }));
       });
     });
     const mockWindow = {

packages/snaps-execution-environments/src/webview/WebViewExecutorStream.ts

@@ -1,7 +1,6 @@
 import type { PostMessageEvent } from '@metamask/post-message-stream';
 import { BasePostMessageStream } from '@metamask/post-message-stream';
 import { isValidStreamMessage } from '@metamask/post-message-stream/dist/utils';
-import { bytesToString } from '@metamask/utils';
 
 type WebViewExecutorStreamArgs = {
   name: string;
@@ -65,12 +64,11 @@ export class WebViewExecutorStream extends BasePostMessageStream {
   //  ignore.
   // eslint-disable-next-line no-restricted-syntax
   private _onMessage(event: PostMessageEvent): void {
-    if (!Array.isArray(event.data)) {
+    if (typeof event.data !== 'string') {
       return;
     }
 
-    const json = bytesToString(new Uint8Array(event.data as number[]));
-    const message = JSON.parse(json);
+    const message = JSON.parse(event.data);
 
     // Notice that we don't check targetWindow or targetOrigin here.
     // This doesn't seem possible to do in RN.