Hold a decor view through its window, and the window through its screen - #2943
Closed
pyricau wants to merge 1 commit into
Closed
Hold a decor view through its window, and the window through its screen#2943pyricau wants to merge 1 commit into
pyricau wants to merge 1 commit into
Conversation
pyricau
force-pushed
the
owner-rules-as-code
branch
from
August 6, 2026 05:11
47066fd to
c4e8739
Compare
The decor view rule was `Activity.mDecor` / `Dialog.mDecor`, and that field is null on a live activity more often than not. `handleResumeActivity` is the only place that writes it, under `r.window == null`, so it is set once per `ActivityClientRecord` rather than once per `Activity` — a screen recreated on a configuration change is resumed, visible, and has a null `mDecor` for the rest of its life. `PhoneWindow.mDecor` has no such gap: every decor view in the seven real dumps here is pointed at by exactly one. Moving the decor view under its window puts a window's 3 MB where the window lands, so the window needs a rule too — 6 of the 16 windows in those dumps were drawn at the top of the tree, held by a `ViewRootImpl` or a `WindowManagerImpl` as well as by their activity. With the second rule every decor view is under its window and every window of a screen that is up is under that screen. That second rule is the one here that reads the state of its owner, and it has to: `Activity.mWindow` is set in `attach` and never cleared and `Dialog.mWindow` is final, so a destroyed activity and a dismissed dialog go on pointing at a window they have nothing to do with. Owning it anyway took the window of a leaked screen away from whatever else held it, and turned two leaks into one on the list. Bytes are conserved on all eight dumps in the repo, no object joins or leaves the tree, and 0 to 16 objects of 34 K to 340 K move. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pyricau
force-pushed
the
window-owns-the-decor-view
branch
from
August 6, 2026 05:12
f29d807 to
cd0f0fc
Compare
Member
Author
|
🤖 Folded into #2941 — the two commits are one change, so they're one PR now. The state read this branch had is gone: |
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.
Stacked on #2941. The question this answers: is there always a window when there's a decor view, and if so, should the rule be
PhoneWindow.mDecorrather thanActivity.mDecor?Is there always a window?
Yes, for a decor view: every one in the seven real dumps in the repo is pointed at by exactly one
PhoneWindow.mDecor(1 of 1 incompose_leak.hprof, 3 of 3 inleak_asynctask_m.hprof, and so on).Not for a root view, which is the distinction that matters: 14 of the 15 root views in
compose_leak.hprof, 9 of the 10 inleak_asynctask_m.hprofand 22 of the 23 ingcroot_unknown_object.hprofhave no window anywhere. They'reToastviews,PopupWindow.mContentView,TextView$MagnifierViews,Toolbar.mNavButtonViewimage buttons and fragment roots, each held the ordinary way by whatever made it. So the rule is window → decor view, and the rest need nothing.Why
Activity.mDecorhad to goActivityThread.handleResumeActivityis the only place that writes it, guarded byif (r.window == null && !a.mFinished && willBeVisible)— so it is set once perActivityClientRecord, not once perActivity. A screen recreated on a configuration change is resumed, visible, and has a nullmDecorfor the rest of its life. It also inverted the nesting where it was set: inleak_asynctask_m.hprofaPhoneWindowwas drawn under its ownDecorView, throughDecorView.this$0.Why the window then needs a rule of its own
A window has 6 to 9 referrers in these dumps. Most are its own inner classes and its decor's, but the external ones —
WindowManagerImpl.mParentWindow,ActivityThread$ActivityClientRecord.window,DecorView.mWindowreached from aViewRootImpl— put 6 of the 16 windows at the top of the tree. Moving the decor view under the window makes that worse than it was, because the hierarchy now floats with it:gcroot_unknown_object.hprof'sPhoneWindowwent from 14 KB at the root to 3.09 MB there,large-dump.hprof's from 12 KB to 2.14 MB. WithActivity.mWindow/Dialog.mWindowowning it, every decor view in every dump is under its window and every window of a screen that is up is under that screen.The one state check
ActivityWindowRulereadsActivity.mDestroyedandDialog.mDecor, which no other rule here needs. It has to:Activity.mWindowis set inattachand never cleared, andDialog.mWindowis final, so a destroyed activity and a dismissed dialog go on pointing at a window they have nothing to do with. Owning it regardless took the window of a leaked screen away from whatever else was holding it and turned two leaks into one on the list — the reference that would still have been there after the activity was fixed stopped being reported.HeapLeaksTestcovers that, andnotes/dominator-tree.mdnow records it as the test to apply to a new rule.Conserved
Bytes conserved on all eight dumps, no object joins or leaves the tree, 0 to 16 objects of 34 K to 340 K move.
shark-explorer-core:checkandshark-explorer-app:checkpass.🤖 Generated with Claude Code