From 9c42d78b2578a598e32b8db63dc8c3b21012a603 Mon Sep 17 00:00:00 2001 From: Torrey Betts Date: Fri, 23 Oct 2026 10:59:31 -0400 Subject: [PATCH 1/3] Track execution contexts across navigations and wait for a frame's next one An evaluation straight after a navigation could fail with "Cannot find context with specified id". The page recorded each frame's main world from Runtime.executionContextCreated only, and nothing dropped it when the browser tore it down, so the old document's id stayed recorded until the new one's event was handled. That event and Page.loadEventFired are read on separate loops, so GotoAsync could return first and the next EvaluateAsync, TitleAsync or ContentAsync named a dead context. Runtime.executionContextDestroyed and Runtime.executionContextsCleared now drop what they name, matched by session as well as id, since ids are numbered per renderer. The three context events are read from one ordered channel per session, so a teardown is never handled after the context that replaces it. Commands that name a context go through one path: a frame other than the main frame with nothing recorded waits for its next context, bounded by the caller's timeout or 30 seconds, and fails at once if the frame is detached or the page closes, instead of leaving the id out and running in the main frame. The main frame never waits, since leaving the id out reaches its current main world. A command refused with the stale-context error is sent once more; the browser refuses it before running anything, so this cannot run a script twice. Evaluations, handles, selector resolution and script and style tags all take this path, and the isolated world is rebuilt and retried the same way. BiDi addresses script by browsing context and has no ids to go stale, so it is left as it was. Signed-off-by: Torrey Betts --- docs/architecture/transport-and-protocol.md | 2 + docs/guides/frames-and-iframes.md | 6 +- src/Motus/Page/Page.Evaluation.cs | 37 +- src/Motus/Page/Page.Events.cs | 41 +- src/Motus/Page/Page.ExecutionContexts.cs | 375 +++++++++++++++ src/Motus/Page/Page.Viewport.cs | 42 +- src/Motus/Page/Page.cs | 72 +-- src/Motus/Protocol/CdpJsonContext.cs | 4 + .../Selectors/SelectorStrategyHelpers.cs | 49 +- .../Browser/FirefoxIntegrationTests.cs | 32 ++ .../Page/ExecutionContextIntegrationTests.cs | 160 +++++++ .../Page/ExecutionContextTrackingTests.cs | 443 ++++++++++++++++++ 12 files changed, 1128 insertions(+), 135 deletions(-) create mode 100644 src/Motus/Page/Page.ExecutionContexts.cs create mode 100644 tests/Motus.Tests/Page/ExecutionContextIntegrationTests.cs create mode 100644 tests/Motus.Tests/Page/ExecutionContextTrackingTests.cs diff --git a/docs/architecture/transport-and-protocol.md b/docs/architecture/transport-and-protocol.md index bbf7129..891a3c0 100644 --- a/docs/architecture/transport-and-protocol.md +++ b/docs/architecture/transport-and-protocol.md @@ -101,6 +101,8 @@ Three kinds of target get a session of their own: **Execution context IDs are numbered per session.** Two renderers will both hand out context 1, so the map from context to frame is keyed by session and context together. A frame's context ID is only meaningful on the session that owns that frame, and every call site pairs the two. +**A frame's context ID is replaced on every navigation.** `Runtime.executionContextCreated` records the main world of the frame named in its `auxData` (an isolated world, with `isDefault: false`, never replaces it), `Runtime.executionContextDestroyed` drops the entry with that ID from the session that reported it, and `Runtime.executionContextsCleared` drops every entry that session reported. The three are read from one channel per session, so a navigation's teardown is always handled before the context that replaces it. They do not share a channel with `Page.loadEventFired`, so a navigation can complete while the old context is still recorded or before the new one is. Commands that name a context cover both cases. A frame with nothing recorded waits for its next context, bounded by the caller's timeout or 30 seconds, and fails at once with `MotusTargetClosedException` if the frame is detached or the page closes; the main frame instead leaves the ID out, which the browser resolves to the main frame's current main world. A command refused with "Cannot find context with specified id" is sent once more against whatever has been recorded since. The browser looks the context up before running anything, so the refused attempt never ran and the retry cannot run a script twice. BiDi has no counterpart: script there is addressed to a browsing context and runs in whatever realm it currently holds, so there is no ID to track or wait for. + ### Routing a frame to its session `Page.SessionFor(IFrame?)` is the single lookup. A frame that owns a session is routed to it; every other frame, including a same-process child of an out-of-process frame, falls through to the page's own session. That fallback is what keeps every existing round trip byte-identical. diff --git a/docs/guides/frames-and-iframes.md b/docs/guides/frames-and-iframes.md index 1d4881b..9a3c831 100644 --- a/docs/guides/frames-and-iframes.md +++ b/docs/guides/frames-and-iframes.md @@ -90,7 +90,9 @@ if (frame.IsDetached) } ``` -`IsDetached` is how a removed frame is told apart from a selector that simply did not match. A held `IFrame` answers rather than failing obscurely several calls later. +`IsDetached` is how a removed frame is told apart from a selector that simply did not match. A held `IFrame` answers rather than failing obscurely several calls later. Evaluating, reading content or resolving a locator through a detached frame throws `MotusTargetClosedException` straight away, and so does a call that was already waiting on the frame when it went. + +An evaluation, a locator or a content read issued the moment a frame finishes navigating runs in the new document. A frame's main world is replaced on every navigation and the browser reports the new one separately from the end of the navigation, so for a frame other than the main frame such a call waits for the new world to be reported, for up to 30 seconds or the call's own timeout, rather than running against the old document or in the main frame by mistake. --- @@ -159,7 +161,7 @@ Each of these costs real time to rediscover. **The endpoint's target list is not a list of a page's frames.** `http://127.0.0.1:9222/json/list` reports one entry per CDP target, and only a frame the browser put in its own process has a target of its own. So the list shows those, typed `iframe` rather than `page`, and shows nothing at all for every frame the page's own renderer hosts. A document with two frames, one cross-origin and one `srcdoc`, produces exactly one `iframe` entry. Counting entries there will undercount the frames on the page, and filtering for `page` entries will miss all of them. Motus builds `page.Frames` from auto-attach and the frame tree instead, which covers both kinds, so read frames from there and treat the target list as a view of processes rather than of documents. -**A frame changing process is not a frame going away, but a frame whose parent navigated is.** The browser announces a frame moved into another renderer process as a detach carrying the reason `swap`, and says the same for a document it puts into the back-forward cache. Motus tells both apart from a real removal: the frame stays in `page.Frames`, `IsDetached` stays false, no `FrameDetached` arrives, and the `IFrame` being held keeps working across the move. Navigation is the other case. Committing a new document in a frame drops the frames that frame hosted, because the new document reports its own, so a child frame is rediscovered after its parent navigates and comes back as a different `IFrame`, even when it loads the same URL. What that leaves to watch for is the handle from before. It reports `IsDetached`, and checking that is worth the line, because a frame the page no longer lists has no execution context of its own to aim at and an evaluation through it falls back to the page's, quietly answering for the main document rather than failing. Read the frame from `page.Frames` again after a navigation and wait for it as under [Traversing](#traversing), since for a moment after the parent commits its new document the subtree is empty. +**A frame changing process is not a frame going away, but a frame whose parent navigated is.** The browser announces a frame moved into another renderer process as a detach carrying the reason `swap`, and says the same for a document it puts into the back-forward cache. Motus tells both apart from a real removal: the frame stays in `page.Frames`, `IsDetached` stays false, no `FrameDetached` arrives, and the `IFrame` being held keeps working across the move. Navigation is the other case. Committing a new document in a frame drops the frames that frame hosted, because the new document reports its own, so a child frame is rediscovered after its parent navigates and comes back as a different `IFrame`, even when it loads the same URL. What that leaves to watch for is the handle from before. It reports `IsDetached`, and anything run through it throws `MotusTargetClosedException` rather than answering for some other document. Read the frame from `page.Frames` again after a navigation and wait for it as under [Traversing](#traversing), since for a moment after the parent commits its new document the subtree is empty. **Application globals are absent from isolated worlds.** An isolated world shares the frame's document and nothing else, so the DOM is fully present while everything the application put on `window` is not. An expression that reads a framework handle returns `undefined` there and the failure looks like the application not having loaded. Use `ExecutionWorld.Main`, which is the default, whenever you are reading application state. diff --git a/src/Motus/Page/Page.Evaluation.cs b/src/Motus/Page/Page.Evaluation.cs index eb9a07c..39c7a20 100644 --- a/src/Motus/Page/Page.Evaluation.cs +++ b/src/Motus/Page/Page.Evaluation.cs @@ -15,20 +15,27 @@ public async Task EvaluateAsync(string expression, object? arg = null) public async Task EvaluateHandleAsync(string expression, object? arg = null) { - var contextId = _mainFrameId is not null - ? GetExecutionContextId(_mainFrameId) - : null; - - var result = await _session.SendAsync( - "Runtime.evaluate", - new RuntimeEvaluateParams( - Expression: WrapExpression(expression, arg), - ReturnByValue: false, - AwaitPromise: true, - ContextId: contextId), - CdpJsonContext.Default.RuntimeEvaluateParams, - CdpJsonContext.Default.RuntimeEvaluateResult, - CancellationToken.None).ConfigureAwait(false); + var wrapped = WrapExpression(expression, arg); + + async Task<(IMotusSession Session, RuntimeEvaluateResult Result)> Evaluate(ExecutionTarget target) + { + var evaluated = await target.Session.SendAsync( + "Runtime.evaluate", + new RuntimeEvaluateParams( + Expression: wrapped, + ReturnByValue: false, + AwaitPromise: true, + ContextId: target.ContextId), + CdpJsonContext.Default.RuntimeEvaluateParams, + CdpJsonContext.Default.RuntimeEvaluateResult, + CancellationToken.None).ConfigureAwait(false); + + return (target.Session, evaluated); + } + + var (session, result) = _mainFrameId is not null + ? await RunInFrameAsync(_mainFrameId, Evaluate).ConfigureAwait(false) + : await Evaluate(new ExecutionTarget(_session, null)).ConfigureAwait(false); if (result.ExceptionDetails is not null) throw new InvalidOperationException( @@ -38,7 +45,7 @@ public async Task EvaluateHandleAsync(string expression, object? arg throw new InvalidOperationException( "Expression did not return an object. Use EvaluateAsync for primitive values."); - return new JsHandle(_session, result.Result.ObjectId); + return new JsHandle(session, result.Result.ObjectId); } public async Task WaitForFunctionAsync(string expression, object? arg = null, double? timeout = null) diff --git a/src/Motus/Page/Page.Events.cs b/src/Motus/Page/Page.Events.cs index 8cd4f0b..1d39a85 100644 --- a/src/Motus/Page/Page.Events.cs +++ b/src/Motus/Page/Page.Events.cs @@ -97,10 +97,11 @@ internal void StartFrameStructureEventPump(IMotusSession session, CancellationTo CdpJsonContext.Default.PageFrameStoppedLoadingEvent, evt => FrameStoppedLoading?.Invoke(evt.FrameId), ct); - _ = PumpEventsAsync(session, - "Runtime.executionContextCreated", - CdpJsonContext.Default.RuntimeExecutionContextCreatedEvent, - OnExecutionContextCreated, ct); + // Contexts are made and torn down by the same navigations, so their events are read + // together for the same reason as the frame tree's. BiDi has no counterpart to listen for + // (see PumpExecutionContextEventsAsync). + if (session is not BiDiSession) + _ = PumpExecutionContextEventsAsync(session, ct); _ = PumpEventsAsync(session, "Runtime.consoleAPICalled", @@ -291,6 +292,10 @@ private void DetachFrame(string frameId) return; frame.MarkDetached(); + + // Anyone waiting for this frame's next context would otherwise wait out the clock. + SignalExecutionContextWaiters(frameId); + FrameDetached?.Invoke(this, frame); } @@ -310,34 +315,6 @@ private void RecordFrameOwnership(string frameId, IMotusSession source, bool isP _frameIdToSession[frameId] = source; } - private void OnExecutionContextCreated(RuntimeExecutionContextCreatedEvent evt) - { - var ctx = evt.Context; - - // Extract frameId from auxData if present - string? frameId = null; - var isDefaultWorld = true; - if (ctx.AuxData is JsonElement aux && aux.ValueKind == JsonValueKind.Object) - { - if (aux.TryGetProperty("frameId", out var fid)) - frameId = fid.GetString(); - - // A frame has one main world and any number of others beside it. Only the main world - // is the frame's context: an isolated world reports the same frame id, and recording - // it here would send every later main-world evaluation into the isolated world, where - // the page's own globals do not exist. That failure is silent, so it is guarded at the - // one place contexts are recorded rather than at each reader. - if (aux.TryGetProperty("isDefault", out var isDefault) - && isDefault.ValueKind is JsonValueKind.False) - { - isDefaultWorld = false; - } - } - - if (frameId is not null && isDefaultWorld) - _frameIdToExecutionContext[frameId] = ctx.Id; - } - private void OnConsoleApiCalled(RuntimeConsoleApiCalledEvent evt) { var text = string.Join(" ", evt.Args.Select(a => diff --git a/src/Motus/Page/Page.ExecutionContexts.cs b/src/Motus/Page/Page.ExecutionContexts.cs new file mode 100644 index 0000000..ee69c04 --- /dev/null +++ b/src/Motus/Page/Page.ExecutionContexts.cs @@ -0,0 +1,375 @@ +using System.Collections.Concurrent; +using System.Text.Json; +using Motus.Abstractions; + +namespace Motus; + +/// +/// The execution context each frame's main world currently runs in, and what a caller does when a +/// frame has none yet. +/// +internal sealed partial class Page +{ + /// + /// How long a command waits for a frame to report its next main world when the caller gives no + /// shorter bound of its own. The same default every other wait in Motus uses. + /// + private static readonly TimeSpan ExecutionContextWaitTimeout = TimeSpan.FromMilliseconds(30_000); + + /// + /// The text the browser answers with when a command names an execution context that no longer + /// exists. + /// + private const string StaleContextMessage = "Cannot find context with specified id"; + + /// + /// A frame's main world: its context id, and the session that reported it. + /// + /// + /// A context id is numbered by the renderer that owns it, so two sessions can report the same + /// number for two different worlds, and an id means nothing on any session but its own. Keeping + /// the session alongside the id is what lets a late notice from one session leave alone a + /// context another session has since reported for the same frame. + /// + private readonly record struct FrameExecutionContext(int Id, IMotusSession Session); + + /// + /// Where a command about a frame is sent: the session, and the context to name, or null for the + /// session's default, which is the main world of the frame at that session's root. + /// + internal readonly record struct ExecutionTarget(IMotusSession Session, int? ContextId); + + private readonly ConcurrentDictionary _frameIdToExecutionContext = new(); + + // Callers waiting for a frame to report its next main world. One completion per frame, shared by + // every caller waiting on that frame and replaced once it fires, so it says only that something + // changed and each caller looks again. It fires when a context is recorded for the frame and when + // the frame goes away, so nobody waits out the clock on a frame that will never answer. + private readonly ConcurrentDictionary _executionContextWaiters = new(); + + /// + /// Reads the events that make and unmake execution contexts on one session, in the order the + /// browser sent them. + /// + /// + /// A navigation tears down the old document's contexts and announces the new document's in that + /// order. Read on separate loops, the new document's context could be recorded first and the + /// teardown handled after it, which would wipe out the one context the frame actually has. One + /// channel for the three keeps the teardown ahead of the replacement. + /// + /// WebDriver BiDi has no counterpart to subscribe to here. Script there is addressed to a + /// browsing context and runs in whatever realm that context holds at the time, so there is no id + /// to go stale and nothing to track. + /// + /// + private async Task PumpExecutionContextEventsAsync(IMotusSession session, CancellationToken ct) + { + string[] events = + [ + "Runtime.executionContextCreated", + "Runtime.executionContextDestroyed", + "Runtime.executionContextsCleared", + ]; + + try + { + await foreach (var raw in session.SubscribeAsync(events, ct).ConfigureAwait(false)) + { + try + { + switch (raw.Method) + { + case "Runtime.executionContextCreated": + if (JsonSerializer.Deserialize(raw.Params, CdpJsonContext.Default.RuntimeExecutionContextCreatedEvent) is { } created) + OnExecutionContextCreated(created, session); + break; + case "Runtime.executionContextDestroyed": + if (JsonSerializer.Deserialize(raw.Params, CdpJsonContext.Default.RuntimeExecutionContextDestroyedEvent) is { } destroyed) + OnExecutionContextDestroyed(destroyed.ExecutionContextId, session); + break; + case "Runtime.executionContextsCleared": + OnExecutionContextsCleared(session); + break; + } + } + catch + { + // Prevent a malformed event from killing the event pump + } + } + } + catch (OperationCanceledException) + { + // Expected on page close + } + } + + private void OnExecutionContextCreated(RuntimeExecutionContextCreatedEvent evt, IMotusSession source) + { + var ctx = evt.Context; + + // Extract frameId from auxData if present + string? frameId = null; + var isDefaultWorld = true; + if (ctx.AuxData is JsonElement aux && aux.ValueKind == JsonValueKind.Object) + { + if (aux.TryGetProperty("frameId", out var fid)) + frameId = fid.GetString(); + + // A frame has one main world and any number of others beside it. Only the main world + // is the frame's context: an isolated world reports the same frame id, and recording + // it here would send every later main-world evaluation into the isolated world, where + // the page's own globals do not exist. That failure is silent, so it is guarded at the + // one place contexts are recorded rather than at each reader. + if (aux.TryGetProperty("isDefault", out var isDefault) + && isDefault.ValueKind is JsonValueKind.False) + { + isDefaultWorld = false; + } + } + + if (frameId is null || !isDefaultWorld) + return; + + _frameIdToExecutionContext[frameId] = new FrameExecutionContext(ctx.Id, source); + SignalExecutionContextWaiters(frameId); + } + + /// + /// Forgets a context the browser has torn down, wherever it was recorded. + /// + /// + /// Only an entry with the same id from the same session is dropped. A frame that has moved to + /// another process already has its new context recorded from the new session by the time the + /// old session reports the old one gone, and that report is not about the new one. + /// + private void OnExecutionContextDestroyed(int contextId, IMotusSession source) + { + foreach (var (frameId, context) in _frameIdToExecutionContext) + { + if (context.Id == contextId && ReferenceEquals(context.Session, source)) + _frameIdToExecutionContext.TryRemove(new KeyValuePair(frameId, context)); + } + + foreach (var frameId in _frameIdToIsolatedWorld.Keys) + { + _frames.TryGetValue(frameId, out var frame); + if (ReferenceEquals(SessionFor(frame), source)) + DropIsolatedWorld(frameId, contextId); + } + } + + /// + /// Forgets every context one session reported, which is what the browser announces when the + /// document at that session's root is replaced. + /// + private void OnExecutionContextsCleared(IMotusSession source) + { + foreach (var (frameId, context) in _frameIdToExecutionContext) + { + if (ReferenceEquals(context.Session, source)) + _frameIdToExecutionContext.TryRemove(new KeyValuePair(frameId, context)); + } + + foreach (var (frameId, world) in _frameIdToIsolatedWorld) + { + _frames.TryGetValue(frameId, out var frame); + if (ReferenceEquals(SessionFor(frame), source)) + _frameIdToIsolatedWorld.TryRemove(new KeyValuePair>>(frameId, world)); + } + } + + /// + /// Drops the frame's isolated world when it is the one with the given id, so the next ask makes + /// a fresh one rather than handing back a world the browser has torn down. + /// + private void DropIsolatedWorld(string frameId, int contextId) + { + if (_frameIdToIsolatedWorld.TryGetValue(frameId, out var world) + && world.IsValueCreated + && world.Value.IsCompletedSuccessfully + && world.Value.Result == contextId) + { + _frameIdToIsolatedWorld.TryRemove(new KeyValuePair>>(frameId, world)); + } + } + + /// + /// Wakes every caller waiting on the frame, so each looks again at what it now has. + /// + private void SignalExecutionContextWaiters(string frameId) + { + if (_executionContextWaiters.TryRemove(frameId, out var waiters)) + waiters.TrySetResult(); + } + + /// + /// Runs a command against a frame's main world, waiting for the frame's next one when it has + /// none, and sending once more when the context it named turns out to be gone. + /// + /// The frame to run in. + /// Sends the command to the session and context it is handed. + /// Bounds the wait for a context, alongside the page's own lifetime. + /// + /// A navigation's load event and its new context arrive on different loops, so a caller that + /// starts work the moment a navigation completes can find the old document's context still + /// recorded, or no context at all. The second case waits here for the new one. The first is + /// caught when the browser rejects the old id, and is retried once against whatever has been + /// recorded since. + /// + internal Task RunInFrameAsync( + string frameId, Func> command, CancellationToken ct = default) => + RunInMainWorldAsync(frameId, mainFrameUsesDefault: false, command, ct); + + /// + /// Runs a selector query against a frame's main world. The page's main frame, and anything + /// that is not a frame of this page, uses the session's default context rather than a recorded + /// id, which is what page-level locators have always sent and which cannot go stale. + /// + /// The frame to run in, or null for the page's main frame. + /// Sends the command to the session and context it is handed. + /// Bounds the wait for a context, alongside the page's own lifetime. + internal Task RunInSelectorContextAsync( + IFrame? frame, Func> command, CancellationToken ct = default) + { + if (frame is not Frame f || f.Id == SyntheticSelectorFrameId) + return command(new ExecutionTarget(SessionFor(frame), null)); + + return RunInMainWorldAsync(f.Id, mainFrameUsesDefault: true, command, ct); + } + + private async Task RunInMainWorldAsync( + string frameId, bool mainFrameUsesDefault, Func> command, CancellationToken ct) + { + var target = await ResolveMainWorldAsync(frameId, mainFrameUsesDefault, ct).ConfigureAwait(false); + + try + { + return await command(target).ConfigureAwait(false); + } + catch (MotusProtocolException ex) when (target.ContextId is { } stale && IsStaleContextError(ex)) + { + // The browser looks a context up before it compiles or runs anything, so a command it + // turns away for naming a dead context has not run at all. Sending it once more cannot + // run the caller's script twice, which is what makes this retry safe for commands that + // change the page as well as for ones that only read it. + ForgetExecutionContext(frameId, new FrameExecutionContext(stale, target.Session)); + target = await ResolveMainWorldAsync(frameId, mainFrameUsesDefault, ct).ConfigureAwait(false); + return await command(target).ConfigureAwait(false); + } + } + + /// + /// Runs a command against the frame's isolated world, making a fresh world and sending once + /// more when the one it named has gone with the document it was made for. + /// + internal async Task RunInIsolatedWorldAsync(string frameId, Func> command) + { + await WhenFrameReadyAsync(frameId).ConfigureAwait(false); + _frames.TryGetValue(frameId, out var frame); + + var target = new ExecutionTarget( + SessionFor(frame), await GetIsolatedWorldContextIdAsync(frameId).ConfigureAwait(false)); + + try + { + return await command(target).ConfigureAwait(false); + } + catch (MotusProtocolException ex) when (target.ContextId is { } stale && IsStaleContextError(ex)) + { + // Safe to send again for the reason given in RunInMainWorldAsync: a dead context is + // refused before anything runs. + DropIsolatedWorld(frameId, stale); + target = new ExecutionTarget( + SessionFor(frame), await GetIsolatedWorldContextIdAsync(frameId).ConfigureAwait(false)); + return await command(target).ConfigureAwait(false); + } + } + + /// + /// Decides where a command about a frame's main world goes, waiting for the frame to report one + /// when it has none. + /// + /// + /// The main frame never waits. With nothing recorded, the page session's default context is the + /// main frame's current main world, so leaving the id out reaches the right document and cannot + /// name a dead one. Any other frame has no such fallback: leaving the id out would run in the + /// main frame instead, quietly and with the wrong answer, so it waits for its own context. + /// A frame that has gone away fails at once rather than waiting for a context that will never + /// come. + /// + private async Task ResolveMainWorldAsync( + string frameId, bool mainFrameUsesDefault, CancellationToken ct) + { + await WhenFrameReadyAsync(frameId).ConfigureAwait(false); + _frames.TryGetValue(frameId, out var frame); + + // BiDi addresses script to a browsing context and has no context ids to wait for. + if (_session is BiDiSession) + return new ExecutionTarget(SessionFor(frame), null); + + var isMainFrame = string.Equals(frameId, _mainFrameId, StringComparison.Ordinal); + if (isMainFrame && mainFrameUsesDefault) + return new ExecutionTarget(_session, null); + + if (_frameIdToExecutionContext.TryGetValue(frameId, out var known)) + return new ExecutionTarget(known.Session, known.Id); + + if (isMainFrame) + return new ExecutionTarget(_session, null); + + using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct, _pageCts.Token); + cts.CancelAfter(ExecutionContextWaitTimeout); + + while (true) + { + // Registered before looking, so a context recorded or a detach made between the look + // and the wait still wakes this caller. + var changed = _executionContextWaiters.GetOrAdd( + frameId, _ => new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously)); + + if (_frameIdToExecutionContext.TryGetValue(frameId, out var context)) + return new ExecutionTarget(context.Session, context.Id); + + ThrowIfUnreachable(frameId, frame); + + try + { + await changed.Task.WaitAsync(cts.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) when (!ct.IsCancellationRequested) + { + if (_pageCts.IsCancellationRequested) + throw PageClosedWhileWaiting(frameId); + + throw new WaitTimeoutException( + condition: "frame execution context", + timeoutDuration: ExecutionContextWaitTimeout, + lastEvaluatedValue: null, + message: $"Frame '{frameId}' did not report an execution context within " + + $"{ExecutionContextWaitTimeout.TotalMilliseconds}ms."); + } + } + } + + private void ThrowIfUnreachable(string frameId, Frame? frame) + { + if (_isClosed) + throw PageClosedWhileWaiting(frameId); + + if (frame is null || frame.IsDetached || !_frames.ContainsKey(frameId)) + throw new MotusTargetClosedException("frame", frameId, $"Frame '{frameId}' was detached."); + } + + private MotusTargetClosedException PageClosedWhileWaiting(string frameId) => + new("page", _targetId, $"The page was closed while waiting for frame '{frameId}' to report an execution context."); + + /// + /// Drops a context the browser has said is gone, unless something newer has replaced it. + /// + private void ForgetExecutionContext(string frameId, FrameExecutionContext stale) => + _frameIdToExecutionContext.TryRemove(new KeyValuePair(frameId, stale)); + + private static bool IsStaleContextError(MotusProtocolException ex) => + ex.Message.Contains(StaleContextMessage, StringComparison.Ordinal); +} diff --git a/src/Motus/Page/Page.Viewport.cs b/src/Motus/Page/Page.Viewport.cs index 1df21b5..c0132f0 100644 --- a/src/Motus/Page/Page.Viewport.cs +++ b/src/Motus/Page/Page.Viewport.cs @@ -66,8 +66,6 @@ public Task AddScriptTagAsync(string? url = null, string? conten internal async Task AddScriptTagAsync(string? url, string? content, IFrame? frame) { await WhenFrameReadyAsync(frame).ConfigureAwait(false); - var session = SessionFor(frame); - var contextId = frame is null ? null : GetSelectorContextId(frame); string js; if (url is not null) @@ -81,13 +79,18 @@ internal async Task AddScriptTagAsync(string? url, string? conte js = "(() => { const s = document.createElement('script'); s.textContent = " + serializedContent + "; document.head.appendChild(s); return s; })()"; } - var result = await session.SendAsync( - "Runtime.evaluate", - new RuntimeEvaluateParams(Expression: js, ReturnByValue: false, AwaitPromise: false, - ContextId: contextId), - CdpJsonContext.Default.RuntimeEvaluateParams, - CdpJsonContext.Default.RuntimeEvaluateResult, - _pageCts.Token).ConfigureAwait(false); + var (session, result) = await RunInSelectorContextAsync(frame, async target => + { + var evaluated = await target.Session.SendAsync( + "Runtime.evaluate", + new RuntimeEvaluateParams(Expression: js, ReturnByValue: false, AwaitPromise: false, + ContextId: target.ContextId), + CdpJsonContext.Default.RuntimeEvaluateParams, + CdpJsonContext.Default.RuntimeEvaluateResult, + _pageCts.Token).ConfigureAwait(false); + + return (target.Session, evaluated); + }, _pageCts.Token).ConfigureAwait(false); if (result.Result.ObjectId is null) throw new InvalidOperationException("Failed to create script element."); @@ -105,8 +108,6 @@ public Task AddStyleTagAsync(string? url = null, string? content internal async Task AddStyleTagAsync(string? url, string? content, IFrame? frame) { await WhenFrameReadyAsync(frame).ConfigureAwait(false); - var session = SessionFor(frame); - var contextId = frame is null ? null : GetSelectorContextId(frame); string js; if (url is not null) @@ -120,13 +121,18 @@ internal async Task AddStyleTagAsync(string? url, string? conten js = "(() => { const s = document.createElement('style'); s.textContent = " + serializedContent + "; document.head.appendChild(s); return s; })()"; } - var result = await session.SendAsync( - "Runtime.evaluate", - new RuntimeEvaluateParams(Expression: js, ReturnByValue: false, AwaitPromise: false, - ContextId: contextId), - CdpJsonContext.Default.RuntimeEvaluateParams, - CdpJsonContext.Default.RuntimeEvaluateResult, - _pageCts.Token).ConfigureAwait(false); + var (session, result) = await RunInSelectorContextAsync(frame, async target => + { + var evaluated = await target.Session.SendAsync( + "Runtime.evaluate", + new RuntimeEvaluateParams(Expression: js, ReturnByValue: false, AwaitPromise: false, + ContextId: target.ContextId), + CdpJsonContext.Default.RuntimeEvaluateParams, + CdpJsonContext.Default.RuntimeEvaluateResult, + _pageCts.Token).ConfigureAwait(false); + + return (target.Session, evaluated); + }, _pageCts.Token).ConfigureAwait(false); if (result.Result.ObjectId is null) throw new InvalidOperationException("Failed to create style element."); diff --git a/src/Motus/Page/Page.cs b/src/Motus/Page/Page.cs index f4181ce..7f615a5 100644 --- a/src/Motus/Page/Page.cs +++ b/src/Motus/Page/Page.cs @@ -25,8 +25,6 @@ internal sealed partial class Page : IPage // own for every frame the browser puts in its own process. private readonly List _frameOrder = []; - private readonly ConcurrentDictionary _frameIdToExecutionContext = new(); - // Frames that render in their own process are reached over a session of their own. A frame // absent from this map is driven over the page session, which is every frame the page's own // renderer hosts. @@ -259,27 +257,6 @@ internal IFrame GetFrameForSelectors() => ? frame : new Frame(this, SyntheticSelectorFrameId, parentFrameId: null); - /// - /// Returns the execution context to resolve selectors in for the given frame, or null to use - /// the session default. - /// - /// - /// The main frame maps to the default context, which is what every page-level locator has - /// always used, so page-level resolution keeps sending exactly what it sent before. Passing an - /// explicit id for the main frame would also be fragile: a context id can go stale across a - /// navigation, where null always resolves to whatever the current default is. - /// - internal int? GetSelectorContextId(IFrame frame) - { - if (frame is not Frame f) - return null; - - if (f.Id == _mainFrameId || f.Id == SyntheticSelectorFrameId) - return null; - - return GetExecutionContextId(f.Id); - } - /// /// Returns the session that owns the given frame, or the page session when the frame is null /// or is hosted by the page's own renderer. @@ -359,30 +336,27 @@ private List OrderedFrames() } } - internal int? GetExecutionContextId(string frameId) => - _frameIdToExecutionContext.TryGetValue(frameId, out var id) ? id : null; - internal async Task EvaluateInFrameAsync( - string frameId, string expression, object? arg = null, ExecutionWorld world = ExecutionWorld.Main) + string frameId, string expression, object? arg = null, ExecutionWorld world = ExecutionWorld.Main, + CancellationToken ct = default) { - await WhenFrameReadyAsync(frameId).ConfigureAwait(false); - - _frames.TryGetValue(frameId, out var frame); - - var contextId = world == ExecutionWorld.Isolated - ? await GetIsolatedWorldContextIdAsync(frameId).ConfigureAwait(false) - : GetExecutionContextId(frameId); - - var result = await SessionFor(frame).SendAsync( - "Runtime.evaluate", - new RuntimeEvaluateParams( - Expression: WrapExpression(expression, arg), - ReturnByValue: true, - AwaitPromise: true, - ContextId: contextId), - CdpJsonContext.Default.RuntimeEvaluateParams, - CdpJsonContext.Default.RuntimeEvaluateResult, - CancellationToken.None).ConfigureAwait(false); + var wrapped = WrapExpression(expression, arg); + + Task Evaluate(ExecutionTarget target) => + target.Session.SendAsync( + "Runtime.evaluate", + new RuntimeEvaluateParams( + Expression: wrapped, + ReturnByValue: true, + AwaitPromise: true, + ContextId: target.ContextId), + CdpJsonContext.Default.RuntimeEvaluateParams, + CdpJsonContext.Default.RuntimeEvaluateResult, + CancellationToken.None); + + var result = world == ExecutionWorld.Isolated + ? await RunInIsolatedWorldAsync(frameId, Evaluate).ConfigureAwait(false) + : await RunInFrameAsync(frameId, Evaluate, ct).ConfigureAwait(false); if (result.ExceptionDetails is not null) throw new InvalidOperationException( @@ -402,7 +376,8 @@ internal async Task WaitForFunctionInFrameAsync( { try { - var result = await EvaluateInFrameAsync(frameId, expression, arg).ConfigureAwait(false); + var result = await EvaluateInFrameAsync( + frameId, expression, arg, ct: cts.Token).ConfigureAwait(false); if (IsTruthy(result)) return result.Deserialize()!; @@ -411,6 +386,11 @@ internal async Task WaitForFunctionInFrameAsync( { // Evaluation failed, will retry } + catch (OperationCanceledException) when (cts.Token.IsCancellationRequested) + { + // Ran out of time waiting for the frame's context + break; + } try { diff --git a/src/Motus/Protocol/CdpJsonContext.cs b/src/Motus/Protocol/CdpJsonContext.cs index 0847dd4..d75bfd0 100644 --- a/src/Motus/Protocol/CdpJsonContext.cs +++ b/src/Motus/Protocol/CdpJsonContext.cs @@ -78,6 +78,7 @@ namespace Motus; [JsonSerializable(typeof(RuntimeExceptionThrownEvent))] [JsonSerializable(typeof(RuntimeBindingCalledEvent))] [JsonSerializable(typeof(RuntimeExecutionContextCreatedEvent))] +[JsonSerializable(typeof(RuntimeExecutionContextDestroyedEvent))] // --- Emulation domain --- [JsonSerializable(typeof(EmulationSetDeviceMetricsOverrideParams))] [JsonSerializable(typeof(EmulationSetDeviceMetricsOverrideResult))] @@ -555,6 +556,9 @@ internal sealed record RuntimeExecutionContextDescription( internal sealed record RuntimeExecutionContextCreatedEvent( RuntimeExecutionContextDescription Context); +internal sealed record RuntimeExecutionContextDestroyedEvent( + int ExecutionContextId); + internal sealed record RuntimeConsoleApiCalledEvent( string Type, RuntimeRemoteObject[] Args, diff --git a/src/Motus/Selectors/SelectorStrategyHelpers.cs b/src/Motus/Selectors/SelectorStrategyHelpers.cs index eabd388..69b0399 100644 --- a/src/Motus/Selectors/SelectorStrategyHelpers.cs +++ b/src/Motus/Selectors/SelectorStrategyHelpers.cs @@ -20,18 +20,22 @@ internal static async Task> EvalToHandlesAsync( IFrame frame, string js, CancellationToken ct) { var page = GetPage(frame); - var session = page.SessionFor(frame); - - var result = await session.SendAsync( - "Runtime.evaluate", - new RuntimeEvaluateParams( - Expression: js, - ReturnByValue: false, - AwaitPromise: false, - ContextId: page.GetSelectorContextId(frame)), - CdpJsonContext.Default.RuntimeEvaluateParams, - CdpJsonContext.Default.RuntimeEvaluateResult, - ct).ConfigureAwait(false); + + var (session, result) = await page.RunInSelectorContextAsync(frame, async target => + { + var evaluated = await target.Session.SendAsync( + "Runtime.evaluate", + new RuntimeEvaluateParams( + Expression: js, + ReturnByValue: false, + AwaitPromise: false, + ContextId: target.ContextId), + CdpJsonContext.Default.RuntimeEvaluateParams, + CdpJsonContext.Default.RuntimeEvaluateResult, + ct).ConfigureAwait(false); + + return (target.Session, evaluated); + }, ct).ConfigureAwait(false); if (result.ExceptionDetails is not null) throw new InvalidOperationException( @@ -101,16 +105,17 @@ internal static async Task ResolveNodeToHandleAsync( { var page = GetPage(frame); - var result = await page.SessionFor(frame).SendAsync( - "Runtime.evaluate", - new RuntimeEvaluateParams( - Expression: "document.documentElement", - ReturnByValue: false, - AwaitPromise: false, - ContextId: page.GetSelectorContextId(frame)), - CdpJsonContext.Default.RuntimeEvaluateParams, - CdpJsonContext.Default.RuntimeEvaluateResult, - ct).ConfigureAwait(false); + var result = await page.RunInSelectorContextAsync(frame, target => + target.Session.SendAsync( + "Runtime.evaluate", + new RuntimeEvaluateParams( + Expression: "document.documentElement", + ReturnByValue: false, + AwaitPromise: false, + ContextId: target.ContextId), + CdpJsonContext.Default.RuntimeEvaluateParams, + CdpJsonContext.Default.RuntimeEvaluateResult, + ct), ct).ConfigureAwait(false); return result.ExceptionDetails is null ? result.Result.ObjectId : null; } diff --git a/tests/Motus.Tests/Browser/FirefoxIntegrationTests.cs b/tests/Motus.Tests/Browser/FirefoxIntegrationTests.cs index 7f4e4d8..e4e81be 100644 --- a/tests/Motus.Tests/Browser/FirefoxIntegrationTests.cs +++ b/tests/Motus.Tests/Browser/FirefoxIntegrationTests.cs @@ -137,4 +137,36 @@ public async Task Firefox_NewPage_HasMainFrameAndTakesContentWithoutNavigation() Assert.AreEqual("T", await page.TitleAsync()); Assert.AreEqual(2, await page.MainFrame.EvaluateAsync("1 + 1")); } + + /// + /// Evaluating straight after a navigation works on Firefox too. BiDi addresses script to a + /// browsing context rather than to an execution context id, so there is no id to go stale, and + /// the waiting Chromium needs must not get in its way. + /// + [TestMethod] + public async Task Firefox_EvaluateRightAfterNavigation_Succeeds() + { + if (!FirefoxAvailable()) + { + Assert.Inconclusive("Firefox not found on this machine."); + return; + } + + await using var browser = await MotusLauncher.LaunchAsync( + new LaunchOptions { Headless = true, Channel = BrowserChannel.Firefox }); + + for (var i = 0; i < 30; i++) + { + var context = await browser.NewContextAsync(); + var page = await context.NewPageAsync(); + + await page.GotoAsync("about:blank"); + Assert.AreEqual(2, await page.EvaluateAsync("1+1"), $"iteration {i}"); + + await page.GotoAsync($"data:text/html,t{i}"); + Assert.AreEqual($"t{i}", await page.TitleAsync(), $"iteration {i}"); + + await context.CloseAsync(); + } + } } diff --git a/tests/Motus.Tests/Page/ExecutionContextIntegrationTests.cs b/tests/Motus.Tests/Page/ExecutionContextIntegrationTests.cs new file mode 100644 index 0000000..bafcb00 --- /dev/null +++ b/tests/Motus.Tests/Page/ExecutionContextIntegrationTests.cs @@ -0,0 +1,160 @@ +using Motus.Abstractions; + +namespace Motus.Tests.Page; + +/// +/// Evaluating the moment a navigation completes, against a real browser. +/// +/// +/// A navigation's load event and the new document's execution context arrive separately, so a +/// caller that evaluates straight after GotoAsync could reach the page before the new +/// context had been recorded and be refused with "Cannot find context with specified id". It only +/// happened some of the time, so these tests repeat the sequence rather than relying on one run. +/// +[TestClass] +[TestCategory("Integration")] +public class ExecutionContextIntegrationTests +{ + private const int Iterations = 40; + + private CrossOriginFixtureServer _server = null!; + private IBrowser? _browser; + + [TestInitialize] + public async Task Setup() + { + _server = new CrossOriginFixtureServer(); + + try + { + _browser = await MotusLauncher.LaunchAsync(new LaunchOptions + { + Headless = true, + Args = _server.IsolationArgs + }); + } + catch (FileNotFoundException) + { + Assert.Inconclusive("No browser found; skipping integration tests."); + } + } + + [TestCleanup] + public async Task Cleanup() + { + if (_browser is not null) + await _browser.DisposeAsync(); + + _server?.Dispose(); + } + + /// + /// The sequence the problem was reported with: a fresh context and page, one navigation, and + /// an evaluation straight after it. + /// + [TestMethod] + public async Task EvaluateRightAfterNavigatingAFreshPage_Succeeds() + { + for (var i = 0; i < Iterations; i++) + { + var context = await _browser!.NewContextAsync(); + var page = await context.NewPageAsync(); + + await page.GotoAsync("about:blank"); + Assert.AreEqual(2, await page.EvaluateAsync("1+1"), $"iteration {i}"); + + await page.GotoAsync($"data:text/html,t{i}

{i}

"); + Assert.AreEqual($"t{i}", await page.TitleAsync(), $"iteration {i}"); + StringAssert.Contains(await page.ContentAsync(), $"

{i}

"); + + await context.CloseAsync(); + } + } + + /// + /// A navigation to another site moves the page into a new renderer process, which replaces the + /// main world with one numbered by a different process. + /// + [TestMethod] + public async Task EvaluateRightAfterACrossOriginNavigation_RunsInTheNewDocument() + { + var page = await _browser!.NewPageAsync(); + + for (var i = 0; i < Iterations / 2; i++) + { + await page.GotoAsync(_server.PrimaryOrigin + "/deep.html"); + Assert.AreEqual("deep", await page.EvaluateAsync("window.marker"), $"iteration {i}"); + + await page.GotoAsync(_server.SecondaryOrigin + "/middle.html"); + Assert.AreEqual("middle", await page.EvaluateAsync("window.marker"), $"iteration {i}"); + Assert.AreEqual(_server.SecondaryOrigin, await page.EvaluateAsync("location.origin")); + } + } + + /// + /// A frame that navigates on its own replaces only its own main world. An evaluation in it must + /// reach the new document, not the old one and not the page's main frame. + /// + [TestMethod] + public async Task EvaluateInAFrameRightAfterItNavigates_RunsInThatFramesNewDocument() + { + var page = await _browser!.NewPageAsync(); + await page.GotoAsync(_server.OrderedUrl); + var frame = await FirstChildFrameAsync(page); + + for (var i = 0; i < Iterations / 4; i++) + { + await frame.GotoAsync(_server.PrimaryOrigin + "/deep.html"); + Assert.AreEqual("deep", await frame.EvaluateAsync("window.marker"), $"iteration {i}"); + Assert.AreEqual(0, await frame.EvaluateAsync("window.clicks")); + + await frame.GotoAsync(_server.PrimaryOrigin + "/ordered-child.html?again=" + i); + Assert.AreEqual( + "child", + await frame.EvaluateAsync("document.querySelector('p').textContent"), + $"iteration {i}"); + } + + Assert.IsNull( + await page.EvaluateAsync("window.marker ?? null"), + "the frame's evaluations never landed in the main frame"); + } + + /// + /// Same as above for a locator, which resolves through the frame's context rather than through + /// an evaluation of the caller's own. + /// + [TestMethod] + public async Task LocatorInAFrameRightAfterItNavigates_ResolvesInThatFramesNewDocument() + { + var page = await _browser!.NewPageAsync(); + await page.GotoAsync(_server.OrderedUrl); + var frame = await FirstChildFrameAsync(page); + + for (var i = 0; i < Iterations / 4; i++) + { + await frame.GotoAsync(_server.PrimaryOrigin + "/deep.html"); + Assert.AreEqual("deep", await frame.Locator("#target").TextContentAsync(), $"iteration {i}"); + + await frame.GotoAsync(_server.PrimaryOrigin + "/ordered-child.html?again=" + i); + Assert.AreEqual("child", await frame.Locator("p").TextContentAsync(), $"iteration {i}"); + } + } + + private static async Task" + + "
" + + $"" + + $""); + + var stopwatch = Stopwatch.StartNew(); + var text = TextOf(await SnapshotAsync()); + stopwatch.Stop(); + + StringAssert.Contains(text, "- button \"Pay now\""); + Assert.IsTrue( + stopwatch.Elapsed < TimeSpan.FromSeconds(10), + $"the snapshot took {stopwatch.ElapsedMilliseconds} ms"); + } + [TestMethod] public async Task WithNoRoomForFrames_ThePageSaysWhatItLeftOut() { diff --git a/tests/Motus.Mcp.Tests/Snapshot/InlineFrameSnapshotTests.cs b/tests/Motus.Mcp.Tests/Snapshot/InlineFrameSnapshotTests.cs index 2d6e63b..d36b08c 100644 --- a/tests/Motus.Mcp.Tests/Snapshot/InlineFrameSnapshotTests.cs +++ b/tests/Motus.Mcp.Tests/Snapshot/InlineFrameSnapshotTests.cs @@ -110,6 +110,30 @@ public async Task EachFrame_NumbersItsOwnRefsBehindItsOwnIndex() /// /// A page whose only content is an element hosting one frame, and that frame's own document. /// + [TestMethod] + public async Task AFrameThatCannotEvaluateYet_DoesNotHoldUpThePageSnapshot() + { + // A lazy iframe below the fold has no document, so an evaluation in it waits for one. + var page = new FakeToolPage(Document( + Node("Iframe", "Payment frame", 7), + Node("Iframe", "Lazy frame", 8))); + var main = new FakeToolFrame(page, "http://example.test/"); + page.FrameTree = main; + + var loaded = main.AddChild("http://example.test/frame.html"); + loaded.Snapshot = Document(Node("button", "Pay now", 42)); + + var lazy = main.AddChild(""); + lazy.EvaluateNeverCompletes = true; + lazy.Snapshot = Document(); + + var snapshot = new PageSnapshotService(page).TakeSnapshotAsync(); + var finished = await Task.WhenAny(snapshot, Task.Delay(TimeSpan.FromSeconds(10))); + + Assert.AreSame(snapshot, finished, "the snapshot waited on a frame that could not answer"); + StringAssert.Contains(await snapshot, "button \"Pay now\""); + } + private static FakeToolPage PageWithOneFrame(out FakeToolFrame frame) { var page = new FakeToolPage(Document(Node("Iframe", "Payment frame", 7))); diff --git a/tests/Motus.Mcp.Tests/Tools/CoreToolsFakes.cs b/tests/Motus.Mcp.Tests/Tools/CoreToolsFakes.cs index 816011f..7b821a7 100644 --- a/tests/Motus.Mcp.Tests/Tools/CoreToolsFakes.cs +++ b/tests/Motus.Mcp.Tests/Tools/CoreToolsFakes.cs @@ -1016,10 +1016,18 @@ public ILocator LocatorByBackendNodeId(long backendNodeId) return page.RecordingLocator; } + /// + /// When set, never completes, like a frame that has no document + /// to evaluate in yet. + /// + public bool EvaluateNeverCompletes { get; set; } + public Task EvaluateAsync(string expression, object? arg = null) { Evaluated.Add(expression); - return Task.FromResult(default(T)!); + return EvaluateNeverCompletes + ? new TaskCompletionSource().Task + : Task.FromResult(default(T)!); } public Task EvaluateAsync(string expression, object? arg, EvaluateOptions options) From 8ff01dd9f242075b30ed8b0e578cfc1d0e4bdab6 Mon Sep 17 00:00:00 2001 From: Torrey Betts Date: Sat, 24 Oct 2026 13:12:12 -0400 Subject: [PATCH 3/3] Keep a frame's new context when its old target leaves afterwards A cross-origin frame that navigates back to its parent's origin gets its new context from the parent's session while its own target goes away, and the two are read on separate loops. Handling the target's departure dropped whatever context was recorded for the frames it hosted, including one the parent's session had just reported, so the next evaluation in that frame waited out the full context timeout for a context that had already arrived. The departure now drops only the contexts its own session reported, matching how destroyed and cleared notices are already scoped by session. Signed-off-by: Torrey Betts --- src/Motus/Page/Page.ExecutionContexts.cs | 21 ++++++++ src/Motus/Page/Page.Frames.cs | 3 +- .../Page/ExecutionContextTrackingTests.cs | 36 +++++++++++++ .../Page/OutOfProcessFrameTests.cs | 51 +++++++++++++++++++ 4 files changed, 110 insertions(+), 1 deletion(-) diff --git a/src/Motus/Page/Page.ExecutionContexts.cs b/src/Motus/Page/Page.ExecutionContexts.cs index ee69c04..2fc929c 100644 --- a/src/Motus/Page/Page.ExecutionContexts.cs +++ b/src/Motus/Page/Page.ExecutionContexts.cs @@ -179,6 +179,27 @@ private void OnExecutionContextsCleared(IMotusSession source) } } + /// + /// Forgets every context reported over a session that has gone away. + /// + /// + /// Only contexts that session reported are dropped, not every context of the frames it hosted. + /// A frame moving back into its parent's process has its new context reported by the parent's + /// session, and that can be recorded before the old target's departure is handled. Dropping it + /// then would leave the frame waiting for a context that has already arrived. + /// + private void ForgetExecutionContextsFrom(string? sessionId) + { + if (sessionId is null) + return; + + foreach (var (frameId, context) in _frameIdToExecutionContext) + { + if (context.Session.SessionId == sessionId) + _frameIdToExecutionContext.TryRemove(new KeyValuePair(frameId, context)); + } + } + /// /// Drops the frame's isolated world when it is the one with the given id, so the next ask makes /// a fresh one rather than handing back a world the browser has torn down. diff --git a/src/Motus/Page/Page.Frames.cs b/src/Motus/Page/Page.Frames.cs index a9456c2..a8ce363 100644 --- a/src/Motus/Page/Page.Frames.cs +++ b/src/Motus/Page/Page.Frames.cs @@ -127,11 +127,12 @@ private void OnTargetDetached(TargetDetachedFromTargetEvent evt) // it to the page session, and Page.frameDetached from the parent stays the only signal // that a frame has actually gone away. _frameIdToSession.TryRemove(frameId, out _); - _frameIdToExecutionContext.TryRemove(frameId, out _); _frameIdToIsolatedWorld.TryRemove(frameId, out _); _frameTargetInit.TryRemove(frameId, out _); } + ForgetExecutionContextsFrom(evt.SessionId); + if (_context.SessionRegistry.TryGetSession(evt.SessionId, out var detached)) detached?.CleanupChannels(); diff --git a/tests/Motus.Tests/Page/ExecutionContextTrackingTests.cs b/tests/Motus.Tests/Page/ExecutionContextTrackingTests.cs index a50f2ec..cd07019 100644 --- a/tests/Motus.Tests/Page/ExecutionContextTrackingTests.cs +++ b/tests/Motus.Tests/Page/ExecutionContextTrackingTests.cs @@ -337,6 +337,33 @@ public async Task ContextEventsFromOneSession_LeaveAnotherSessionsContextAlone() Assert.AreEqual(5, last.ContextId, "the frame's own process still holds its context"); } + [TestMethod] + public async Task FrameReturningToThePageProcess_KeepsTheContextItReportedThere() + { + var page = await _browser.NewPageAsync(); + ContextCreated(5, "frame-main"); + AdoptOutOfProcessFrame(); + await Settle(250); + ContextCreated(5, "frame-oop", sessionId: "session-2"); + await Settle(); + + // The frame navigates back to its parent's origin. The page's process reports the new + // document's context, and the frame's own target goes away. The two travel separately, so + // the context can be recorded before the target's departure is handled, and the departure + // must not take the new context with it. + ContextCreated(20, "frame-oop"); + await Settle(); + TargetDetached("session-2"); + await Settle(); + + var oop = Frame(page, "frame-oop"); + Assert.AreEqual(2, await oop.EvaluateAsync("1 + 1").WaitAsync(TimeSpan.FromSeconds(5))); + + var last = LastEvaluation(); + Assert.AreEqual("session-1", last.SessionId); + Assert.AreEqual(20, last.ContextId, "the context the page's process reported for the frame"); + } + private (string? SessionId, int? ContextId) LastEvaluation() { Assert.IsTrue(_evaluations.Count > 0, "no Runtime.evaluate was sent"); @@ -400,6 +427,15 @@ private void DetachFrame(string frameId) } """); + private void TargetDetached(string sessionId) + => _socket.Enqueue($$""" + { + "method": "Target.detachedFromTarget", + "sessionId": "session-1", + "params": { "sessionId": "{{sessionId}}" } + } + """); + /// /// Has the page adopt a frame that renders in a process of its own, reached over session-2. /// diff --git a/tests/Motus.Tests/Page/OutOfProcessFrameTests.cs b/tests/Motus.Tests/Page/OutOfProcessFrameTests.cs index a64be1f..1d475b3 100644 --- a/tests/Motus.Tests/Page/OutOfProcessFrameTests.cs +++ b/tests/Motus.Tests/Page/OutOfProcessFrameTests.cs @@ -315,6 +315,57 @@ private static (int Width, int Height) ReadPngSize(byte[] png) return (width, height); } + /// + /// A frame that leaves its own process for its parent's is the same frame, and evaluating in it + /// once it gets there has to reach the document it now holds. + /// + /// + /// The page's process reports the frame's new context and the frame's old target goes away, + /// and those two arrive separately. Handling the departure second must not forget the context + /// the page's process has just reported, or the next evaluation waits for one that has already + /// come. A few round trips give both orders a chance to happen. + /// + [TestMethod] + public async Task CrossOriginFrame_MovingIntoItsParentsProcess_StillEvaluates() + { + var middle = await FrameAsync("middle"); + var page = (Motus.Page)_page!; + + for (var i = 0; i < 4; i++) + { + await _page!.EvaluateAsync( + $"document.getElementById('remote').src = '{_server.PrimaryOrigin}/deep.html?i={i}'"); + await WaitUntilAsync( + () => middle.Url.Contains($"/deep.html?i={i}", StringComparison.Ordinal) && !page.HasOwnSession(middle), + "the frame never moved into its parent's process"); + + Assert.AreEqual( + "deep", + await middle.EvaluateAsync("window.marker").WaitAsync(TimeSpan.FromSeconds(10)), + $"round trip {i}"); + + await _page.EvaluateAsync( + $"document.getElementById('remote').src = '{_server.SecondaryOrigin}/middle.html?i={i}'"); + await WaitUntilAsync( + () => middle.Url.Contains($"/middle.html?i={i}", StringComparison.Ordinal) && page.HasOwnSession(middle), + "the frame never moved back into a process of its own"); + + Assert.AreEqual( + "middle", + await middle.EvaluateAsync("window.marker").WaitAsync(TimeSpan.FromSeconds(10)), + $"round trip {i}"); + } + + static async Task WaitUntilAsync(Func condition, string message) + { + var deadline = DateTime.UtcNow.AddSeconds(10); + while (!condition() && DateTime.UtcNow < deadline) + await Task.Delay(50); + + Assert.IsTrue(condition(), message); + } + } + [TestMethod] public async Task DetachingAFrame_LeavesItsHandleReportingDetached() {