From 18fb49bad8a63f62b7845ecc3e172eaacdc48769 Mon Sep 17 00:00:00 2001
From: Randolf <jrandolf@chromium.org>
Date: Mon, 30 Jan 2023 08:38:58 +0000
Subject: [PATCH] Separate modifier key press from click helper
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This CL separates the modifier key press from the click helper to lower the complexity of the click API and improve overall API safety of using modifier clicks.

Bug: 1411188
Change-Id: Ide04871c14589d7b60dd361d9a607ea3d8f75b28
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4201723
Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Randolf Jung <jrandolf@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
---
 test/e2e/sources/sourcemap_test.ts |  9 +++++----
 test/shared/helper.ts              | 25 +++++++++++++------------
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/test/e2e/sources/sourcemap_test.ts b/test/e2e/sources/sourcemap_test.ts
index 07c70fe463..5c34380c18 100644
--- a/test/e2e/sources/sourcemap_test.ts
+++ b/test/e2e/sources/sourcemap_test.ts
@@ -15,6 +15,7 @@ import {
   waitFor,
   waitForElementWithTextContent,
   waitForFunction,
+  withControlOrMetaKey,
 } from '../../shared/helper.js';
 import {describe, it} from '../../shared/mocha-extensions.js';
 import {CONSOLE_TAB_SELECTOR, focusConsolePrompt, getCurrentConsoleMessages} from '../helpers/console-helpers.js';
@@ -553,7 +554,7 @@ describe('The Elements Tab', async () => {
       await clickNthChildOfSelectedElementNode(1);
     });
     const value = await waitForCSSPropertyValue('body .text', 'color', 'green', 'app.scss:6');
-    await click(value, {clickOptions: {modifier: 'ControlOrMeta'}});
+    await withControlOrMetaKey(() => click(value));
     await waitForElementWithTextContent('Line 12, Column 9');
   });
 
@@ -566,7 +567,7 @@ describe('The Elements Tab', async () => {
       await clickNthChildOfSelectedElementNode(1);
     });
     const value = await waitForCSSPropertyValue('body .text', 'color', 'green', 'app.scss:6');
-    await click(value, {clickOptions: {modifier: 'ControlOrMeta'}});
+    await withControlOrMetaKey(() => click(value));
     await waitForElementWithTextContent('Line 12, Column 9');
   });
 
@@ -579,7 +580,7 @@ describe('The Elements Tab', async () => {
       await clickNthChildOfSelectedElementNode(1);
     });
     const value = await waitForCSSPropertyValue('body .text', 'color', 'green', 'app.scss:6');
-    await click(value, {clickOptions: {modifier: 'ControlOrMeta'}});
+    await withControlOrMetaKey(() => click(value));
     await waitForElementWithTextContent('Line 12, Column 9');
   });
 
@@ -592,7 +593,7 @@ describe('The Elements Tab', async () => {
       await clickNthChildOfSelectedElementNode(1);
     });
     const value = await waitForCSSPropertyValue('body .text', 'color', 'green', 'app.scss:6');
-    await click(value, {clickOptions: {modifier: 'ControlOrMeta'}});
+    await withControlOrMetaKey(() => click(value));
     await waitForElementWithTextContent('Line 12, Column 9');
   });
 });
diff --git a/test/shared/helper.ts b/test/shared/helper.ts
index 9a3e80f119..9b4b804388 100644
--- a/test/shared/helper.ts
+++ b/test/shared/helper.ts
@@ -95,13 +95,22 @@ export const getElementPosition =
 
 export interface ClickOptions {
   root?: puppeteer.JSHandle;
-  clickOptions?: PuppeteerClickOptions;
+  clickOptions?: puppeteer.ClickOptions;
   maxPixelsFromLeft?: number;
 }
 
-interface PuppeteerClickOptions extends puppeteer.ClickOptions {
-  modifier?: 'ControlOrMeta';
-}
+const CONTROL_OR_META = platform === 'mac' ? 'Meta' : 'Control';
+export const withControlOrMetaKey = async (action: () => Promise<void>, root = getBrowserAndPages().frontend) => {
+  await waitForFunction(async () => {
+    await root.keyboard.down(CONTROL_OR_META);
+    try {
+      await action();
+      return true;
+    } finally {
+      await root.keyboard.up(CONTROL_OR_META);
+    }
+  });
+};
 
 export const click = async (selector: string|puppeteer.ElementHandle, options?: ClickOptions) => {
   const {frontend} = getBrowserAndPages();
@@ -112,20 +121,12 @@ export const click = async (selector: string|puppeteer.ElementHandle, options?:
     throw new Error(`Unable to locate clickable element "${selector}".`);
   }
 
-  const modifier = platform === 'mac' ? 'Meta' : 'Control';
-  if (options?.clickOptions?.modifier) {
-    await frontend.keyboard.down(modifier);
-  }
-
   // Click on the button and wait for the console to load. The reason we use this method
   // rather than elementHandle.click() is because the frontend attaches the behavior to
   // a 'mousedown' event (not the 'click' event). To avoid attaching the test behavior
   // to a specific event we instead locate the button in question and ask Puppeteer to
   // click on it instead.
   await frontend.mouse.click(clickableElement.x, clickableElement.y, options && options.clickOptions);
-  if (options?.clickOptions?.modifier) {
-    await frontend.keyboard.up(modifier);
-  }
 };
 
 export const doubleClick =