From 800e7e945c26e73bb3d3535cac7763f30982376f Mon Sep 17 00:00:00 2001
From: Liang Gong <lgong@meta.com>
Date: Fri, 17 Nov 2023 16:09:32 -0800
Subject: [PATCH] fix(api): not repeatedly accepting dialog

Summary:
Fix the issue reported in OSS:
https://github.com/facebook/memlab/issues/98

Close #98

Reviewed By: tulga1970

Differential Revision: D51419953

fbshipit-source-id: 32fb0d9cfa8b6745b6dac2875a7b946f297f28c4
---
 packages/api/src/API.ts                       | 10 +--
 .../__tests__/API/E2EFindMemoryLeaks.test.ts  | 65 +++++++++++++++++++
 packages/core/src/lib/BrowserInfo.ts          |  3 +-
 3 files changed, 67 insertions(+), 11 deletions(-)

diff --git a/packages/api/src/API.ts b/packages/api/src/API.ts
index c490431d9..ecfd55c29 100644
--- a/packages/api/src/API.ts
+++ b/packages/api/src/API.ts
@@ -387,18 +387,11 @@ async function setupPage(page: Page, options: APIOptions = {}): Promise<void> {
   await page.setCacheEnabled(cache);
 
   // automatically accept dialog
-  page.on('dialog', async dialog => {
-    await dialog.accept();
-  });
-}
-
-function autoDismissDialog(page: Page, options: APIOptions = {}): void {
-  const config = options.config ?? defaultConfig;
   page.on('dialog', async dialog => {
     if (config.verbose) {
       info.lowLevel(`Browser dialog: ${dialog.message()}`);
     }
-    await dialog.dismiss();
+    await dialog.accept();
   });
 }
 
@@ -446,7 +439,6 @@ export async function testInBrowser(options: APIOptions = {}): Promise<void> {
     const visitPlan = testPlanner.getVisitPlan();
     // setup page configuration
     config.setDevice(visitPlan.device);
-    autoDismissDialog(page);
     await initBrowserInfoInConfig(browser);
     browserInfo.monitorWebConsole(page);
     await setupPage(page, options);
diff --git a/packages/api/src/__tests__/API/E2EFindMemoryLeaks.test.ts b/packages/api/src/__tests__/API/E2EFindMemoryLeaks.test.ts
index 6f035be7b..2b13a7e38 100644
--- a/packages/api/src/__tests__/API/E2EFindMemoryLeaks.test.ts
+++ b/packages/api/src/__tests__/API/E2EFindMemoryLeaks.test.ts
@@ -93,3 +93,68 @@ test(
   },
   testTimeout,
 );
+
+function injectDetachedDOMElementsWithPrompt() {
+  // @ts-ignore
+  window.injectHookForLink4 = () => {
+    class TestObject {
+      key: 'value';
+    }
+    const arr = [];
+    for (let i = 0; i < 23; ++i) {
+      arr.push(document.createElement('div'));
+    }
+    // @ts-ignore
+    window.__injectedValue = arr;
+    // @ts-ignore
+    window._path_1 = {x: {y: document.createElement('div')}};
+    // @ts-ignore
+    window._path_2 = new Set([document.createElement('div')]);
+    // @ts-ignore
+    window._randomObject = [new TestObject()];
+    // pop up dialogs
+    alert('alert test');
+    confirm('confirm test');
+    prompt('prompt test:', 'default');
+  };
+}
+
+test(
+  'test runner should work with pages having pop up dialog',
+  async () => {
+    const selfDefinedScenario: IScenario = {
+      app: (): string => 'test-spa',
+      url: (): string => '',
+      action: async (page: Page): Promise<void> => {
+        await page.evaluate(() => {
+          // pop up dialogs
+          alert('alert test');
+          confirm('confirm test');
+          prompt('prompt test:', 'default');
+        });
+        await page.click('[data-testid="link-4"]');
+      },
+      leakFilter: (node: IHeapNode) => {
+        return node.name === 'TestObject' && node.type === 'object';
+      },
+    };
+
+    const workDir = path.join(os.tmpdir(), 'memlab-api-test', `${process.pid}`);
+    fs.mkdirsSync(workDir);
+
+    const result = await run({
+      scenario: selfDefinedScenario,
+      evalInBrowserAfterInitLoad: injectDetachedDOMElementsWithPrompt,
+      workDir,
+    });
+    // detected all different leak trace cluster
+    expect(result.leaks.length).toBe(1);
+    // expect all traces are found
+    expect(
+      result.leaks.some(leak => JSON.stringify(leak).includes('_randomObject')),
+    );
+    const reader = result.runResult;
+    expect(path.resolve(reader.getRootDirectory())).toBe(path.resolve(workDir));
+  },
+  testTimeout,
+);
diff --git a/packages/core/src/lib/BrowserInfo.ts b/packages/core/src/lib/BrowserInfo.ts
index 9976ced6d..06b2746ed 100644
--- a/packages/core/src/lib/BrowserInfo.ts
+++ b/packages/core/src/lib/BrowserInfo.ts
@@ -114,7 +114,6 @@ class BrowserInfo {
     page.on('pageerror', handleError);
     page.on('error', handleError);
 
-    // automatically accept dialog
     page.on('dialog', async (dialog: Dialog) => {
       let msg = this.formatDialogMessage(dialog);
       this._consoleMessages.push(msg);
@@ -122,7 +121,7 @@ class BrowserInfo {
         msg = this.formatDialogMessage(dialog, {color: true});
         info.highLevel(msg);
       }
-      await dialog.accept();
+      // dialog will be auto accepted or dismissed in setupPage
     });
   }
 }