Skip to content

Fix idling? blocking on lazy iframes that Chrome never loads#585

Open
agrberg wants to merge 1 commit into
rubycdp:mainfrom
agrberg:ar/fix-idling-lazy-iframes
Open

Fix idling? blocking on lazy iframes that Chrome never loads#585
agrberg wants to merge 1 commit into
rubycdp:mainfrom
agrberg:ar/fix-idling-lazy-iframes

Conversation

@agrberg
Copy link
Copy Markdown

@agrberg agrberg commented May 17, 2026

Fixes #583.

TL;DR

Pages with loading="lazy" iframes outside the viewport hang page.go_to for the full browser timeout (default 5s) because Chrome attaches such iframes but never starts loading them, leaving Frame#state at nil. The previous idling? required every frame to be :stopped_loading, so any nil-state lazy frame blocked idle detection. The fix relaxes the predicate to also accept nil.

Problem

Chrome fires Page.frameAttached for every iframe in the DOM, but only fires Page.frameStoppedLoading for frames it actually loads. For a loading="lazy" iframe parked outside the viewport (for example inside a closed <details>), Chrome never starts loading it: no frameStartedLoading, no network request, no frameStoppedLoading. The frame's state stays at nil indefinitely.

The previous idling? required every frame to be :stopped_loading:

def idling?
  @frames.values.all? { |f| f.state == :stopped_loading }
end

so any lazy iframe blocked idle detection for the full browser timeout. The reporter confirmed via FERRUM_DEBUG=true that Frame#state was nil for the stuck frame.

Fix

Treat freshly-attached frames (state nil) as idle. A frame that has never started loading is not something to wait for.

def idling?
  @frames.values.all? { |f| f.state == :stopped_loading || f.state.nil? }
end

Semantics: a frame is idle if it has finished loading or has never started loading. This is consistent with Frame#state's documented nil initial value and with subscribe_frame_attached deliberately leaving state unset on attach.

I considered the alternative of setting frame.state = :stopped_loading inside subscribe_frame_attached, but that would lie about state (a freshly-attached frame hasn't stopped anything) and would break any code that distinguishes "attached but never loaded" from "loaded". The predicate change is documented inline so future cleanup doesn't quietly re-close the issue.

Behavior change for in-viewport lazy iframes

Ferrum can't tell at idle-check time whether a nil-state frame is one Chrome will never load (the #583 bug) or one Chrome is about to load (an in-viewport lazy iframe whose frameStartedLoading is deferred). Treating both as idle is the only correct choice for the never-load case; for the about-to-load case, the consequence is that page.go_to may now return before an in-viewport lazy iframe finishes loading. Chrome still loads the iframe; callers that need its content can wait on it explicitly via frame.body (which blocks until the frame is loaded) or page.network.wait_for_idle. The in-viewport regression test below documents and pins this behavior.

Test

Five regression tests in spec/frame_spec.rb. Each of the four bug-path tests bounds the failure with with_timeout(1) so a failing test fails fast, and asserts the operation completes well under that bound.

  • Scenario 1 — initial load: loading="lazy" iframe inside a closed <details> on initial page load. Bug surfaces as go_to returning after browser.timeout (silently rescued, no error raised).
  • Scenario 2 — click handler: a click handler attaches a lazy iframe and, by design, also triggers a same-document navigation (location.hash). The same-document nav is intentional: it causes idling? to be re-evaluated while the nil-state frame is in @frames. A full reload or cross-document nav here would fire Runtime.executionContextsCleared and overwrite the frame state, masking the bug. Bug surfaces as the click hanging until ferrum's internal mouse_event raises Ferrum::TimeoutError.
  • Reload: page.reload on a page with a lazy iframe. Bug surfaces as Ferrum::TimeoutError from page.reload.
  • Multiple lazy iframes: a page with two lazy iframes side by side, mirroring the reporter's real-world case (two YouTube embeds, structurally identical to two /slow iframes for the purposes of this bug — the iframe target doesn't matter, only that Chrome defers the load).
  • In-viewport lazy iframe: pins the behavior change called out above. Asserts that Chrome still loads the iframe and its content is reachable via frame.body even though page.go_to no longer waits for it.

Each of the four bug-path tests also asserts that a lazy frame in nil state is present at the bug's precondition point. This is load-bearing because Runtime.executionContextsCleared (in subscribe_execution_contexts_cleared) unconditionally overwrites every frame's state to :stopped_loading, which on some Chrome versions can fire during navigation and make the old idling? return true for the wrong reason. The nil-state assertion catches that false-pass. The reload test captures the precondition before page.reload runs, because reload itself triggers executionContextsCleared and clobbers the state before a post-reload assertion would run; the others capture it after the navigation under test.

Verified each bug-path test fails when the f.state.nil? branch is removed from idling?.

Files

  • lib/ferrum/page/frames.rb — one-line fix plus inline documentation of the nil branch
  • spec/frame_spec.rb — five regression tests
  • spec/support/views/lazy_iframe.erb — scenario 1 fixture
  • spec/support/views/lazy_iframe_via_click.erb — scenario 2 fixture
  • spec/support/views/lazy_iframes_two.erb — multi-frame fixture
  • spec/support/views/lazy_iframe_in_viewport.erb — in-viewport behavior-change fixture
  • CHANGELOG.md — entry under [Unreleased] → Fixed

@agrberg agrberg force-pushed the ar/fix-idling-lazy-iframes branch from 7ef14d6 to d4423ed Compare May 17, 2026 17:10
@agrberg
Copy link
Copy Markdown
Author

agrberg commented May 17, 2026

Pre-review pass on the regression test (force-pushed):

  • Replaced the wall-clock 2s threshold with a deterministic predicate test that asserts on idling? state directly. No timing assertions — fails on main, passes with the fix.
  • Added a second scenario from the issue: lazy iframe inserted by JS outside the viewport (the original report's scenario 2). Same code path, but explicit coverage.
  • Added the <link rel="icon" href="data:,"> line to the fixture to match the neighbor iframe fixtures.
  • Linked the changelog entry to the PR rather than the issue, matching the surrounding convention.
  • Added a comment block above the spec context explaining the Chrome-side root cause so the next reader doesn't have to git-blame.

Diff is still small: one production line + two specs + one fixture + one changelog line.

@agrberg agrberg force-pushed the ar/fix-idling-lazy-iframes branch from d4423ed to 25868a6 Compare May 17, 2026 17:43
@agrberg
Copy link
Copy Markdown
Author

agrberg commented May 17, 2026

Tiny review-pass-2 update (force-pushed):

The JS-injected lazy iframe test had a theoretical race — page.execute's CDP ack doesn't order with the Page.frameAttached event, so reading page.frames immediately after could miss the lazy frame on a slow CI. Wrapped the lookup in wait_for { ... }.not_to be_nil (the existing rspec-wait idiom used at spec/page_spec.rb:228), which polls until the frame appears or hits the 1s wait_timeout. The first scenario (closed <details> via go_to) was already deterministic — go_to waits for the initial frame tree — so it's unchanged.

No production code changes.

@agrberg agrberg force-pushed the ar/fix-idling-lazy-iframes branch from 25868a6 to 41be006 Compare May 17, 2026 18:03
@agrberg
Copy link
Copy Markdown
Author

agrberg commented May 17, 2026

Review-pass-3 update (force-pushed): both specs now use only public API — no more page.send(:idling?).

Approach. The bug's only externally-observable effect is the wall-clock duration of page.go_to: with the bug, go_to blocks for the full browser.timeout before silently rescuing the resulting TimeoutError; with the fix, it returns when the main frame's frameStoppedLoading fires. The earlier reviewer-suggested with_timeout + not_to raise_error shape doesn't work because go_to swallows the timeout. So the test wraps go_to in with_timeout(1) to bound the bug path to 1s, then uses Ferrum::Utils::ElapsedTime (matching the project pattern at spec/downloads_spec.rb:31, spec/network_spec.rb:506) to assert the call returns well under 0.5s. The fix path is typically <100ms — 5-10x headroom against the threshold. The lazy.state.nil? assertion confirms the spec actually hit the bug's precondition rather than passing for an unrelated reason.

JS-injection scenario. Reworked: instead of injecting via page.execute after go_to (which can't reproduce the blocking because by then the main frame is already idle and a subsequent navigation detaches the lazy frame before its frameStoppedLoading would matter), there's a new fixture spec/support/views/lazy_iframe_via_script.erb that injects the lazy iframe in an inline <script> during parse. That gives the same frameAttached-before-frameStoppedLoading ordering as the closed-<details> scenario, just via a different DOM construction path — and the test now genuinely reproduces the bug (verified on main: both specs fail at ~1.3s; with the fix they pass at <500ms).

No production code changes.

@agrberg agrberg force-pushed the ar/fix-idling-lazy-iframes branch from 5c83954 to 32e7b5c Compare May 17, 2026 19:59
Chrome fires Page.frameAttached for every iframe in the DOM, but only
fires Page.frameStoppedLoading for frames it actually loads. For a
loading="lazy" iframe parked outside the viewport (e.g. inside a closed
<details>, or injected by an inline <script> during page parse), Chrome
never starts loading it, so it never transitions to :started_loading or
:stopped_loading. It stays at state nil indefinitely.

The previous idling? required every frame to be :stopped_loading, which
meant any such iframe blocked idle detection for the full browser
timeout (default 5s). Treat freshly-attached frames (state nil) as idle
too — they haven't started loading, so there's nothing to wait for.

Specs use with_timeout(1) to bound the bug path's wall-clock and
Ferrum::Utils::ElapsedTime to measure (matches the project pattern at
spec/downloads_spec.rb:31, spec/network_spec.rb:506). Assertion
threshold is 0.5s; fix path is well under 100ms; bug path is bounded
to ~1s. The lazy.state.nil? assertion confirms the spec hit the bug's
precondition rather than passing because no lazy frame was attached.

Fixes rubycdp#583
@agrberg agrberg force-pushed the ar/fix-idling-lazy-iframes branch from 32e7b5c to bc2d0b6 Compare May 17, 2026 20:22
@agrberg
Copy link
Copy Markdown
Author

agrberg commented May 17, 2026

Similar to #586 I used Claude Code for this and personally reviewed all the code and comments to ensure it takes the least amount of human time to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

idling? blocks for full browser.timeout when lazy iframes never fire Page.frameStoppedLoading

1 participant