diff --git a/.changeset/follow-the-runtime-context.md b/.changeset/follow-the-runtime-context.md new file mode 100644 index 00000000..838ae2d6 --- /dev/null +++ b/.changeset/follow-the-runtime-context.md @@ -0,0 +1,16 @@ +--- +"@wdio/devtools-service": patch +"@wdio/devtools-app": patch +--- + +Capture a hybrid app's webview half as a page, and frame a mobile capture as a device in the trace player as well as live. + +**Following the context.** Document availability was answered from the startup capabilities and never revisited, so a session that switched into a webview was still treated as native: the collector injection, the DOM drain and the `__wdioSnapMark` tag stayed skipped, and its per-action snapshot read a real HTML document through the page-source XML reader. `sessionHasDocument(capabilities, context)` in shared is now the question a capture guard asks; `isNativeAppSession` remains the capability-level answer for what genuinely cannot change. Anything that is not Appium's `NATIVE_APP` counts as a webview, because the `WEBVIEW_` prefix is a convention and a driver naming its webview otherwise would have its capture skipped. Following it costs no round trip: `switchContext` carries the context it moves to in its own arguments, and a switch that failed is ignored. + +Verified on a real hybrid app (Appium's ApiDemos on an Android emulator): the webview action is exported with a page snapshot — `[Page: I am a page title — file:///android_asset/html/index.html]`, a heading, a link and a working locator — where the native actions on either side stay `[android] hierarchy FrameLayout…`. + +**Per-action snapshots are skipped where Appium has a document to probe, and nowhere else.** They are issued from inside the command hook, and Appium serialises a probe behind the command it is observing: a hybrid trace run measured the DIRECT transport timing out exactly as `browser.execute` had, so the serialisation is Appium's own and going round the client cannot escape it. That run spent 2m6s hitting timeouts where the same spec takes 34s untouched. + +The IN-PAGE probes are what hang, though, so the gate asks whether a document is in play rather than whether the driver is Appium. A native session passes no `runScript` at all — page source and a screenshot only — and completes fine: measured on an Android emulator, 13.6s against 5.5s with the capture skipped, no timeout. Gating on the driver instead left every native trace with **one** snapshot for the whole run, the one taken at its end, so all eleven actions of a sample spec replayed the final frame; it now carries ten, one per action. Because the context answers the question, a hybrid app is judged by the half it is currently in: its webview actions are still skipped, and those are the ones that carry no per-action element data, accessibility tree or settle screenshot. Command rows and their screenshots, console, network and the archive itself are unaffected, as is every desktop session. + +**The device column now applies in both modes.** It was live-only, on the reasoning that the player's own layout worked — but the player had never actually rendered one: `#deviceCapture` requires a measurable image, and both of its sources read `command.screenshot`, which a trace's commands never carry. So a native trace was framed as a desktop browser. The player now falls back to the recorded viewport when there is no screenshot to measure — second, not first, because a native screenshot's pixels and its window size genuinely differ. In that layout the capture takes a full-height column with the action list and the dock stacked beside it, and the playback controls ride above the capture. diff --git a/packages/app/src/components/browser/snapshot.ts b/packages/app/src/components/browser/snapshot.ts index 0efd9d99..63656c88 100644 --- a/packages/app/src/components/browser/snapshot.ts +++ b/packages/app/src/components/browser/snapshot.ts @@ -191,7 +191,30 @@ export class DevtoolsBrowser extends Element { get #captureSize(): ImageSize | null { const screenshot = this.#screenshotData ?? this.#latestAutoScreenshot if (!screenshot) { - return null + // A REPLAYED capture has no screenshot to measure: both sources above + // read `command.screenshot`, and a trace's commands carry none — its + // images are separate resources. Without a fallback no native trace + // could ever draw device chrome, because the shape was undecidable, and + // the player framed a phone as a desktop browser. + // + // Only a PORTRAIT viewport is trusted for it. The reader substitutes a + // synthetic 1280x720 for a trace that recorded no viewport, and that + // shape is indistinguishable from a real one here — so a capture whose + // geometry was never measured would be drawn as a landscape, + // desktop-proportioned "device". Landscape costs nothing to refuse: + // `#deviceLayout` already sends that shape to the stacked layout, so a + // frame is not wanted there either. + // + // Still a fallback, not the primary: a native screenshot's pixels and + // its window size genuinely differ (a Pixel reports 1080x2219 for a + // 1080x2400 shot), so the image is the truer answer when there is one. + const viewport = this.metadata?.viewport + const portrait = Boolean( + viewport?.width && viewport?.height && viewport.height > viewport.width + ) + return portrait + ? { width: viewport!.width, height: viewport!.height } + : null } if (this.#captureShape?.screenshot !== screenshot) { this.#captureShape = { screenshot, size: imageDimensions(screenshot) } diff --git a/packages/app/src/components/workbench.ts b/packages/app/src/components/workbench.ts index bc4d2367..07255f99 100644 --- a/packages/app/src/components/workbench.ts +++ b/packages/app/src/components/workbench.ts @@ -127,8 +127,12 @@ export class DevtoolsWorkbench extends Element { floor is min-content unless this is set, so switching to a wide tab (the Network table) grew it and shoved the device column sideways — only the drag handle may move that boundary. Scoped here rather than as - a utility class so it holds wherever the shadow root is styled from. */ - section[data-device-row] > wdio-devtools-tabs { + a utility class so it holds wherever the shadow root is styled from. + + A DESCENDANT selector, not a child one: the dock sits a level deeper + now that the action list and the dock share a column beside the + capture, and as a direct-child rule this silently stopped applying. */ + section[data-device-row] wdio-devtools-tabs { min-width: 0; } ` @@ -137,12 +141,18 @@ export class DevtoolsWorkbench extends Element { #dragVertical = new DragController(this, { localStorageKey: 'toolbarHeight', minPosition: minWorkbenchHeight, - maxPosition: () => window.innerHeight * 0.7, + maxPosition: () => this.#verticalSplitMax(), initialPosition: () => window.innerHeight * BROWSER_HEIGHT_RATIO, getContainerEl: () => this.#getVerticalWindow(), direction: Direction.vertical }) + /** Both layouts render the row, so this is right in either. */ + async #getDeviceRow() { + await this.updateComplete + return (this.deviceRow ?? this.verticalResizerWindow) as Element + } + async #getVerticalWindow() { await this.updateComplete return this.verticalResizerWindow as Element @@ -223,6 +233,34 @@ export class DevtoolsWorkbench extends Element { * The arithmetic is exact whenever the workbench fills the window, which is * every case but an embedded panel. */ + /** + * Ceiling for the vertical split. + * + * In the device layout the split lives in a COLUMN beside the capture, and + * that column is shorter than the window by the header, the playback + * controls and the timeline. A window-derived 70% therefore exceeded it, and + * because the controller's position is applied as a non-shrinking + * `flex-basis`, the action list could push the dock to zero and carry the + * handle outside the clipped row — leaving the split unreachable. + * + * Capped HERE rather than with a CSS `max-height`, which is what the earlier + * attempt did: the controller draws its grip from its own value and cannot + * see a cap the stylesheet applies, so the grip parts company with the + * boundary. Clamping the value keeps the two in step. + */ + /** Last composite column height, so a change in any of its inputs is seen. */ + #lastColumnHeight = 0 + + #verticalSplitMax(): number { + if (!this.#liveDeviceLayout) { + return window.innerHeight * 0.7 + } + return Math.max( + minWorkbenchHeight(), + this.#deviceColumnHeight() - minWorkbenchHeight() + ) + } + #deviceColumnHeight(): number { return Math.max( minWorkbenchHeight(), @@ -273,7 +311,13 @@ export class DevtoolsWorkbench extends Element { // "fills the height" the drag buys backdrop and costs the dock. maxPosition: () => this.#deviceFillWidth(), initialPosition: () => this.#deviceFillWidth(), - getContainerEl: () => this.#getVerticalWindow(), + // The ROW it divides, not the vertical split. Those were the same element + // while the dock sat beside the capture; now the vertical split is the + // column holding the action list and the dock, so measuring it clamped the + // capture against the dock's own box — and a 15px scrollbar appearing in a + // wide dock tab moved the column, which is exactly what this pane's tests + // forbid (seen on Linux, invisible on macOS's overlay scrollbars). + getContainerEl: () => this.#getDeviceRow(), direction: Direction.horizontal, // The pane is on the right, so its handle sits on its inner edge and // dragging left widens it. @@ -340,6 +384,9 @@ export class DevtoolsWorkbench extends Element { @query('section[data-vertical-resizer-window]') verticalResizerWindow?: HTMLElement + @query('section[data-device-row]') + deviceRow?: HTMLElement + // Height of the screencast pane; the dock fills the rest of the right column. // Collapsed dock → empty string so the browser flex-grows to fill. #computeBrowserPaneStyle(): string { @@ -576,11 +623,32 @@ export class DevtoolsWorkbench extends Element { * capture outlives it. Guarded on the property, so not a per-render pass. */ protected updated(changed: PropertyValues): void { - if ( - changed.has('metadata') && - this.#deviceLayout && - this.#dragDevice.refreshBounds() - ) { + if (!this.#deviceLayout) { + return + } + // The column's width derives from the capture's shape; the vertical + // split's ceiling derives from the column's HEIGHT, which the header, + // player mode and the DRAGGABLE timeline all feed. Metadata is therefore + // not the only input — enlarging the timeline, or entering player mode + // after a larger split was stored, leaves a non-shrinking flex-basis above + // its new maximum, and the dock collapses with its handle outside the + // clipped row. + // + // Watched through the composite height rather than each input, so a new + // contributor to it cannot be forgotten here. Still not a per-render pass: + // a pass that moves nothing requests no update. + const columnHeight = this.#deviceColumnHeight() + const inputsMoved = + changed.has('metadata') || + changed.has('playerMode') || + columnHeight !== this.#lastColumnHeight + this.#lastColumnHeight = columnHeight + if (!inputsMoved) { + return + } + const device = this.#dragDevice.refreshBounds() + const vertical = this.#dragVertical.refreshBounds() + if (device || vertical) { this.requestUpdate() } } @@ -594,6 +662,35 @@ export class DevtoolsWorkbench extends Element { * spent its left edge on the suite tree, so a third column squeezed the dock * into an unreadable strip and the tab row overflowed under the capture. */ + /** + * The action list's share of the column beside the capture — the TOP pane, + * because `#dragVertical` is start-anchored and its stored position is the + * top pane's height. Sizing the DOCK from that value put the handle and the + * boundary in different places: dragging down moved the handle down while + * growing the dock upward. + * + * Sized against the COLUMN, never the window. The window-derived + * `#computeBrowserPaneStyle` gave this pane a fixed height that could exceed + * the space it had, the column then overflowed its row, and an ancestor grew + * a scrollbar — which on a classic-scrollbar platform shifts the capture + * sideways. `max-height` is a percentage for the same reason: whatever the + * drag has stored, the column cannot be made to overflow. + */ + #deviceColumnTopStyle(): string { + if (this.#toolbarCollapsed) { + return 'flex:1 1 auto; min-height:0;' + } + // `getPosition()` applied LITERALLY, because the controller's contract is + // an inline `flex-basis: Npx`: `getSlider` draws the grip at that value + // and `#adjustPosition` finds the pane it resizes by matching that exact + // string. Expressed any other way — `height:Npx`, or a percentage default + // — the handle and the boundary part company, measured at 240px apart. + const pos = this.#dragVertical.getPosition() + return pos + ? `${pos}; flex-grow:0; flex-shrink:0; min-height:0;` + : 'flex:1 1 auto; min-height:0;' + } + #renderLiveDeviceLayout() { const width = basisPx(this.#dragDevice.getPosition()) return html` @@ -610,7 +707,7 @@ export class DevtoolsWorkbench extends Element { class="relative flex min-h-0 min-w-0 overflow-hidden ${ this.#workbenchSidebarCollapsed ? 'hidden' : '' }" - style="${this.#computeBrowserPaneStyle()}" + style="${this.#deviceColumnTopStyle()}" > ${this.#renderActionsSidebar()} @@ -625,7 +722,12 @@ export class DevtoolsWorkbench extends Element { ? this.#dragVertical.getSlider('z-[999] pointer-events-auto') : nothing } - ${this.#renderWorkbenchTabs()} +
+ ${this.#renderWorkbenchTabs()} +
${ !this.#toolbarCollapsed @@ -637,6 +739,15 @@ export class DevtoolsWorkbench extends Element { class="relative flex flex-col min-w-0 min-h-0 overflow-hidden" style="${this.#dragDevice.getPosition()}; flex:0 1 auto; width:${width}px; max-width:100%;" > + ${ + // Playback belongs with what it plays, so in this layout the + // controls ride above the capture rather than above the dock. + this.playerMode + ? html`` + : nothing + } ${this.#renderBrowserPane(true)} @@ -762,10 +873,11 @@ export class DevtoolsWorkbench extends Element { ` } - /** The capture-as-right-column arrangement, live only — the player keeps the - * dock beside the capture. */ + /** The capture-as-right-column arrangement. Applies to any device capture in + * either mode: a phone is the same tall frame whether it is being watched + * live or replayed, and the dock beside it was unreadable in both. */ get #liveDeviceLayout(): boolean { - return this.#deviceLayout && !this.playerMode + return this.#deviceLayout } } diff --git a/packages/app/test-ui/workbench/workbench.test.ts b/packages/app/test-ui/workbench/workbench.test.ts index 0dbc1050..0103b54b 100644 --- a/packages/app/test-ui/workbench/workbench.test.ts +++ b/packages/app/test-ui/workbench/workbench.test.ts @@ -796,12 +796,15 @@ describe('wdio-devtools-workbench', () => { const paneBox = pane!.getBoundingClientRect() const dockBox = dock.getBoundingClientRect() - // Beside, not under: the dock ends exactly where the column begins, and - // both span the same rows. + // Beside, not under: the dock ends where the column begins, and the + // column is the rightmost thing in the row. expect(dockBox.right).toBeCloseTo(paneBox.left, 0) - expect(dockBox.top).toBeCloseTo(paneBox.top, 0) - // ...and the column is the rightmost thing in the row. expect(paneBox.right).toBeGreaterThanOrEqual(dockBox.right) + // The dock no longer shares the column's top edge: it sits BELOW the + // action list, which owns the upper half of that left column. Beside + // the capture the dock was an unreadable strip once the suite tree had + // taken the left edge as well. + expect(dockBox.top).toBeGreaterThan(paneBox.top) }) it('lets the capture fill the whole column', async () => { @@ -827,8 +830,15 @@ describe('wdio-devtools-workbench', () => { paneOf(workbench)!, BROWSER )!.getBoundingClientRect() + // Playback rides above the capture INSIDE the column, so the capture + // fills what the controls leave rather than the whole pane. + const controls = shadow(paneOf(workbench)!, PLAYER_CONTROLS) + const controlsHeight = controls + ? controls.getBoundingClientRect().height + : 0 + expect(controlsHeight).toBeGreaterThan(0) expect(capture.width).toBeCloseTo(pane.width, 1) - expect(capture.height).toBeCloseTo(pane.height, 1) + expect(capture.height).toBeCloseTo(pane.height - controlsHeight, 1) }) it('leaves a desktop capture in the stacked layout', async () => { @@ -867,6 +877,22 @@ describe('wdio-devtools-workbench', () => { expect(getComputedStyle(dock).minWidth).toBe('0px') + // Sized out of flow, because an unsized host makes the geometry below + // measure the harness. The column is then CONTENT-sized (measured 61px, + // against 576px — the row minus the capture's basis — once sized), so + // its width tracks the dock's content and the platform's font metrics, + // which is the thing being asserted absent; and the workbench overflows + // the page, so a scrollbar arriving shifts every viewport-relative rect + // by its width where it takes layout space and by nothing where it + // overlays. A fixed host is outside the document's scroll area, so it + // settles both at once. + const host = workbench.parentElement as HTMLElement + host.style.position = 'fixed' + host.style.inset = '0' + host.style.overflow = 'hidden' + await workbench.updateComplete + await new Promise((resolve) => requestAnimationFrame(resolve)) + const before = paneOf(workbench)!.getBoundingClientRect() const network = shadowAll(dock, '[role="tab"], button').find( (el) => text(el).includes('Network') @@ -890,6 +916,39 @@ describe('wdio-devtools-workbench', () => { * harness does not apply Tailwind utilities inside a shadow root, so where * an absolutely positioned handle actually lands cannot be measured here. */ + /** + * `#dragVertical` is start-anchored: its stored position is the TOP pane's + * height. Sizing the DOCK from that value put the handle and the boundary + * in different places — dragging down moved the handle down while growing + * the dock upward, so the control stopped tracking what it resizes. + */ + it('puts the column handle on the boundary it moves', async () => { + const { workbench } = await mountWorkbench( + { metadata: IPHONE }, + { playerMode: true } + ) + const host = workbench.parentElement as HTMLElement + host.style.width = '1200px' + host.style.height = '800px' + await workbench.updateComplete + await new Promise((resolve) => requestAnimationFrame(resolve)) + + const column = shadow(workbench, 'section[data-vertical-resizer-window]')! + const actions = column.querySelector('section[data-sidebar]')! + // Scoped to the column: in player mode the timeline strip renders a + // row-resize handle too, and an unscoped search finds that one first. + const handle = Array.from( + column.querySelectorAll('button[data-draggable-id]') + ).find((el) => el.className.includes('cursor-row-resize')) + expect(handle).toBeTruthy() + + // The handle sits where the action list ends, which is where the dock + // begins. Within a few pixels: the handle is a grab strip with height. + const edge = actions.getBoundingClientRect().bottom + const grip = handle!.getBoundingClientRect() + expect(Math.abs(grip.top + grip.height / 2 - edge)).toBeLessThan(8) + }) + it('anchors the column handle to its own edge, not the row start', async () => { const { workbench } = await mountWorkbench( { metadata: IPHONE }, diff --git a/packages/core/src/session-capturer.ts b/packages/core/src/session-capturer.ts index 2a5e37c4..e7f66b3a 100644 --- a/packages/core/src/session-capturer.ts +++ b/packages/core/src/session-capturer.ts @@ -11,7 +11,11 @@ import type { TraceMutation } from '@wdio/devtools-shared' import { WORKER_WS_QUERY, WS_PATHS, WS_SCOPE } from '@wdio/devtools-shared' -import { isNativeAppSession, mapCommandToAction } from '@wdio/devtools-shared' +import { + isNativeAppSession, + mapCommandToAction, + sessionHasDocument +} from '@wdio/devtools-shared' import { resolveRunId } from './run-id.js' import { reattributeDomAnchors } from '@wdio/devtools-trace/trace-mutations' import { @@ -104,11 +108,30 @@ export abstract class SessionCapturerBase { /** Whether this session drove an app rather than a browser. Resolved from * the published metadata because selenium's own `getCapabilities()` is - * async, and a guard cannot await it where it has to decide. */ + * async, and a guard cannot await it where it has to decide. + * + * Answers from the startup bag, so it stays true for a hybrid app that has + * switched into a webview. Guards protecting a PAGE-side call want + * {@link hasDocument} instead. */ get isNativeAppSession(): boolean { return isNativeAppSession(this.metadata?.capabilities) } + /** The Appium context last observed, fed by the adapter's command hook. The + * switch command carries the new context in its own arguments, so following + * it costs no round trip — undefined means none was ever seen, which for a + * native session is the native context it started in. */ + currentContext: string | undefined + + /** Whether there is a web document to run page script in RIGHT NOW: true for + * any browser session, and for a native one only while it sits in a webview + * context. The question a capture guard should ask — `isNativeAppSession` + * cannot change after session start, so a hybrid app's webview portion was + * captured as if it had no DOM. */ + get hasDocument(): boolean { + return sessionHasDocument(this.metadata?.capabilities, this.currentContext) + } + // ── Construction ──────────────────────────────────────────────────────── constructor(opts: SessionCapturerOptions = {}) { const { hostname, port, reconnect } = opts diff --git a/packages/service/src/action-snapshot.ts b/packages/service/src/action-snapshot.ts index 4dccf4f3..b0834a89 100644 --- a/packages/service/src/action-snapshot.ts +++ b/packages/service/src/action-snapshot.ts @@ -13,7 +13,7 @@ import { mapCommandToAction, upsertRichestSnapshot } from '@wdio/devtools-core' -import { isNativeAppSession, type ActionSnapshot } from '@wdio/devtools-shared' +import { sessionHasDocument, type ActionSnapshot } from '@wdio/devtools-shared' import { mobilePlatform } from './mobile.js' import { directProbes } from './direct-probes.js' import { INTERNAL_COMMANDS } from './constants.js' @@ -69,21 +69,29 @@ export async function captureActionResult( browser: WebdriverIO.Browser, command: string, actionSnapshots: ActionSnapshot[], - stampTimestamp: () => number + stampTimestamp: () => number, + /** Appium context the session is in, so a hybrid app's webview takes the web + * path. Undefined for every non-Appium session, which has no contexts. */ + context?: string ): Promise { if (!mapCommandToAction(command) || INTERNAL_COMMANDS.includes(command)) { return } // Keyed on having a document, matching `#markDocument`, which writes the tag // this reads — split, a session tags a document nothing settles on. - if (!isNativeAppSession(browser.capabilities)) { + if (sessionHasDocument(browser.capabilities, context)) { await waitForActionResult(browser) } // Stamped before the capture, not after: a snapshot probe can never enter // commandsLog (beforeCommand requires an empty command stack), so the latest // logged action is the same either way — and reading it up front keeps the // stamp a capture input rather than a post-hoc mutation. - const snap = await captureActionSnapshot(browser, command, stampTimestamp()) + const snap = await captureActionSnapshot( + browser, + command, + stampTimestamp(), + context + ) if (snap) { upsertRichestSnapshot(actionSnapshots, snap) } @@ -98,9 +106,10 @@ export async function pushActionSnapshotAt( browser: WebdriverIO.Browser, command: string, timestamp: number, - actionSnapshots: ActionSnapshot[] + actionSnapshots: ActionSnapshot[], + context?: string ): Promise { - const snap = await captureActionSnapshot(browser, command, timestamp) + const snap = await captureActionSnapshot(browser, command, timestamp, context) if (snap) { upsertRichestSnapshot(actionSnapshots, snap) } @@ -109,11 +118,14 @@ export async function pushActionSnapshotAt( export function captureActionSnapshot( browser: WebdriverIO.Browser, command: string, - timestamp?: number + timestamp?: number, + context?: string ): Promise { // A mobile BROWSER session takes the web path below: it has a document, and - // the native path would read its HTML through the page-source XML parser. - const native = isNativeAppSession(browser.capabilities) + // the native path would read its HTML through the page-source XML parser. So + // does a hybrid app while it sits in a webview context — its DOM is real, and + // reading it as page-source XML loses the whole replay. + const native = !sessionHasDocument(browser.capabilities, context) // A driver that serialises per session deadlocks on a probe issued from // inside the command hook, so those go straight to it (#374). const direct = directProbes(browser) diff --git a/packages/service/src/index.ts b/packages/service/src/index.ts index 70a13f3d..5d7ef87b 100644 --- a/packages/service/src/index.ts +++ b/packages/service/src/index.ts @@ -34,7 +34,7 @@ import { captureActionSnapshot } from './action-snapshot.js' import { - isNativeAppSession, + sessionHasDocument, type ActionSnapshot, type TestMetadataMap } from '@wdio/devtools-shared' @@ -60,7 +60,7 @@ import { LOCATOR_COMMANDS, PAGE_TRANSITION_COMMANDS } from './constants.js' -import { isAppiumSession } from './mobile.js' +import { inPageProbesDeadlock, isAppiumSession } from './mobile.js' import { directProbes } from './direct-probes.js' import { resolveSessionMetadata } from './session-metadata.js' import { stampRunnerMetadata } from './wdio-runner-id.js' @@ -655,17 +655,25 @@ export default class DevToolsHookService implements Services.ServiceInstance { } // Pre-action capture: state BEFORE this action executes. Stamped at the // previous action's end time (or 0 for the first). Trace mode only. + // + // Not while Appium has a document to probe — see `inPageProbesDeadlock`, + // which carries the measurements. A native session is captured normally. if ( topLevelUserCommand && this.#options.mode === 'trace' && this.#browser && + !inPageProbesDeadlock( + this.#browser, + this.#sessionCapturer.currentContext + ) && mapCommandToAction(command) && !INTERNAL_COMMANDS.includes(command) ) { const snap = await captureActionSnapshot( this.#browser, command, - this.#lastActionTimestamp() + this.#lastActionTimestamp(), + this.#sessionCapturer.currentContext ) if (snap) { upsertRichestSnapshot(this.#actionSnapshots, snap) @@ -679,7 +687,13 @@ export default class DevToolsHookService implements Services.ServiceInstance { #markDocument(): Promise { // Keyed on having a document: `waitForActionResult` reads this tag on the // same condition, so the pair must not be split across the two predicates. - if (!this.#browser || isNativeAppSession(this.#browser.capabilities)) { + if ( + !this.#browser || + !sessionHasDocument( + this.#browser.capabilities, + this.#sessionCapturer.currentContext + ) + ) { return Promise.resolve() } // Issued from inside beforeCommand, so it takes the direct path on a @@ -740,12 +754,21 @@ export default class DevToolsHookService implements Services.ServiceInstance { this.#currentTestUid, this.#currentStepUid ) - if (this.#options.mode === 'trace') { + // Paired with the pre-action capture above, and gated on the same + // question: this settles and screenshots from inside the command hook. + if ( + this.#options.mode === 'trace' && + !inPageProbesDeadlock( + this.#browser, + this.#sessionCapturer.currentContext + ) + ) { await captureActionResult( this.#browser, command, this.#actionSnapshots, - () => this.#lastActionTimestamp() + () => this.#lastActionTimestamp(), + this.#sessionCapturer.currentContext ) } else { await this.#drainAfterLiveCommand(command) diff --git a/packages/service/src/mobile.ts b/packages/service/src/mobile.ts index 25c01031..5a5b2f60 100644 --- a/packages/service/src/mobile.ts +++ b/packages/service/src/mobile.ts @@ -1,3 +1,5 @@ +import { sessionHasDocument } from '@wdio/devtools-shared' + // Mobile-aware browser — Appium sessions expose `isMobile`, `isAndroid`, // `isIOS` at runtime. These flags are absent from WDIO's published types // so we narrow through a single cast here rather than repeating @@ -16,6 +18,33 @@ export function isAppiumSession(browser: WebdriverIO.Browser): boolean { return Boolean(b.isMobile || b.isAndroid || b.isIOS) } +/** + * Whether a per-action snapshot issued from inside the command hook would + * deadlock. Appium serialises a probe behind the command it is observing, and + * the IN-PAGE probes are the ones that hang: measured against a hybrid webview, + * `browser.execute` and the raw-HTTP transport timed out alike and a run took + * 2m6s where the same spec takes 34s untouched (#374). + * + * A NATIVE session issues none of them — `captureActionSnapshot` passes no + * `runScript` there, only page source and a screenshot — and completes fine: + * measured 13.6s against 5.5s with the capture skipped, no timeout. Skipping it + * for every Appium session therefore cost each native trace its per-action data + * to avoid a hazard native does not have, leaving a whole run sharing the one + * snapshot taken at its end. + * + * So the question is a document, not a driver — and the context answers it, so + * a hybrid app is judged by the half it is currently in. + */ +export function inPageProbesDeadlock( + browser: WebdriverIO.Browser, + context?: string +): boolean { + return ( + isAppiumSession(browser) && + sessionHasDocument(browser.capabilities, context) + ) +} + export function mobilePlatform( browser: WebdriverIO.Browser ): 'android' | 'ios' | undefined { diff --git a/packages/service/src/session.ts b/packages/service/src/session.ts index 8cd17b5e..0e62666c 100644 --- a/packages/service/src/session.ts +++ b/packages/service/src/session.ts @@ -15,7 +15,7 @@ import { rememberElementSelector, selectorForCommand } from './command-selectors.js' -import { isNativeAppSession } from '@wdio/devtools-shared' +import { isNativeAppSession, sessionHasDocument } from '@wdio/devtools-shared' import { isAppiumSession } from './mobile.js' import { CAPTURE_PERFORMANCE_SCRIPT, @@ -206,12 +206,24 @@ export class SessionCapturer extends SessionCapturerBase { selectorForCommand(args, this.#lastSelector) ) + // A hybrid app's webview HAS a document, and the switch command carries + // the context it moved to, so following it costs nothing. Recorded after + // the command so a failed switch does not move the capture's idea of where + // the session is. + // Appium's command, so not in WebDriverCommands — compared as a string. + if (String(command) === 'switchContext' && !error) { + this.currentContext = + typeof args[0] === 'string' + ? args[0] + : (args[0] as { name?: string })?.name + } + this.#captureOrReplace(commandLogEntry) // Capture trace + perf on commands that could trigger a page transition. // Skipped when there is no document to run either script in; a mobile // BROWSER session has one, so it keeps both. if ( - !isNativeAppSession(browser.capabilities) && + sessionHasDocument(browser.capabilities, this.currentContext) && PAGE_TRANSITION_COMMANDS.includes(command) ) { await Promise.all([ @@ -419,7 +431,7 @@ export class SessionCapturer extends SessionCapturerBase { // recovery injection and the url read are all round trips that can only // fail. Guarded here rather than at each call site, because two of the four // asked and two did not. - if (isNativeAppSession(browser.capabilities)) { + if (!sessionHasDocument(browser.capabilities, this.currentContext)) { return } // No `#isScriptInjected` gate: that flag tracks the preload REGISTRATION, diff --git a/packages/service/tests/mobile.test.ts b/packages/service/tests/mobile.test.ts index 9a440082..e9912ba7 100644 --- a/packages/service/tests/mobile.test.ts +++ b/packages/service/tests/mobile.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from 'vitest' -import { isAppiumSession } from '../src/mobile.js' +import { NATIVE_APP_CONTEXT } from '@wdio/devtools-shared' + +import { inPageProbesDeadlock, isAppiumSession } from '../src/mobile.js' /** * `isAppiumSession` answers "can this session serve WebDriver BiDi", and @@ -39,3 +41,44 @@ describe('isAppiumSession', () => { expect(isAppiumSession(session({}))).toBe(false) }) }) + +/** + * The per-action snapshot is issued from inside the command hook, and Appium + * serialises it behind the command it observes. Only the IN-PAGE probes hang + * there, so the gate has to ask whether a document is in play rather than + * whether the driver is Appium — the blanket answer left a native trace with + * one snapshot for the whole run, taken at its end. + */ +const appium = (caps: Record) => + ({ isMobile: true, isAndroid: true, capabilities: caps }) as never + +const NATIVE_CAPS = { + platformName: 'Android', + 'appium:automationName': 'UiAutomator2' +} +const MOBILE_WEB_CAPS = { platformName: 'Android', browserName: 'chrome' } + +describe('inPageProbesDeadlock', () => { + it('clears a native session, which runs no in-page script', () => { + expect(inPageProbesDeadlock(appium(NATIVE_CAPS))).toBe(false) + expect(inPageProbesDeadlock(appium(NATIVE_CAPS), NATIVE_APP_CONTEXT)).toBe( + false + ) + }) + + it('holds for the webview half of that same session', () => { + expect(inPageProbesDeadlock(appium(NATIVE_CAPS), 'WEBVIEW_com.x')).toBe( + true + ) + }) + + it('holds for a mobile browser, which is all document', () => { + expect(inPageProbesDeadlock(appium(MOBILE_WEB_CAPS))).toBe(true) + }) + + it('clears a desktop session, which is not serialised at all', () => { + expect( + inPageProbesDeadlock({ capabilities: { browserName: 'chrome' } } as never) + ).toBe(false) + }) +}) diff --git a/packages/service/tests/session.test.ts b/packages/service/tests/session.test.ts index 59de683b..7289d910 100644 --- a/packages/service/tests/session.test.ts +++ b/packages/service/tests/session.test.ts @@ -1051,6 +1051,70 @@ describe('SessionCapturer', () => { expect(row.id).toBeUndefined() // still no cross-spec-colliding public id }) }) + // #376: the switch command carries the context it moved to, so the capture + // follows a hybrid app between its native and webview halves without a round + // trip of its own. + describe('following the Appium context', () => { + const nativeBrowser = () => + ({ + ...mockBrowser, + isMobile: true, + capabilities: { + platformName: 'Android', + 'appium:automationName': 'UiAutomator2' + } + }) as never + + const switchTo = async ( + capturer: SessionCapturer, + context: unknown, + error?: Error + ) => + capturer.afterCommand( + nativeBrowser(), + 'switchContext' as never, + [context], + undefined, + error, + undefined + ) + + it('starts with no document, which is the native context it booted in', () => { + const capturer = new SessionCapturer() + expect(capturer.currentContext).toBeUndefined() + }) + + it('records the context a successful switch moved to', async () => { + const capturer = new SessionCapturer() + await switchTo(capturer, 'WEBVIEW_com.example') + expect(capturer.currentContext).toBe('WEBVIEW_com.example') + }) + + it('accepts the object form the protocol also takes', async () => { + const capturer = new SessionCapturer() + await switchTo(capturer, { name: 'WEBVIEW_com.example' }) + expect(capturer.currentContext).toBe('WEBVIEW_com.example') + }) + + // Moving the capture's idea of where the session is on a failed switch + // would point every page-side probe at a context that was never entered. + it('ignores a switch that failed', async () => { + const capturer = new SessionCapturer() + await switchTo( + capturer, + 'WEBVIEW_com.example', + new Error('no such context') + ) + expect(capturer.currentContext).toBeUndefined() + }) + + it('follows the session back into the native context', async () => { + const capturer = new SessionCapturer() + await switchTo(capturer, 'WEBVIEW_com.example') + await switchTo(capturer, 'NATIVE_APP') + expect(capturer.currentContext).toBe('NATIVE_APP') + }) + }) }) /** diff --git a/packages/shared/src/device.ts b/packages/shared/src/device.ts index 9c90614b..006abb92 100644 --- a/packages/shared/src/device.ts +++ b/packages/shared/src/device.ts @@ -151,10 +151,12 @@ export function deviceFromCapabilities( * So a Mac2 or tvOS session, which `NATIVE_PLATFORMS` excludes because that * list chooses a device frame, has to answer true here too. * - * Residual: a hybrid app switched into a webview context does have a document, - * and no capability can say so — only a runtime context read knows that. And a - * bag this cannot read at all answers false, which is the expensive direction; - * in practice every adapter reads capabilities straight off its own session. + * Answers from CAPABILITIES alone, so a hybrid app switched into a webview is + * still "native" here — that session does have a document, and only a runtime + * context read knows it. `sessionHasDocument` is the context-aware answer and + * is what a capture guard should ask. A bag this cannot read at all answers + * false, which is the expensive direction; in practice every adapter reads + * capabilities straight off its own session. */ export function isNativeAppSession(capabilities: unknown): boolean { if (!capabilities || typeof capabilities !== 'object') { @@ -167,6 +169,43 @@ export function isNativeAppSession(capabilities: unknown): boolean { return !deepCapString(caps, 'browserName') } +/** Appium's name for the context a native app runs in. Every other context it + * reports is a webview, conventionally `WEBVIEW_`. */ +export const NATIVE_APP_CONTEXT = 'NATIVE_APP' + +/** + * Whether an Appium context is a webview, i.e. one that has a document. + * + * Anything that is not the native context counts, rather than matching + * `WEBVIEW_` — the prefix is a convention, and a driver naming its webview + * differently would otherwise be read as native and have its DOM capture + * skipped. An unknown context (nothing has been switched to yet) is not a + * webview: a hybrid session starts in the native one. + */ +export function isWebviewContext(context: unknown): boolean { + return typeof context === 'string' && context.length > 0 + ? context !== NATIVE_APP_CONTEXT + : false +} + +/** + * Whether the session has a web document to run page script in RIGHT NOW. + * + * The question every capture guard actually wants. `isNativeAppSession` answers + * from the startup bag and cannot change, so a hybrid app that switches into a + * webview kept being treated as native and its webview portion carried no DOM. + * + * `context` is what the adapter last observed; undefined means it has not seen + * a context switch, which for a native session is the native context it + * started in. + */ +export function sessionHasDocument( + capabilities: unknown, + context?: string +): boolean { + return !isNativeAppSession(capabilities) || isWebviewContext(context) +} + /** * Narrow a `device` read back off a trace's `context-options`. The field is * untrusted — a foreign zip may carry anything under that name, and one of ours diff --git a/packages/shared/tests/session-has-document.test.ts b/packages/shared/tests/session-has-document.test.ts new file mode 100644 index 00000000..2283603b --- /dev/null +++ b/packages/shared/tests/session-has-document.test.ts @@ -0,0 +1,60 @@ +/** + * #376: `isNativeAppSession` answers from the startup bag and cannot change, so + * a hybrid app that switched into a webview kept being treated as native and + * its webview portion carried no DOM at all. + */ + +import { describe, it, expect } from 'vitest' +import { + NATIVE_APP_CONTEXT, + isWebviewContext, + sessionHasDocument +} from '../src/device.js' + +const NATIVE = { + platformName: 'Android', + 'appium:automationName': 'UiAutomator2' +} +const MOBILE_WEB = { platformName: 'Android', browserName: 'chrome' } +const DESKTOP = { browserName: 'chrome', platformName: 'linux' } + +describe('isWebviewContext', () => { + it('treats anything that is not the native context as a webview', () => { + expect(isWebviewContext('WEBVIEW_com.example')).toBe(true) + expect(isWebviewContext('WEBVIEW_chrome')).toBe(true) + // The WEBVIEW_ prefix is a convention, not a guarantee — matching on it + // would read a differently-named webview as native and skip its capture. + expect(isWebviewContext('CHROMIUM')).toBe(true) + }) + + it('treats the native context, and no context at all, as not a webview', () => { + expect(isWebviewContext(NATIVE_APP_CONTEXT)).toBe(false) + expect(isWebviewContext(undefined)).toBe(false) + expect(isWebviewContext('')).toBe(false) + }) +}) + +describe('sessionHasDocument', () => { + // A hybrid session starts in the native context, so no observed context is + // the native one rather than an unknown. + it('answers false for a native session that has not switched', () => { + expect(sessionHasDocument(NATIVE)).toBe(false) + expect(sessionHasDocument(NATIVE, NATIVE_APP_CONTEXT)).toBe(false) + }) + + it('answers true once that session enters a webview', () => { + expect(sessionHasDocument(NATIVE, 'WEBVIEW_com.example')).toBe(true) + }) + + it('answers false again when it switches back', () => { + expect(sessionHasDocument(NATIVE, NATIVE_APP_CONTEXT)).toBe(false) + }) + + // A browser session has a document regardless; it has no contexts to switch. + it('is unaffected by context for a browser session', () => { + for (const context of [undefined, NATIVE_APP_CONTEXT, 'WEBVIEW_x']) { + expect(sessionHasDocument(MOBILE_WEB, context)).toBe(true) + expect(sessionHasDocument(DESKTOP, context)).toBe(true) + } + }) +})