Skip to content

Commit

Permalink
chore(trace): remove dependency on handle._previewPromise (#3906)
Browse files Browse the repository at this point in the history
We now mark the target with '__playwright_target__' attribute and
let the trace viewer do whatever it wants.
  • Loading branch information
dgozman committed Sep 16, 2020
1 parent 73db4a4 commit 36f2420
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 15 deletions.
6 changes: 2 additions & 4 deletions src/server/dom.ts
Expand Up @@ -99,20 +99,18 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
readonly _context: FrameExecutionContext;
readonly _page: Page;
readonly _objectId: string;
readonly _previewPromise: Promise<string>;

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<T> | null {
Expand Down
11 changes: 7 additions & 4 deletions src/trace/snapshotter.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -113,14 +114,14 @@ export class Snapshotter {
this._delegate.onBlob({ sha1, buffer: body });
}

async takeSnapshot(page: Page, timeout: number): Promise<PageSnapshot | null> {
async takeSnapshot(page: Page, target: ElementHandle | undefined, timeout: number): Promise<PageSnapshot | null> {
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 = {
Expand Down Expand Up @@ -180,14 +181,16 @@ export class Snapshotter {
}, timeout).catch(e => null);
}

private async _snapshotFrame(progress: Progress, frame: Frame): Promise<FrameSnapshotAndMapping | null> {
private async _snapshotFrame(progress: Progress, target: ElementHandle | undefined, frame: Frame): Promise<FrameSnapshotAndMapping | null> {
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;

Expand Down
4 changes: 3 additions & 1 deletion src/trace/snapshotterInjected.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/trace/traceTypes.ts
Expand Up @@ -56,7 +56,7 @@ export type ActionTraceEvent = {
contextId: string,
action: string,
pageId?: string,
target?: string,
selector?: string,
label?: string,
value?: string,
startTime?: number,
Expand Down
10 changes: 5 additions & 5 deletions src/trace/tracer.ts
Expand Up @@ -137,7 +137,7 @@ class ContextTracer implements SnapshotterDelegate {
}

async captureSnapshot(page: Page, options: types.TimeoutOptions & { label?: string } = {}): Promise<void> {
const snapshot = await this._takeSnapshot(page, options.timeout);
const snapshot = await this._takeSnapshot(page, undefined, options.timeout);
if (!snapshot)
return;
const event: ActionTraceEvent = {
Expand All @@ -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,
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 36f2420

Please sign in to comment.