From b4dd24d3daadc7075a6447ddd9c29729b0f640d3 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 22 Oct 2021 21:28:55 -0700 Subject: [PATCH] fix(snapshot): empty adopted stylesheet should not prevent node refs We never marked empty stylesheets as "stale", so we never computed css text for them. This prevented node reuse, because empty string is not equal to undefined. --- .../trace/recorder/snapshotterInjected.ts | 7 +++-- tests/snapshotter.spec.ts | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts b/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts index 66b3a74aedae0..31c9712992f40 100644 --- a/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts +++ b/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts @@ -145,14 +145,15 @@ export function frameSnapshotStreamer(snapshotStreamer: string) { this._staleStyleSheets.add(sheet); } - private _updateStyleElementStyleSheetTextIfNeeded(sheet: CSSStyleSheet): string | undefined { + private _updateStyleElementStyleSheetTextIfNeeded(sheet: CSSStyleSheet, forceText?: boolean): string | undefined { const data = ensureCachedData(sheet); - if (this._staleStyleSheets.has(sheet)) { + if (this._staleStyleSheets.has(sheet) || (forceText && data.cssText === undefined)) { this._staleStyleSheets.delete(sheet); try { data.cssText = this._getSheetText(sheet); } catch (e) { // Sometimes we cannot access cross-origin stylesheets. + data.cssText = ''; } } return data.cssText; @@ -438,7 +439,7 @@ export function frameSnapshotStreamer(snapshotStreamer: string) { const visitStyleSheet = (sheet: CSSStyleSheet) => { const data = ensureCachedData(sheet); const oldCSSText = data.cssText; - const cssText = this._updateStyleElementStyleSheetTextIfNeeded(sheet) || ''; + const cssText = this._updateStyleElementStyleSheetTextIfNeeded(sheet, true /* forceText */)!; if (cssText === oldCSSText) return { equals: true, n: [[ snapshotNumber - data.ref![0], data.ref![1] ]] }; data.ref = [snapshotNumber, nodeCounter++]; diff --git a/tests/snapshotter.spec.ts b/tests/snapshotter.spec.ts index 89f8094e02308..84edbf08f157a 100644 --- a/tests/snapshotter.spec.ts +++ b/tests/snapshotter.spec.ts @@ -180,6 +180,35 @@ it.describe('snapshots', () => { expect(distillSnapshot(snapshot)).toBe(''); } }); + + it('empty adopted style sheets should not prevent node refs', async ({ page, toImpl, snapshotter, browserName }) => { + it.skip(browserName !== 'chromium', 'Constructed stylesheets are only in Chromium.'); + + await page.setContent(''); + await page.evaluate(() => { + const sheet = new CSSStyleSheet(); + (document as any).adoptedStyleSheets = [sheet]; + + const sheet2 = new CSSStyleSheet(); + for (const element of [document.createElement('div'), document.createElement('span')]) { + const root = element.attachShadow({ + mode: 'open' + }); + root.append('foo'); + (root as any).adoptedStyleSheets = [sheet2]; + document.body.appendChild(element); + } + }); + + const renderer1 = await snapshotter.captureSnapshot(toImpl(page), 'snapshot1'); + // Expect some adopted style sheets. + expect(distillSnapshot(renderer1)).toContain('__playwright_style_sheet_'); + + const renderer2 = await snapshotter.captureSnapshot(toImpl(page), 'snapshot2'); + const snapshot2 = renderer2.snapshot(); + // Second snapshot should be just a copy of the first one. + expect(snapshot2.html).toEqual([[1, 13]]); + }); }); function distillSnapshot(snapshot, distillTarget = true) {