feat(snapshots): use double-buffer to avoid white flash on hover (#21828)

packages/trace-viewer/src/ui/snapshotTab.css

@@ -76,11 +76,25 @@
   box-shadow: 0 12px 28px 0 rgba(0,0,0,.2),0 2px 4px 0 rgba(0,0,0,.1);
 }
 
-iframe#snapshot {
+.snapshot-switcher {
   width: 100%;
   height: calc(100% - var(--window-header-height));
+  position: relative;
+}
+
+iframe[name=snapshot] {
+  position: absolute;
+  top: 0;
+  left: 0;
+  width: 100%;
+  height: 100%;
   border: none;
   background: white;
+  visibility: hidden;
+}
+
+iframe.snapshot-visible[name=snapshot] {
+  visibility: visible;
 }
 
 .no-snapshot {

packages/trace-viewer/src/ui/snapshotTab.tsx

@@ -77,31 +77,61 @@ export const SnapshotTab: React.FunctionComponent<{
     return { snapshots, snapshotInfoUrl, snapshotUrl, pointX, pointY, popoutUrl };
   }, [snapshots, snapshotTab]);
 
-  const iframeRef = React.useRef<HTMLIFrameElement>(null);
+  const iframeRef0 = React.useRef<HTMLIFrameElement>(null);
+  const iframeRef1 = React.useRef<HTMLIFrameElement>(null);
   const [snapshotInfo, setSnapshotInfo] = React.useState({ viewport: kDefaultViewport, url: '' });
+  const loadingRef = React.useRef({ iteration: 0, visibleIframe: 0 });
+
   React.useEffect(() => {
     (async () => {
+      const thisIteration = loadingRef.current.iteration + 1;
+      const newVisibleIframe = 1 - loadingRef.current.visibleIframe;
+      loadingRef.current.iteration = thisIteration;
+
+      const newSnapshotInfo = { url: '', viewport: kDefaultViewport };
       if (snapshotInfoUrl) {
         const response = await fetch(snapshotInfoUrl);
         const info = await response.json();
-        if (!info.error)
+        if (!info.error) {
-          setSnapshotInfo(info);
+          newSnapshotInfo.url = info.url;
-      } else {
+          newSnapshotInfo.viewport = info.viewport;
-        setSnapshotInfo({ viewport: kDefaultViewport, url: '' });
+        }
       }
-      if (!iframeRef.current)
+
+      // Interrupted by another load - bail out.
+      if (loadingRef.current.iteration !== thisIteration)
         return;
-      try {
+
-        const newUrl = snapshotUrl + (pointX === undefined ? '' : `&pointX=${pointX}&pointY=${pointY}`);
+      const iframe = [iframeRef0, iframeRef1][newVisibleIframe].current;
-        // Try preventing history entry from being created.
+      if (iframe) {
-        if (iframeRef.current.contentWindow)
+        let loadedCallback = () => {};
-          iframeRef.current.contentWindow.location.replace(newUrl);
+        const loadedPromise = new Promise<void>(f => loadedCallback = f);
-        else
+        try {
-          iframeRef.current.src = newUrl;
+          iframe.addEventListener('load', loadedCallback);
-      } catch (e) {
+          iframe.addEventListener('error', loadedCallback);
+
+          const newUrl = snapshotUrl + (pointX === undefined ? '' : `&pointX=${pointX}&pointY=${pointY}`);
+          // Try preventing history entry from being created.
+          if (iframe.contentWindow)
+            iframe.contentWindow.location.replace(newUrl);
+          else
+            iframe.src = newUrl;
+
+          await loadedPromise;
+        } catch {
+        } finally {
+          iframe.removeEventListener('load', loadedCallback);
+          iframe.removeEventListener('error', loadedCallback);
+        }
       }
+      // Interrupted by another load - bail out.
+      if (loadingRef.current.iteration !== thisIteration)
+        return;
+
+      loadingRef.current.visibleIframe = newVisibleIframe;
+      setSnapshotInfo(newSnapshotInfo);
     })();
-  }, [iframeRef, snapshotUrl, snapshotInfoUrl, pointX, pointY]);
+  }, [snapshotUrl, snapshotInfoUrl, pointX, pointY]);
 
   const windowHeaderHeight = 40;
   const snapshotContainerSize = {
@@ -130,7 +160,14 @@ export const SnapshotTab: React.FunctionComponent<{
       testIdAttributeName={testIdAttributeName}
       highlightedLocator={highlightedLocator}
       setHighlightedLocator={setHighlightedLocator}
-      iframe={iframeRef.current} />
+      iframe={iframeRef0.current} />
+    <InspectModeController
+      isInspecting={isInspecting}
+      sdkLanguage={sdkLanguage}
+      testIdAttributeName={testIdAttributeName}
+      highlightedLocator={highlightedLocator}
+      setHighlightedLocator={setHighlightedLocator}
+      iframe={iframeRef1.current} />
     <Toolbar>
       <ToolbarButton title='Pick locator' disabled={!popoutUrl} toggled={pickerVisible} onClick={() => {
         setPickerVisible(!pickerVisible);
@@ -184,7 +221,10 @@ export const SnapshotTab: React.FunctionComponent<{
             </div>
           </div>
         </div>
-        <iframe ref={iframeRef} id='snapshot' name='snapshot'></iframe>
+        <div className='snapshot-switcher'>
+          <iframe ref={iframeRef0} name='snapshot' className={loadingRef.current.visibleIframe === 0 ? 'snapshot-visible' : ''}></iframe>
+          <iframe ref={iframeRef1} name='snapshot' className={loadingRef.current.visibleIframe === 1 ? 'snapshot-visible' : ''}></iframe>
+        </div>
       </div>
     </div>
   </div>;
@@ -210,14 +250,17 @@ export const InspectModeController: React.FunctionComponent<{
 }> = ({ iframe, isInspecting, sdkLanguage, testIdAttributeName, highlightedLocator, setHighlightedLocator }) => {
   React.useEffect(() => {
     const win = iframe?.contentWindow as any;
+    let recorder: Recorder | undefined;
     try {
-      if (!win || !isInspecting && !highlightedLocator && !win._recorder)
+      if (!win)
+        return;
+      recorder = win._recorder;
+      if (!recorder && !isInspecting && !highlightedLocator)
         return;
     } catch {
-      // Potential cross-origin exception.
+      // Potential cross-origin exception when accessing win._recorder.
       return;
     }
-    let recorder: Recorder | undefined = win._recorder;
     if (!recorder) {
       const injectedScript = new InjectedScript(win, false, sdkLanguage, testIdAttributeName, 1, 'chromium', []);
       recorder = new Recorder(injectedScript, {

tests/config/traceViewerFixtures.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import type { Fixtures, Frame, Locator, Page, Browser, BrowserContext } from '@playwright/test';
+import type { Fixtures, FrameLocator, Locator, Page, Browser, BrowserContext } from '@playwright/test';
 import { showTraceViewer } from '../../packages/playwright-core/lib/server';
 
 type BaseTestFixtures = {
@@ -51,7 +51,7 @@ class TraceViewerPage {
     this.consoleStacks = page.locator('.console-stack');
     this.stackFrames = page.getByTestId('stack-trace').locator('.list-view-entry');
     this.networkRequests = page.locator('.network-request-title');
-    this.snapshotContainer = page.locator('.snapshot-container iframe');
+    this.snapshotContainer = page.locator('.snapshot-container iframe.snapshot-visible[name=snapshot]');
   }
 
   async actionIconsText(action: string) {
@@ -96,15 +96,11 @@ class TraceViewerPage {
     return result.sort();
   }
 
-  async snapshotFrame(actionName: string, ordinal: number = 0, hasSubframe: boolean = false): Promise<Frame> {
+  async snapshotFrame(actionName: string, ordinal: number = 0, hasSubframe: boolean = false): Promise<FrameLocator> {
-    const existing = this.page.mainFrame().childFrames()[0];
+    await this.selectAction(actionName, ordinal);
-    await Promise.all([
+    while (this.page.frames().length < (hasSubframe ? 4 : 3))
-      existing ? existing.waitForNavigation() as any : Promise.resolve(),
-      this.selectAction(actionName, ordinal),
-    ]);
-    while (this.page.frames().length < (hasSubframe ? 3 : 2))
       await this.page.waitForEvent('frameattached');
-    return this.page.mainFrame().childFrames()[0];
+    return this.page.frameLocator('iframe.snapshot-visible[name=snapshot]');
   }
 }
 

tests/library/trace-viewer.spec.ts

@@ -260,7 +260,7 @@ test('should capture iframe with sandbox attribute', async ({ page, server, runA
 
   // Render snapshot, check expectations.
   const snapshotFrame = await traceViewer.snapshotFrame('page.evaluate', 0, true);
-  const button = await snapshotFrame.childFrames()[0].waitForSelector('button');
+  const button = snapshotFrame.frameLocator('iframe').locator('button');
   expect(await button.textContent()).toBe('Hello iframe');
 });
 
@@ -283,8 +283,8 @@ test('should capture data-url svg iframe', async ({ page, server, runAndTrace })
 
   // Render snapshot, check expectations.
   const snapshotFrame = await traceViewer.snapshotFrame('page.evaluate', 0, true);
-  await expect(snapshotFrame.childFrames()[0].locator('svg')).toBeVisible();
+  await expect(snapshotFrame.frameLocator('iframe').locator('svg')).toBeVisible();
-  const content = await snapshotFrame.childFrames()[0].content();
+  const content = await snapshotFrame.frameLocator('iframe').locator(':root').innerHTML();
   expect(content).toContain(`d="M16.5 3c-1.74 0-3.41.81-4.5 2.09C10.91 3.81 9.24 3 7.5 3 4.42 3 2 5.42 2 8.5c0 3.78 3.4 6.86 8.55 11.54L12 21.35l1.45-1.32C18.6 15.36 22 12.28 22 8.5 22 5.42 19.58 3 16.5 3zm-4.4 15.55l-.1.1-.1-.1C7.14 14.24 4 11.39 4 8.5 4 6.5 5.5 5 7.5 5c1.54 0 3.04.99 3.57 2.36h1.87C13.46 5.99 14.96 5 16.5 5c2 0 3.5 1.5 3.5 3.5 0 2.89-3.14 5.74-7.9 10.05z"`);
 });
 
@@ -313,19 +313,9 @@ test('should contain adopted style sheets', async ({ page, runAndTrace, browserN
   });
 
   const frame = await traceViewer.snapshotFrame('page.evaluate');
-  await frame.waitForSelector('button');
+  await expect(frame.locator('button')).toHaveCSS('color', 'rgb(255, 0, 0)');
-  const buttonColor = await frame.$eval('button', button => {
+  await expect(frame.locator('div')).toHaveCSS('color', 'rgb(0, 0, 255)');
-    return window.getComputedStyle(button).color;
+  await expect(frame.locator('span')).toHaveCSS('color', 'rgb(0, 0, 255)');
-  });
-  expect(buttonColor).toBe('rgb(255, 0, 0)');
-  const divColor = await frame.$eval('div', div => {
-    return window.getComputedStyle(div).color;
-  });
-  expect(divColor).toBe('rgb(0, 0, 255)');
-  const spanColor = await frame.$eval('span', span => {
-    return window.getComputedStyle(span).color;
-  });
-  expect(spanColor).toBe('rgb(0, 0, 255)');
 });
 
 test('should work with adopted style sheets and replace/replaceSync', async ({ page, runAndTrace, browserName }) => {
@@ -350,27 +340,15 @@ test('should work with adopted style sheets and replace/replaceSync', async ({ p
 
   {
     const frame = await traceViewer.snapshotFrame('page.evaluate', 0);
-    await frame.waitForSelector('button');
+    await expect(frame.locator('button')).toHaveCSS('color', 'rgb(255, 0, 0)');
-    const buttonColor = await frame.$eval('button', button => {
-      return window.getComputedStyle(button).color;
-    });
-    expect(buttonColor).toBe('rgb(255, 0, 0)');
   }
   {
     const frame = await traceViewer.snapshotFrame('page.evaluate', 1);
-    await frame.waitForSelector('button');
+    await expect(frame.locator('button')).toHaveCSS('color', 'rgb(0, 0, 255)');
-    const buttonColor = await frame.$eval('button', button => {
-      return window.getComputedStyle(button).color;
-    });
-    expect(buttonColor).toBe('rgb(0, 0, 255)');
   }
   {
     const frame = await traceViewer.snapshotFrame('page.evaluate', 2);
-    await frame.waitForSelector('button');
+    await expect(frame.locator('button')).toHaveCSS('color', 'rgb(0, 255, 0)');
-    const buttonColor = await frame.$eval('button', button => {
-      return window.getComputedStyle(button).color;
-    });
-    expect(buttonColor).toBe('rgb(0, 255, 0)');
   }
 });
 
@@ -402,8 +380,7 @@ test('should restore scroll positions', async ({ page, runAndTrace, browserName
 
   // Render snapshot, check expectations.
   const frame = await traceViewer.snapshotFrame('scrollIntoViewIfNeeded');
-  const div = await frame.waitForSelector('div');
+  expect(await frame.locator('div').evaluate(div => div.scrollTop)).toBe(136);
-  expect(await div.evaluate(div => div.scrollTop)).toBe(136);
 });
 
 test('should restore control values', async ({ page, runAndTrace }) => {
@@ -450,13 +427,10 @@ test('should restore control values', async ({ page, runAndTrace }) => {
   await expect(textarea).toHaveText('old');
   await expect(textarea).toHaveValue('hello');
 
-  expect(await frame.$eval('option >> nth=0', o => o.hasAttribute('selected'))).toBe(false);
+  expect(await frame.locator('option >> nth=0').evaluate(o => o.hasAttribute('selected'))).toBe(false);
-  expect(await frame.$eval('option >> nth=1', o => o.hasAttribute('selected'))).toBe(true);
+  expect(await frame.locator('option >> nth=1').evaluate(o => o.hasAttribute('selected'))).toBe(true);
-  expect(await frame.$eval('option >> nth=2', o => o.hasAttribute('selected'))).toBe(false);
+  expect(await frame.locator('option >> nth=2').evaluate(o => o.hasAttribute('selected'))).toBe(false);
-  expect(await frame.locator('select').evaluate(s => {
+  await expect(frame.locator('select')).toHaveValues(['opt1', 'opt3']);
-    const options = [...(s as HTMLSelectElement).selectedOptions];
-    return options.map(option => option.value);
-  })).toEqual(['opt1', 'opt3']);
 });
 
 test('should work with meta CSP', async ({ page, runAndTrace, browserName }) => {
@@ -479,9 +453,8 @@ test('should work with meta CSP', async ({ page, runAndTrace, browserName }) =>
 
   // Render snapshot, check expectations.
   const frame = await traceViewer.snapshotFrame('$eval');
-  await frame.waitForSelector('div');
   // Should render shadow dom with post-processing script.
-  expect(await frame.textContent('span')).toBe('World');
+  await expect(frame.locator('span')).toHaveText('World');
 });
 
 test('should handle multiple headers', async ({ page, server, runAndTrace, browserName }) => {
@@ -497,9 +470,8 @@ test('should handle multiple headers', async ({ page, server, runAndTrace, brows
   });
 
   const frame = await traceViewer.snapshotFrame('setContent');
-  await frame.waitForSelector('div');
+  await frame.locator('div').waitFor();
-  const padding = await frame.$eval('body', body => window.getComputedStyle(body).paddingLeft);
+  await expect(frame.locator('body')).toHaveCSS('padding-left', '42px');
-  expect(padding).toBe('42px');
 });
 
 test('should handle src=blob', async ({ page, server, runAndTrace, browserName }) => {
@@ -521,14 +493,12 @@ test('should handle src=blob', async ({ page, server, runAndTrace, browserName }
   });
 
   const frame = await traceViewer.snapshotFrame('page.evaluate');
-  const img = await frame.waitForSelector('img');
+  const size = await frame.locator('img').evaluate(e => (e as HTMLImageElement).naturalWidth);
-  const size = await img.evaluate(e => (e as HTMLImageElement).naturalWidth);
   expect(size).toBe(10);
 });
 
 test('should register custom elements', async ({ page, server, runAndTrace }) => {
   const traceViewer = await runAndTrace(async () => {
-    page.on('console', console.log);
     await page.goto(server.EMPTY_PAGE);
     await page.evaluate(() => {
       customElements.define('my-element', class extends HTMLElement {
@@ -767,8 +737,7 @@ test('should serve overridden request', async ({ page, runAndTrace, server }) =>
   });
   // Render snapshot, check expectations.
   const snapshotFrame = await traceViewer.snapshotFrame('page.goto');
-  const color = await snapshotFrame.locator('body').evaluate(body => getComputedStyle(body).backgroundColor);
+  await expect(snapshotFrame.locator('body')).toHaveCSS('background-color', 'rgb(255, 0, 0)');
-  expect(color).toBe('rgb(255, 0, 0)');
 });
 
 test('should display waitForLoadState even if did not wait for it', async ({ runAndTrace, server, page }) => {
@@ -803,7 +772,7 @@ test('should pick locator', async ({ page, runAndTrace, server }) => {
   });
   const snapshot = await traceViewer.snapshotFrame('page.setContent');
   await traceViewer.page.getByTitle('Pick locator').click();
-  await snapshot.click('button');
+  await snapshot.locator('button').click();
   await expect(traceViewer.page.locator('.cm-wrapper')).toContainText(`getByRole('button', { name: 'Submit' })`);
 });
 

tests/playwright-test/ui-mode-test-progress.spec.ts

@@ -73,7 +73,7 @@ test('should update trace live', async ({ runUITest, server }) => {
   onePromise.resolve();
 
   await expect(
-      page.frameLocator('id=snapshot').locator('body'),
+      page.frameLocator('iframe.snapshot-visible[name=snapshot]').locator('body'),
       'verify snapshot'
   ).toHaveText('One', { timeout: 15000 });
   await expect(listItem).toHaveText([
@@ -99,7 +99,7 @@ test('should update trace live', async ({ runUITest, server }) => {
   twoPromise.resolve();
 
   await expect(
-      page.frameLocator('id=snapshot').locator('body'),
+      page.frameLocator('iframe.snapshot-visible[name=snapshot]').locator('body'),
       'verify snapshot'
   ).toHaveText('Two');
 

tests/playwright-test/ui-mode-trace.spec.ts

@@ -89,7 +89,7 @@ test('should show snapshots for sync assertions', async ({ runUITest, server })
   ], { timeout: 15000 });
 
   await expect(
-      page.frameLocator('id=snapshot').locator('button'),
+      page.frameLocator('iframe.snapshot-visible[name=snapshot]').locator('button'),
       'verify snapshot'
   ).toHaveText('Submit');
 });