From 36f2420b0fdef4f57d96ce157f04ea086c5a1de9 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 16 Sep 2020 15:26:59 -0700 Subject: [PATCH] chore(trace): remove dependency on handle._previewPromise (#3906) We now mark the target with '__playwright_target__' attribute and let the trace viewer do whatever it wants. --- src/server/dom.ts | 6 ++---- src/trace/snapshotter.ts | 11 +++++++---- src/trace/snapshotterInjected.ts | 4 +++- src/trace/traceTypes.ts | 2 +- src/trace/tracer.ts | 10 +++++----- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/server/dom.ts b/src/server/dom.ts index 2beab23dad82d..b025e8500f35c 100644 --- a/src/server/dom.ts +++ b/src/server/dom.ts @@ -99,20 +99,18 @@ export class ElementHandle extends js.JSHandle { readonly _context: FrameExecutionContext; readonly _page: Page; readonly _objectId: string; - readonly _previewPromise: Promise; constructor(context: FrameExecutionContext, objectId: string) { super(context, 'node', objectId); this._objectId = objectId; this._context = context; this._page = context.frame._page; - this._previewPromise = this._initializePreview().catch(e => 'node'); - this._previewPromise.then(preview => this._setPreview('JSHandle@' + preview)); + this._initializePreview().catch(e => {}); } async _initializePreview() { const utility = await this._context.injectedScript(); - return utility.evaluate((injected, e) => injected.previewNode(e), this); + this._setPreview(await utility.evaluate((injected, e) => 'JSHandle@' + injected.previewNode(e), this)); } asElement(): ElementHandle | null { diff --git a/src/trace/snapshotter.ts b/src/trace/snapshotter.ts index 60f9a2d9a5f67..d826f156e4933 100644 --- a/src/trace/snapshotter.ts +++ b/src/trace/snapshotter.ts @@ -25,6 +25,7 @@ import * as js from '../server/javascript'; import * as types from '../server/types'; import { SnapshotData, takeSnapshotInFrame } from './snapshotterInjected'; import { assert, calculateSha1, createGuid } from '../utils/utils'; +import { ElementHandle } from '../server/dom'; export type SnapshotterResource = { pageId: string, @@ -113,14 +114,14 @@ export class Snapshotter { this._delegate.onBlob({ sha1, buffer: body }); } - async takeSnapshot(page: Page, timeout: number): Promise { + async takeSnapshot(page: Page, target: ElementHandle | undefined, timeout: number): Promise { assert(page.context() === this._context); const frames = page.frames(); const frameSnapshotPromises = frames.map(async frame => { // TODO: use different timeout depending on the frame depth/origin // to avoid waiting for too long for some useless frame. - const frameResult = await runAbortableTask(progress => this._snapshotFrame(progress, frame), timeout).catch(e => null); + const frameResult = await runAbortableTask(progress => this._snapshotFrame(progress, target, frame), timeout).catch(e => null); if (frameResult) return frameResult; const frameSnapshot = { @@ -180,14 +181,16 @@ export class Snapshotter { }, timeout).catch(e => null); } - private async _snapshotFrame(progress: Progress, frame: Frame): Promise { + private async _snapshotFrame(progress: Progress, target: ElementHandle | undefined, frame: Frame): Promise { if (!progress.isRunning()) return null; + if (target && (await target.ownerFrame()) !== frame) + target = undefined; const context = await frame._utilityContext(); const guid = createGuid(); const removeNoScript = !frame._page.context()._options.javaScriptEnabled; - const result = await js.evaluate(context, false /* returnByValue */, takeSnapshotInFrame, guid, removeNoScript) as js.JSHandle; + const result = await js.evaluate(context, false /* returnByValue */, takeSnapshotInFrame, guid, removeNoScript, target) as js.JSHandle; if (!progress.isRunning()) return null; diff --git a/src/trace/snapshotterInjected.ts b/src/trace/snapshotterInjected.ts index d79e9a82b7ac5..c49153e1e2081 100644 --- a/src/trace/snapshotterInjected.ts +++ b/src/trace/snapshotterInjected.ts @@ -25,7 +25,7 @@ type SnapshotResult = { frameElements: Element[], }; -export function takeSnapshotInFrame(guid: string, removeNoScript: boolean): SnapshotResult { +export function takeSnapshotInFrame(guid: string, removeNoScript: boolean, target: Node | undefined): SnapshotResult { const shadowAttribute = 'playwright-shadow-root'; const win = window; const doc = win.document; @@ -161,6 +161,8 @@ export function takeSnapshotInFrame(guid: string, removeNoScript: boolean): Snap const element = node as Element; builder.push('<'); builder.push(nodeName); + if (node === target) + builder.push(' __playwright_target__="true"'); for (let i = 0; i < element.attributes.length; i++) { const name = element.attributes[i].name; let value = element.attributes[i].value; diff --git a/src/trace/traceTypes.ts b/src/trace/traceTypes.ts index 591064cc4a70a..32b384a771070 100644 --- a/src/trace/traceTypes.ts +++ b/src/trace/traceTypes.ts @@ -56,7 +56,7 @@ export type ActionTraceEvent = { contextId: string, action: string, pageId?: string, - target?: string, + selector?: string, label?: string, value?: string, startTime?: number, diff --git a/src/trace/tracer.ts b/src/trace/tracer.ts index 417313c54d7fd..a6627f0134493 100644 --- a/src/trace/tracer.ts +++ b/src/trace/tracer.ts @@ -137,7 +137,7 @@ class ContextTracer implements SnapshotterDelegate { } async captureSnapshot(page: Page, options: types.TimeoutOptions & { label?: string } = {}): Promise { - const snapshot = await this._takeSnapshot(page, options.timeout); + const snapshot = await this._takeSnapshot(page, undefined, options.timeout); if (!snapshot) return; const event: ActionTraceEvent = { @@ -152,13 +152,13 @@ class ContextTracer implements SnapshotterDelegate { } async recordAction(result: ActionResult, metadata: ActionMetadata) { - const snapshot = await this._takeSnapshot(metadata.page); + const snapshot = await this._takeSnapshot(metadata.page, typeof metadata.target === 'string' ? undefined : metadata.target); const event: ActionTraceEvent = { type: 'action', contextId: this._contextId, pageId: this._pageToId.get(metadata.page), action: metadata.type, - target: metadata.target instanceof ElementHandle ? await metadata.target._previewPromise : metadata.target, + selector: typeof metadata.target === 'string' ? metadata.target : undefined, value: metadata.value, snapshot, startTime: result.startTime, @@ -194,14 +194,14 @@ class ContextTracer implements SnapshotterDelegate { }); } - private async _takeSnapshot(page: Page, timeout: number = 0): Promise<{ sha1: string, duration: number } | undefined> { + private async _takeSnapshot(page: Page, target: ElementHandle | undefined, timeout: number = 0): Promise<{ sha1: string, duration: number } | undefined> { if (!timeout) { // Never use zero timeout to avoid stalling because of snapshot. // Use 20% of the default timeout. timeout = (page._timeoutSettings.timeout({}) || DEFAULT_TIMEOUT) / 5; } const startTime = monotonicTime(); - const snapshot = await this._snapshotter.takeSnapshot(page, timeout); + const snapshot = await this._snapshotter.takeSnapshot(page, target, timeout); if (!snapshot) return; const buffer = Buffer.from(JSON.stringify(snapshot));