Separate modifier key press from click helper

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

@@ -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');
   });
 });

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 =