Convert createWindow parameters to options bag and add testid option (#…

…2765)

This PR refactors the `createWindow` function to use an options bag for
its parameters, improving clarity and extensibility. Additionally, it
introduces a new `testId` option, allowing customization of the
`data-testid` attribute for the generated iframe.

We need this change in the [MetaMask/ocap-kernel
](https://github.com/MetaMask/ocap-kernel/) package ([see
issue](MetaMask/ocap-kernel#91)), where the
`createWindow` function is used, to set a different testId that doesn't
conflict with the snaps iframe. This will ensure clear differentiation
when using iframes across different packages.

Updated and expanded the test suite to cover the presence and absence of
sandbox and testId attributes.

packages/snaps-controllers/src/services/iframe/IframeExecutionService.ts

@@ -35,7 +35,10 @@ export class IframeExecutionService extends AbstractExecutionService<Window> {
     worker: Window;
     stream: BasePostMessageStream;
   }> {
-    const iframeWindow = await createWindow(this.iframeUrl.toString(), jobId);
+    const iframeWindow = await createWindow({
+      uri: this.iframeUrl.toString(),
+      id: jobId,
+    });
 
     const stream = new WindowPostMessageStream({
       name: 'parent',

packages/snaps-controllers/src/services/webworker/WebWorkerExecutionService.ts

@@ -98,11 +98,11 @@ export class WebWorkerExecutionService extends AbstractExecutionService<string>
       return;
     }
 
-    const window = await createWindow(
-      this.#documentUrl.href,
-      WORKER_POOL_ID,
-      false,
-    );
+    const window = await createWindow({
+      uri: this.#documentUrl.href,
+      id: WORKER_POOL_ID,
+      sandbox: false,
+    });
 
     this.#runtimeStream = new WindowPostMessageStream({
       name: 'parent',

packages/snaps-execution-environments/src/proxy/ProxySnapExecutor.ts

@@ -96,7 +96,7 @@ export class ProxySnapExecutor {
    * @returns The executor job object.
    */
   async #initializeJob(jobId: string): Promise<ExecutorJob> {
-    const window = await createWindow(this.#frameUrl, jobId);
+    const window = await createWindow({ uri: this.#frameUrl, id: jobId });
     const jobStream = new WindowPostMessageStream({
       name: 'parent',
       target: 'child',

packages/snaps-utils/src/iframe.test.browser.ts

@@ -5,12 +5,57 @@ const IFRAME_URL = `http://localhost:4569`;
 const MOCK_JOB_ID = 'job-id';
 
 describe('createWindow', () => {
+  afterEach(() => {
+    const iframe = document.getElementById(MOCK_JOB_ID);
+    if (iframe) {
+      document.body.removeChild(iframe);
+    }
+  });
+
   it('creates an iframe window with the provided job ID as the iframe ID', async () => {
-    const window = await createWindow(IFRAME_URL, MOCK_JOB_ID);
+    const window = await createWindow({ uri: IFRAME_URL, id: MOCK_JOB_ID });
     const iframe = document.getElementById(MOCK_JOB_ID) as HTMLIFrameElement;
 
     expect(iframe).toBeDefined();
     expect(iframe.contentWindow).toBe(window);
     expect(iframe.id).toBe(MOCK_JOB_ID);
   });
+
+  it('sets the sandbox attribute when the sandbox option is true', async () => {
+    await createWindow({ uri: IFRAME_URL, id: MOCK_JOB_ID, sandbox: true });
+    const iframe = document.getElementById(MOCK_JOB_ID) as HTMLIFrameElement;
+
+    expect(iframe).toBeDefined();
+    expect(iframe.getAttribute('sandbox')).toBe('allow-scripts');
+  });
+
+  it('does not set the sandbox attribute when the sandbox option is false', async () => {
+    await createWindow({ uri: IFRAME_URL, id: MOCK_JOB_ID, sandbox: false });
+    const iframe = document.getElementById(MOCK_JOB_ID) as HTMLIFrameElement;
+
+    expect(iframe).toBeDefined();
+    expect(iframe.getAttribute('sandbox')).toBeNull();
+  });
+
+  it('sets the data-testid attribute when provided', async () => {
+    const testId = 'test-id';
+
+    await createWindow({
+      uri: IFRAME_URL,
+      id: MOCK_JOB_ID,
+      testId,
+    });
+    const iframe = document.getElementById(MOCK_JOB_ID) as HTMLIFrameElement;
+
+    expect(iframe).toBeDefined();
+    expect(iframe.getAttribute('data-testid')).toBe(testId);
+  });
+
+  it('uses the default data-testid attribute when not provided', async () => {
+    await createWindow({ uri: IFRAME_URL, id: MOCK_JOB_ID });
+    const iframe = document.getElementById(MOCK_JOB_ID) as HTMLIFrameElement;
+
+    expect(iframe).toBeDefined();
+    expect(iframe.getAttribute('data-testid')).toBe('snaps-iframe');
+  });
 });

packages/snaps-utils/src/iframe.ts

@@ -3,22 +3,31 @@
  * forever if the iframe never loads, but the promise should be wrapped in
  * an initialization timeout in the SnapController.
  *
- * @param uri - The iframe URI.
- * @param id - The ID to assign to the iframe.
- * @param sandbox - Whether to enable the sandbox attribute.
+ *
+ * @param options - The options for createWindow.
+ * @param options.uri - The iframe URI.
+ * @param options.id - The ID to assign to the iframe.
+ * @param options.sandbox - Whether to enable the sandbox attribute.
+ * @param options.testId - The data-testid attribute to assign to the iframe.
  * @returns A promise that resolves to the contentWindow of the iframe.
  */
-export async function createWindow(
-  uri: string,
-  id: string,
+export async function createWindow({
+  uri,
+  id,
   sandbox = true,
-): Promise<Window> {
+  testId = 'snaps-iframe',
+}: {
+  uri: string;
+  id: string;
+  sandbox?: boolean;
+  testId?: string;
+}): Promise<Window> {
   return await new Promise((resolve, reject) => {
     const iframe = document.createElement('iframe');
     // The order of operations appears to matter for everything except this
     // attribute. We may as well set it here.
     iframe.setAttribute('id', id);
-    iframe.setAttribute('data-testid', 'snaps-iframe');
+    iframe.setAttribute('data-testid', testId);
 
     if (sandbox) {
       // For the sandbox property to have any effect it needs to be set before the iframe is appended.