Fix re-layout loop from leaked NTP global layout listener in native input - #9787
Open
LukasPaczos wants to merge 1 commit into
Open
LukasPaczos wants to merge 1 commit into
LukasPaczos wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
Author
|
Hey @malmstein, this is ready for review. I don't think it should cause any issues to actual offsets being applied, but I'd appreciate if you could double-check edge cases that you know much better than I do. |
21 tasks
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.
Task/Issue URL: https://app.asana.com/1/137249556945/project/1204912272578138/task/1218383698693718?focus=true
Tech Design URL (if applicable):
API Proposals URL(s) (if applicable):
Description
On a Galaxy Tab S9+ (doesn't reproduce the same on all devices), holding backspace in the native input widget became ~3x slower (about 220 ms per character instead of about 66 ms) after switching tabs a few times via the tab switcher. Swiping between tabs does not reproduce it.
Cause:
NativeInputLayoutCoordinator.configureContentOffsetregistered anOnGlobalLayoutListeneronntpContentView.viewTreeObserverand removed it via the same getter on widget detach. When a tab is opened from the tab switcher while the previous tab had the input open, the outgoing tab'sBrowserTabViewModel#onViewVisibleis triggered, showing the native input and registering the global layout listener, split second before its torn down for the selected tab instance. By the time the widget's detach callback runs, the NTP content is already detached and the getter returns a different observer, so the removal is a no-op. The listener leaks, once per switch.Each leaked session (widget not shown, wants padding 0) then fights the live session (wants the widget offset) on every global layout pass, both trying to
setPaddingwhich posts arequestLayout. This keeps the main thread busy relayouting on every frame, so each keyboard event has to wait for a free frame before the app can handle it. Held backspace, which fires many events per second, is where this shows up most. Nothing visibly redraws, so it looks like a slow text field.Fix: capture the observer instance at registration and remove from that instance.
Steps to test this PR
Held backspace after tab switches
No regression in content offset
Note
Low Risk
Small lifecycle fix in native input content-offset setup; reduces leak/re-layout risk without changing offset logic.
Overview
Fixes a leaked NTP
OnGlobalLayoutListenerinNativeInputLayoutCoordinator.configureContentOffsetthat could accumulate after tab-switcher tab changes when native input was open on the outgoing tab.Registration still adds the listener on the new-tab content’s
ViewTreeObserver, but cleanup now removes it from the same observer instance captured at add time (ntpObserver) instead of callingntpContentView.viewTreeObserveragain on widget detach. After the NTP view is gone, that getter can return a different observer, so the old removal path was a no-op and stale listeners kept callingapplyOffset/setPaddingon every global layout—fighting the live session and causing extra relayout (e.g. very slow held backspace).Comments document why the observer reference must be held for the session’s lifetime.
Reviewed by Cursor Bugbot for commit 2b3ad03. Bugbot is set up for automated code reviews on this repo. Configure here.