diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json index 3a5693221a..4f40812790 100644 --- a/packages/snaps-controllers/coverage.json +++ b/packages/snaps-controllers/coverage.json @@ -1,6 +1,6 @@ { "branches": 93.31, - "functions": 96.8, - "lines": 98.15, - "statements": 97.88 + "functions": 97.05, + "lines": 98.2, + "statements": 97.93 } diff --git a/packages/snaps-controllers/src/services/webview/WebViewExecutionService.test.ts b/packages/snaps-controllers/src/services/webview/WebViewExecutionService.test.ts index dbce1a6073..c67bf41aaf 100644 --- a/packages/snaps-controllers/src/services/webview/WebViewExecutionService.test.ts +++ b/packages/snaps-controllers/src/services/webview/WebViewExecutionService.test.ts @@ -14,28 +14,19 @@ import type { WebViewInterface } from './WebViewMessageStream'; * Create a response message for the given request. This function assumes that * the response is for the parent, and uses the command stream. * - * @param message - The request message. * @param request - The request to respond to. * @param response - The response to send. * @returns The response message. */ -function getResponse( - message: Record, - request: JsonRpcRequest, - response: Json, -) { +function getResponse(request: JsonRpcRequest, response: Json) { return { target: 'parent', data: { - jobId: message.jobId, - frameUrl: message.frameUrl, + name: 'command', data: { - name: 'command', - data: { - jsonrpc: '2.0', - id: request.id, - result: response, - }, + jsonrpc: '2.0', + id: request.id, + result: response, }, }, }; @@ -50,11 +41,13 @@ describe('WebViewExecutionService', () => { }; const { service } = createService(WebViewExecutionService, { - getWebView: async () => - Promise.resolve(mockedWebView as unknown as WebViewInterface), + createWebView: async (_id: string) => + mockedWebView as unknown as WebViewInterface, + removeWebView: jest.fn(), }); expect(service).toBeDefined(); + await service.terminateAllSnaps(); }); it('can execute snaps', async () => { @@ -89,11 +82,10 @@ describe('WebViewExecutionService', () => { // Handle incoming requests. if ( isPlainObject(data) && - isPlainObject(data.data) && - data.data.name === 'command' && - isJsonRpcRequest(data.data.data) + data.name === 'command' && + isJsonRpcRequest(data.data) ) { - const request = data.data.data; + const request = data.data; // Respond "OK" to the `ping`, `executeSnap`, and `terminate` request. if ( @@ -101,18 +93,18 @@ describe('WebViewExecutionService', () => { request.method === 'executeSnap' || request.method === 'terminate' ) { - sendMessage(getResponse(data, request, 'OK')); + sendMessage(getResponse(request, 'OK')); } } }); const { service } = createService(WebViewExecutionService, { - getWebView: async () => - Promise.resolve({ - registerMessageListener, - unregisterMessageListener: jest.fn(), - injectJavaScript, - }), + createWebView: async (_id: string) => ({ + registerMessageListener, + unregisterMessageListener: jest.fn(), + injectJavaScript, + }), + removeWebView: jest.fn(), }); expect( @@ -122,5 +114,7 @@ describe('WebViewExecutionService', () => { endowments: [], }), ).toBe('OK'); + + await service.terminateAllSnaps(); }); }); diff --git a/packages/snaps-controllers/src/services/webview/WebViewExecutionService.ts b/packages/snaps-controllers/src/services/webview/WebViewExecutionService.ts index 053119c9bd..95c5b53628 100644 --- a/packages/snaps-controllers/src/services/webview/WebViewExecutionService.ts +++ b/packages/snaps-controllers/src/services/webview/WebViewExecutionService.ts @@ -1,32 +1,35 @@ -import type { ExecutionServiceArgs } from '../AbstractExecutionService'; -import { ProxyExecutionService } from '../proxy/ProxyExecutionService'; +import type { TerminateJobArgs } from '../AbstractExecutionService'; +import { + AbstractExecutionService, + type ExecutionServiceArgs, +} from '../AbstractExecutionService'; import type { WebViewInterface } from './WebViewMessageStream'; import { WebViewMessageStream } from './WebViewMessageStream'; export type WebViewExecutionServiceArgs = ExecutionServiceArgs & { - getWebView: () => Promise; + createWebView: (jobId: string) => Promise; + removeWebView: (jobId: string) => void; }; -export class WebViewExecutionService extends ProxyExecutionService { - #getWebView; +export class WebViewExecutionService extends AbstractExecutionService { + readonly #createWebView; + + readonly #removeWebView; constructor({ messenger, setupSnapProvider, - getWebView, + createWebView, + removeWebView, ...args }: WebViewExecutionServiceArgs) { super({ ...args, messenger, setupSnapProvider, - stream: new WebViewMessageStream({ - name: 'parent', - target: 'child', - getWebView, - }), }); - this.#getWebView = getWebView; + this.#createWebView = createWebView; + this.#removeWebView = removeWebView; } /** @@ -37,16 +40,18 @@ export class WebViewExecutionService extends ProxyExecutionService { * @returns An object with the worker ID and stream. */ protected async initEnvStream(jobId: string) { - // Ensure that the WebView has been loaded before we proceed. - await this.#ensureWebViewLoaded(); + const webView = await this.#createWebView(jobId); + + const stream = new WebViewMessageStream({ + name: 'parent', + target: 'child', + webView, + }); - return super.initEnvStream(jobId); + return { worker: webView, stream }; } - /** - * Ensure that the WebView has been loaded by awaiting the getWebView promise. - */ - async #ensureWebViewLoaded() { - await this.#getWebView(); + protected terminateJob(jobWrapper: TerminateJobArgs): void { + this.#removeWebView(jobWrapper.id); } } diff --git a/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts b/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts index edd25c45b5..6fb6a9e254 100644 --- a/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts +++ b/packages/snaps-controllers/src/services/webview/WebViewMessageStream.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 { logError } from '@metamask/snaps-utils'; import { assert, stringToBytes } from '@metamask/utils'; export type WebViewInterface = { @@ -13,7 +12,7 @@ export type WebViewInterface = { export type WebViewStreamArgs = { name: string; target: string; - getWebView: () => Promise; + webView: WebViewInterface; }; /** @@ -33,9 +32,9 @@ export class WebViewMessageStream extends BasePostMessageStream { * @param args.name - The name of the stream. Used to differentiate between * multiple streams sharing the same window object. * @param args.target - The name of the stream to exchange messages with. - * @param args.getWebView - A asynchronous getter for the webview. + * @param args.webView - A reference to the WebView. */ - constructor({ name, target, getWebView }: WebViewStreamArgs) { + constructor({ name, target, webView }: WebViewStreamArgs) { super(); this.#name = name; @@ -43,19 +42,11 @@ export class WebViewMessageStream extends BasePostMessageStream { this._onMessage = this._onMessage.bind(this); - // This is a bit atypical from other post-message streams. - // We have to wait for the WebView to fully load before we can continue using the stream. - getWebView() - .then((webView) => { - this.#webView = webView; - // This method is already bound. - // eslint-disable-next-line @typescript-eslint/unbound-method - webView.registerMessageListener(this._onMessage); - this._handshake(); - }) - .catch((error) => { - logError(error); - }); + this.#webView = webView; + // This method is already bound. + // eslint-disable-next-line @typescript-eslint/unbound-method + this.#webView.registerMessageListener(this._onMessage); + this._handshake(); } protected _postMessage(data: unknown): void { diff --git a/packages/snaps-controllers/src/test-utils/webview.ts b/packages/snaps-controllers/src/test-utils/webview.ts index c51eff998f..6037533f97 100644 --- a/packages/snaps-controllers/src/test-utils/webview.ts +++ b/packages/snaps-controllers/src/test-utils/webview.ts @@ -17,7 +17,7 @@ export function parseInjectedJS(js: string) { /** * Takes no param and return mocks necessary for testing WebViewMessageStream. * - * @returns The mockWebView, mockGetWebView, and mockStream. + * @returns The mockWebView, and mockStream. */ export function createWebViewObjects() { const registerMessageListenerA = jest.fn(); @@ -45,26 +45,21 @@ export function createWebViewObjects() { }), }; - const mockGetWebViewA = jest.fn().mockResolvedValue(mockWebViewA); - const mockGetWebViewB = jest.fn().mockResolvedValue(mockWebViewB); - const streamA = new WebViewMessageStream({ name: 'a', target: 'b', - getWebView: mockGetWebViewA, + webView: mockWebViewA, }); const streamB = new WebViewMessageStream({ name: 'b', target: 'a', - getWebView: mockGetWebViewB, + webView: mockWebViewB, }); return { mockWebViewA, mockWebViewB, - mockGetWebViewA, - mockGetWebViewB, streamA, streamB, }; diff --git a/packages/snaps-execution-environments/lavamoat/browserify/webview/policy.json b/packages/snaps-execution-environments/lavamoat/browserify/webview/policy.json index df5877fd7f..520b71350d 100644 --- a/packages/snaps-execution-environments/lavamoat/browserify/webview/policy.json +++ b/packages/snaps-execution-environments/lavamoat/browserify/webview/policy.json @@ -1,5 +1,26 @@ { "resources": { + "@metamask/json-rpc-engine": { + "packages": { + "@metamask/providers>@metamask/safe-event-emitter": true, + "@metamask/rpc-errors": true, + "@metamask/utils": true + } + }, + "@metamask/object-multiplex": { + "globals": { + "console.warn": true + }, + "packages": { + "@metamask/object-multiplex>once": true, + "readable-stream": true + } + }, + "@metamask/object-multiplex>once": { + "packages": { + "@metamask/object-multiplex>once>wrappy": true + } + }, "@metamask/post-message-stream": { "globals": { "MessageEvent.prototype": true, @@ -16,6 +37,39 @@ "readable-stream": true } }, + "@metamask/providers": { + "globals": { + "console": true + }, + "packages": { + "@metamask/json-rpc-engine": true, + "@metamask/providers>@metamask/json-rpc-middleware-stream": true, + "@metamask/providers>@metamask/safe-event-emitter": true, + "@metamask/providers>is-stream": true, + "@metamask/rpc-errors": true, + "eslint>fast-deep-equal": true, + "readable-stream": true + } + }, + "@metamask/providers>@metamask/json-rpc-middleware-stream": { + "globals": { + "console.warn": true, + "setTimeout": true + }, + "packages": { + "@metamask/providers>@metamask/safe-event-emitter": true, + "@metamask/utils": true, + "readable-stream": true + } + }, + "@metamask/providers>@metamask/safe-event-emitter": { + "globals": { + "setTimeout": true + }, + "packages": { + "browserify>events": true + } + }, "@metamask/rpc-errors": { "packages": { "@metamask/rpc-errors>fast-safe-stringify": true, diff --git a/packages/snaps-execution-environments/src/webview/index.ts b/packages/snaps-execution-environments/src/webview/index.ts index 4db0d3f11c..8583a9910e 100644 --- a/packages/snaps-execution-environments/src/webview/index.ts +++ b/packages/snaps-execution-environments/src/webview/index.ts @@ -1,6 +1,6 @@ import { executeLockdownEvents } from '../common/lockdown/lockdown-events'; import { executeLockdownMore } from '../common/lockdown/lockdown-more'; -import { ProxySnapExecutor } from '../proxy/ProxySnapExecutor'; +import { IFrameSnapExecutor } from '../iframe/IFrameSnapExecutor'; import { WebViewExecutorStream } from './WebViewExecutorStream'; // Lockdown is already applied in LavaMoat @@ -13,4 +13,4 @@ const parentStream = new WebViewExecutorStream({ targetWindow: window.ReactNativeWebView, }); -ProxySnapExecutor.initialize(parentStream); +IFrameSnapExecutor.initialize(parentStream);