fix(focus): a restored page is ready for keys from a paint it cannot miss (follow-up of #32633)

Merged
LA
3d ago
+111-0

Summary

Follow-up of #32633 (P0 reopen-browser-plain-keys). With #32633, a restored Chromium page takes held keys only after its restored document loaded and painted. In about 1 of 43 trace restores no paint report came (the DevTools firstContentfulPaint lifecycle event on fork API 24, CEFFirstPaintBridge). The page then never counted as ready, and the keys waited the 2 s creation-hold bound before they arrived in order. No key was lost (REOPEN VIDEO round rpk-trace2-cmux22/a3, STATUS 8918, 9015).

This PR removes the bound's role in that case. Readiness comes from a signal that cannot be lost. At load end, when the current document loaded and no paint report came yet, the tab asks the page once: BrowserFirstPaintScript.query, a PerformanceObserver of type paint with buffered: true, awaited, so a paint before the ask still counts. A true answer makes the page ready. A new commit, a new browser or the browser's end cancels a pending ask, so an answer about a replaced document never applies. The 2 s bound is not changed or tuned. The trace line paint-query painted shows when the answer made the page ready.

Testing

Fleet, package-test-lane.sh suite, 5 runs each, proof-step.py. A step counts only with "Test run with N tests", N > 0.

  • Red 933b834 (the tests, with the seam not yet called): step 5d7af84a32595f2e6ffd5344, CEFDeferredFocusTests failed 5/5 on aLoadedPageWhosePaintReportIsLostIsReadyFromThePagesOwnAnswer and aPaintAnswerForAReplacedDocumentOrAFailedAnswerMakesNothingReady.
  • Green 0958383: step 25005714b56aca84a1ad9118, 19 test runs passed.
  • Green at this head 37c8bbc (with #32633's merged head 9778e42): step b26a9e0499e78d6814c7a6d3, 19 test runs passed (CEFDeferredFocusTests 18 tests, AwaitedContentTests, CreationInputHoldTests 5x each, plus CEFFirstPaintBaseTests, ColdBrowserFocusKeysTests, PageFirstKeysTests, ColdFocusedTerminalTypeAheadTests). Compile-check app and CmuxNext passed at 37c8bbc.
  • Green at this head 125c5de (feat merged in after #32633 landed; the PR diff is unchanged): step 820862076e8e58edf2f01ff8, 19 test runs passed. Compile-check app and CmuxNext passed at 125c5de.
  • Journeys (real keys, kit 4ff1d5e5ce6, reopen-browser-plain-keys): 12/12 pass on build f221cf12 (this fix + #32537), 6 on cmux22 and 6 on cmux23, 0 rounds at the 2 s bound. In round rpk-pq-cmux22/a3, loading stopped at 193 ms with no paint report, and the page was ready from the query at 272 ms.

Observed / Not run

  • Observed: the runs above.
  • Known, not in this PR (review 6608769157): a real page that never has a contentful paint (an empty body, only colored boxes) still waits the 2 s bound; that is #32633's readiness rule. Such a page answers no within 5 s (the evaluate's own bound) and stays waiting.
  • Not run: a renderer crash between a true answer and the reload's commit (the review calls it very unlikely; the paint report has the same gap).

Changelog

Fixed: Keys you type while a restored web page loads reach it as soon as it shows, also when Chromium does not report its first paint

Proof

No capture: key routing; the proof is the tests and the journeys.