Repository navigation
fix(focus): keys typed right after a reopen wait until the restored page has loaded, then reach it in order (dogfood port of #32633) - #32684
Draft
lawrencecchen wants to merge 6 commits into
Conversation
…d loading on a real document or an error page; a page that never gets ready is not held again after its bound; two ready signals never send a held key twice (reopen-browser-plain-keys cycle B, dogfood port of #32633; red: the behavior is stubbed)
…age stopped loading on a real document, then reach it in order; a page that never gets ready ends the wait at its bound (reopen-browser-plain-keys cycle B, dogfood port of #32633)
…s document, a restored blank tab counts its blank one, and Cmd-L passes an empty page hold once the page exists (dogfood port of the #32633 review fix; red: stubs)
…a restored blank tab counts its blank one); a New Tab page and other new pages take keys once Chromium created them, and Cmd-L passes an empty page hold once the page exists (dogfood port of the #32633 review fix)
…that page, not the empty document before its navigation; a page with nothing to load is ready at once (dogfood port; lead on STATUS 7916; red)
…cument, a page with nothing to load is ready at once, and only a blank target counts a blank document, so a reopen by address no longer sends held keys to the empty document before its navigation (dogfood port; lead on STATUS 7916)
Contributor
Author
|
Not needed: the dogfood line is retired as a dogfood source; dogfood builds come from the feat tip (lead, STATUS 7979). The fix lands through the feat PR (link below) and reaches dogfood with the next feat dogfood build. Feat PR: #32633. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Dogfood port of #32633 (P0
reopen-browser-plain-keys, cycle B; journey kit cp/reopen-video-kit@4ff1d5e5ce6). After Cmd-Q (keep) and reopen, plain keys typed at once into a restored web page were lost. Lawrence's rule: a key typed before the restored content is ready is held and delivered in order, never dropped.Cause (as on feat, REOPEN VIDEO 7493, 7660, 7710, 7801): the window key hold awaited a restored Chromium tab only until its pane showed the content. The tab shows before Chromium creates its browser, and its first loads are an empty document and documents that the same load replaces. Keys sent on at any of those points were lost.
Fix, same rule as #32633:
AwaitedContent.Pane.shownPageReady,BrowserTab.pageReady). It is ready when Chromium created its browser and stopped loading (.loadingState(loading: false)) with a real (not blank) document loaded, or with an error page. An aborted load is no page, and a new commit forgets the document loaded before it (Chrome'sDidStopLoading).CEFTab.onReadyForKeysreports it once per browser and writesready-for-keysto the lifecycle trace.TabContentCacheforwards it toonBrowserReady. On this lineonBrowserReadydid not end a page hold (only a focus settle did), so it now calls the newCreationInputCoordinator.pageArrived, which flushes the holds that await that tab, each key once, in order.holdLimit(2 s) and its keys go on in order. This line lacked feat'sCreationHoldLog.pageExpired(fix(focus): the reopen key hold also waits for a restored browser tab's page; Cmd-L reaches its address bar (REOPEN-F4 browser case) #32030), so with the new readiness rule the next key would open a new 2 s hold for the same page. This PR ports it: a page whose hold ended at its bound is not held again until it is ready.ifblocks inTabContentCacheand theonBrowserReadyline.Cost: none per key. One flag check when a hold decides what it awaits; one callback per browser.
Testing
Fleet, 5 runs each,
proof-step.py(step ids follow in a comment when done):AwaitedContentTests(aShownBrowserTabIsAwaitedUntilItsPageIsReady,aTabExcludedAfterItsBoundIsHeldAgainOnceItsPageArrived),CEFTabHostFocusTests.aTabIsReadyForKeysOnlyOnceLoadingStoppedOnARealDocument.CreationInputHoldTests(withtwoReadySignalsWithKeysHeldBetweenThemDeliverEachKeyOnceInOrder, a guard that is green before and after),ColdBrowserFocusKeysTests,PageFirstKeysTests.reopen-browser-plain-keys(both cycles) andbrowser-click-then-type, 3 rounds each on a dogfood proof build of this PR + fix(focus): keys typed into a page right after an address-bar load reach it (dogfood port of #32537) #32579 (link hints) + dogfood port: cmux next focus: no reclaim inside the grant of a page that still has focus (port of #25689) #32654 (ChildPageFocus grant guard: the dogfood cycle A miss in STATUS 7801).Observed / Not run
ready-for-keysin every cycle B round.Changelog
Fixed: Keys you type at once into a web page that a reopen restores reach the page, in order
Proof
No capture: key routing; the proof is the tests and the journeys.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes lost keystrokes after a reopen: keys typed right after Cmd-Q (keep) and reopen now wait until the restored Chromium page has actually loaded, then reach it in order.
After a reopen, a restored Chromium tab showed in its pane before Chromium created its browser, and its first loads were an empty document or documents the same load replaced, so keys sent immediately never reached the page.
CEFTab.onReadyForKeysreports once per browser, and the hold's keys are flushed once each, in order, when the page arrives.pageExpiredfrom fix(focus): the reopen key hold also waits for a restored browser tab's page; Cmd-L reaches its address bar (REOPEN-F4 browser case) #32030).Written for commit 2001f3b. Summary will update on new commits.