fix(chrome): wait out a route transition before sampling facts

the tree stays byte-identical and quiet across a cross-fade, so both the
quiet timer and the unchanged-tree escape called it settled mid-flight
and extractors read two screens at once.
This commit is contained in:
pj committed 2026-08-13 21:45:24 +05:30
1 parent 5c420ef72a
commit 5017036b62
2 files changed
+232 -8

No files matched your search

+102 -8
View File
@@ -543,36 +543,83 @@ func (d *Driver) RecentLogs(_ context.Context, since time.Time, minLevel string)
// text, and the next step types into a field it believes is still empty. // text, and the next step types into a field it believes is still empty.
const domQuietPeriod = 150 * time.Millisecond const domQuietPeriod = 150 * time.Millisecond
// transitionSettlePeriod is how much longer the settle waits for a route
// transition to finish once the DOM has gone quiet. A canvas app's cross-fade
// is invisible to a mutation observer: Compose splices the incoming screen's
// accessibility nodes in when the animation STARTS and removes the outgoing
// screen's when it ends, and nothing in between touches the DOM, so the tree
// sits byte-identical (and quiet) with both routes live for the whole
// animation. Settling on quiet alone returns there, and the next step then
// verifies a tree that names the screen the app is leaving: on the folio wasm
// build a submit that landed on Home was recorded as still being on the
// transaction screen, so a property gated on where the action landed read the
// wrong route and went vacuous. The wait is bounded so a page that genuinely
// shows two *Screen ids at rest costs this much per step and no more.
const transitionSettlePeriod = 800 * time.Millisecond
func (d *Driver) WaitForIdle(ctx context.Context, timeout time.Duration) error { func (d *Driver) WaitForIdle(ctx context.Context, timeout time.Duration) error {
runCtx, cancel := d.runCtx(ctx) runCtx, cancel := d.runCtx(ctx)
defer cancel() defer cancel()
// Leave the caller's deadline some room: returning late by our own doing // Leave the caller's deadline some room: returning late by our own doing
// would surface as a context cancellation instead of a settled page. // would surface as a context cancellation instead of a settled page.
budget := max(timeout-100*time.Millisecond, domQuietPeriod) budget := max(timeout-100*time.Millisecond, domQuietPeriod)
script := fmt.Sprintf(settleScript, domQuietPeriod.Milliseconds(), budget.Milliseconds()) script := fmt.Sprintf(settleScript,
domQuietPeriod.Milliseconds(),
budget.Milliseconds(),
transitionSettlePeriod.Milliseconds(),
)
return chromedp.Run(runCtx, return chromedp.Run(runCtx,
chromedp.WaitReady("body", chromedp.ByQuery), chromedp.WaitReady("body", chromedp.ByQuery),
chromedp.Evaluate(script, nil, awaitPromise), chromedp.Evaluate(script, nil, awaitPromise),
) )
} }
// settleScript resolves once the document has gone quiet for %d ms, or after // liveScreensFunction defines liveScreens(), the page-side count of live ids
// %d ms whatever happens. Shadow roots get their own observer: a canvas app // ending in "Screen". More than one is a route transition in flight: the same
// keeps its whole accessibility tree inside one, and mutations there do not // rule the tree parser applies (Transitional in internal/hierarchy), so the
// driver and the runner agree on what a settled route looks like. It descends
// shadow roots because a canvas app keeps its whole accessibility tree inside
// one.
const liveScreensFunction = `
const liveScreens = () => {
let count = 0;
const visit = (root) => {
count += root.querySelectorAll('[id$="Screen"]').length;
for (const element of root.querySelectorAll('*')) {
if (element.shadowRoot) visit(element.shadowRoot);
}
};
visit(document);
return count;
};`
// settleScript resolves once the document has gone quiet for %d ms and is not
// mid route transition, or after %d ms whatever happens; the transition wait
// itself gives up after %d ms. Shadow roots get their own observer: a canvas
// app keeps its whole accessibility tree inside one, and mutations there do not
// reach an observer on the document. // reach an observer on the document.
const settleScript = ` const settleScript = `
new Promise(resolve => { new Promise(resolve => {
const quietMillis = %d, budgetMillis = %d; const quietMillis = %d, budgetMillis = %d, transitionMillis = %d;
const observers = []; const observers = [];
const transitionDeadline = Date.now() + transitionMillis;
let timer = null; let timer = null;
const finish = () => { const finish = () => {
clearTimeout(timer); clearTimeout(timer);
for (const observer of observers) observer.disconnect(); for (const observer of observers) observer.disconnect();
resolve(); resolve();
}; };
` + liveScreensFunction + `
const quiet = () => {
if (liveScreens() > 1 && Date.now() < transitionDeadline) {
timer = setTimeout(quiet, 16);
return;
}
finish();
};
const restart = () => { const restart = () => {
clearTimeout(timer); clearTimeout(timer);
timer = setTimeout(finish, quietMillis); timer = setTimeout(quiet, quietMillis);
}; };
const watch = (root) => { const watch = (root) => {
const observer = new MutationObserver(restart); const observer = new MutationObserver(restart);
@@ -683,12 +730,21 @@ func (d *Driver) InstallBundle(ctx context.Context, source []byte) error {
// EvaluateExtractors invokes the bundle-installed extractor table and returns // EvaluateExtractors invokes the bundle-installed extractor table and returns
// each extractor's JSON-encoded current value keyed by its registration index. // each extractor's JSON-encoded current value keyed by its registration index.
//
// The read waits out a route transition first, bounded by
// transitionSettlePeriod. The hierarchy fetch already re-fetches a transitional
// tree (fetchSyncedState in internal/runner); without the same rule here the
// two halves of one step describe different moments, and the spec's own
// extractors are the half that loses: on the folio wasm build the extractors
// sampled mid cross-fade and reported the route the app was leaving, so a
// property gated on where the action landed skipped the only step that action
// could be judged on.
func (d *Driver) EvaluateExtractors(ctx context.Context) (map[int]json.RawMessage, error) { func (d *Driver) EvaluateExtractors(ctx context.Context) (map[int]json.RawMessage, error) {
const script = `JSON.stringify(window.__sanderlingExtractors__ ? window.__sanderlingExtractors__() : {})` script := fmt.Sprintf(extractorScript, transitionSettlePeriod.Milliseconds())
var encoded string var encoded string
runCtx, cancel := d.runCtx(ctx) runCtx, cancel := d.runCtx(ctx)
defer cancel() defer cancel()
if err := chromedp.Run(runCtx, chromedp.Evaluate(script, &encoded)); err != nil { if err := chromedp.Run(runCtx, chromedp.Evaluate(script, &encoded, awaitPromise)); err != nil {
return nil, fmt.Errorf("evaluate extractors: %w", err) return nil, fmt.Errorf("evaluate extractors: %w", err)
} }
if encoded == "" || encoded == "{}" { if encoded == "" || encoded == "{}" {
@@ -709,6 +765,44 @@ func (d *Driver) EvaluateExtractors(ctx context.Context) (map[int]json.RawMessag
return result, nil return result, nil
} }
// SetLastAction installs the previous step's action as state.lastAction inside
// the page runtime. The page cannot derive it: only the runner knows which
// action was actually applied. Without this call every web state.lastAction is
// null, so a property gated on what the last action did is vacuously true and
// reports a green run while checking nothing.
func (d *Driver) SetLastAction(ctx context.Context, encoded json.RawMessage) error {
payload := strings.TrimSpace(string(encoded))
if payload == "" {
payload = "null"
}
script := fmt.Sprintf(
`window.__sanderlingSetLastAction__ && window.__sanderlingSetLastAction__(%s)`,
payload,
)
runCtx, cancel := d.runCtx(ctx)
defer cancel()
if err := chromedp.Run(runCtx, chromedp.Evaluate(script, nil)); err != nil {
return fmt.Errorf("set last action: %w", err)
}
return nil
}
// extractorScript resolves the extractor table once the page is not mid route
// transition, giving up on that wait after %d ms.
const extractorScript = `
new Promise(resolve => {
const deadline = Date.now() + %d;` + liveScreensFunction + `
const read = () => {
if (liveScreens() > 1 && Date.now() < deadline) {
setTimeout(read, 16);
return;
}
resolve(JSON.stringify(
window.__sanderlingExtractors__ ? window.__sanderlingExtractors__() : {}));
};
read();
})`
// NextActionFromV8 invokes the bundle-installed action generator and returns // NextActionFromV8 invokes the bundle-installed action generator and returns
// the resulting Action JSON. Returns an empty json.RawMessage when the // the resulting Action JSON. Returns an empty json.RawMessage when the
// generator declines to act this tick. // generator declines to act this tick.
+130
View File
@@ -663,3 +663,133 @@ func TestWaitForIdle_ReturnsOnABusyPage(t *testing.T) {
t.Errorf("WaitForIdle took %s on a busy page; it must return inside its budget", elapsed) t.Errorf("WaitForIdle took %s on a busy page; it must return inside its budget", elapsed)
} }
} }
// TestWaitForIdle_WaitsOutARouteTransition covers the settle case a mutation
// observer cannot see. A canvas app splices the incoming screen's
// accessibility nodes in when its cross-fade STARTS and drops the outgoing
// screen's when it ends; between those two mutations the DOM is quiet with both
// routes live. Returning there hands the next step a tree naming the screen the
// app is leaving, which on the folio wasm build recorded a submit that had
// landed on Home as still being on the transaction screen: the route gate of an
// action-gated property then skipped the very step the action landed on.
func TestWaitForIdle_WaitsOutARouteTransition(t *testing.T) {
const page = `<body><div id="app"></div><script>
const root = document.getElementById("app").attachShadow({mode: "open"});
root.innerHTML = '<button id="go" style="width:200px;height:80px">go</button>' +
'<div id="LedgerScreen">ledger</div>';
root.getElementById("go").addEventListener("click", function () {
const incoming = document.createElement("div");
incoming.id = "HomeScreen";
incoming.textContent = "home";
root.appendChild(incoming);
setTimeout(function () { root.getElementById("LedgerScreen").remove(); }, 400);
});
</script></body>`
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "text/html")
_, _ = w.Write([]byte(page))
}))
defer server.Close()
d := New()
defer d.Terminate(context.Background())
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
if err := d.Launch(ctx, server.URL, false, nil); err != nil {
t.Fatalf("Launch: %v", err)
}
if err := d.Tap(ctx, 40, 40); err != nil {
t.Fatalf("Tap: %v", err)
}
if err := d.WaitForIdle(ctx, 2*time.Second); err != nil {
t.Fatalf("WaitForIdle: %v", err)
}
var live []string
script := `Array.from(document.getElementById("app").shadowRoot
.querySelectorAll('[id$="Screen"]')).map(e => e.id)`
if err := chromedp.Run(d.tabCtx, chromedp.Evaluate(script, &live)); err != nil {
t.Fatalf("read: %v", err)
}
if len(live) != 1 || live[0] != "HomeScreen" {
t.Errorf("live screens after the settle = %v, want [HomeScreen]; WaitForIdle "+
"returned mid-transition, so the next step verifies the outgoing route", live)
}
}
// TestWaitForIdle_BoundsTheTransitionWait is the other half of the transition
// wait: a page that shows two *Screen ids at rest is not mid-transition, it
// just matches the heuristic, and it must cost one bounded wait rather than the
// whole step budget on every step.
func TestWaitForIdle_BoundsTheTransitionWait(t *testing.T) {
const page = `<body><div id="HomeScreen">home</div><div id="LedgerScreen">ledger</div></body>`
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "text/html")
_, _ = w.Write([]byte(page))
}))
defer server.Close()
d := New()
defer d.Terminate(context.Background())
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
if err := d.Launch(ctx, server.URL, false, nil); err != nil {
t.Fatalf("Launch: %v", err)
}
start := time.Now()
if err := d.WaitForIdle(ctx, 10*time.Second); err != nil {
t.Fatalf("WaitForIdle: %v", err)
}
if elapsed := time.Since(start); elapsed > transitionSettlePeriod+time.Second {
t.Errorf("WaitForIdle took %s on a page with two resting screens; the "+
"transition wait must be bounded by %s", elapsed, transitionSettlePeriod)
}
}
// TestEvaluateExtractors_WaitsOutARouteTransition covers the other sampler. A
// step reads the page twice: the hierarchy dump (which re-fetches while the
// tree looks transitional) and the spec's own extractors in V8. Sampling the
// extractors mid cross-fade reports the route the app is leaving, and an
// action-gated property then skips the one step its action can be judged on:
// on the folio wasm build a double-submit that landed on Home was extracted as
// still being on the transaction screen.
func TestEvaluateExtractors_WaitsOutARouteTransition(t *testing.T) {
const page = `<body><div id="app"></div><script>
const root = document.getElementById("app").attachShadow({mode: "open"});
root.innerHTML = '<div id="LedgerScreen">ledger</div>';
window.__sanderlingExtractors__ = function () {
return {0: Array.from(root.querySelectorAll('[id$="Screen"]')).map(e => e.id).join(",")};
};
window.startTransition = function () {
const incoming = document.createElement("div");
incoming.id = "HomeScreen";
root.appendChild(incoming);
setTimeout(function () { root.getElementById("LedgerScreen").remove(); }, 400);
};
</script></body>`
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "text/html")
_, _ = w.Write([]byte(page))
}))
defer server.Close()
d := New()
defer d.Terminate(context.Background())
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
if err := d.Launch(ctx, server.URL, false, nil); err != nil {
t.Fatalf("Launch: %v", err)
}
if err := chromedp.Run(d.tabCtx, chromedp.Evaluate(`window.startTransition()`, nil)); err != nil {
t.Fatalf("start transition: %v", err)
}
values, err := d.EvaluateExtractors(ctx)
if err != nil {
t.Fatalf("EvaluateExtractors: %v", err)
}
if got := string(values[0]); got != `"HomeScreen"` {
t.Errorf("extractor read %s, want \"HomeScreen\"; the extractors sampled "+
"mid-transition, so the spec sees the route the app is leaving", got)
}
}