fix(android): improve home press reliability#2
Merged
Conversation
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
Fix the Android race where the privacy cover would sometimes miss the Recents thumbnail, the underlying app content leaked through intermittently, especially under load or with a
<Modal>open. Replaces the racyview.alphatoggle with aSurfaceControlViewHostbacked cover that toggles visibility via a thread-safeSurfaceControl.Transactiondirectly from the broadcast HandlerThread, and proactively re-parents the cover when a Modal Dialog opens so the very first Home press behind a modal also wins.Type of change
Scope
ios/HybridCover.swiftand friends)android/src/main/java/com/margelo/nitro/cover/HybridCover.ktand friends)src/specs/cover.nitro.ts) — regenerate withnpm run specsexample/).maestro/flows/)README.md)Checklist
npm run typecheckand it passesnpm run specsafter editing any*.nitro.tsfile and committed the regeneratednitrogen/generated/files (no spec changes)<Modal>open; 10+ background cycles per mode)How to test
Then in the example app:
Add icon as overlay(image mode) — same test, Recents should show indigo + the icon overlay.Optional diagnostics (recommended on slower devices to confirm the fast path engaged):
adb logcat -c adb logcat -s Cover:I Cover:W -v timeExpected lines on a clean run:
attachCover scvh: attached size=…x… visible=false sc=ok— SCVH path engaged on API 30+broadcast: fast scvh=true refl=false dt=0-2ms—SurfaceControl.Transactionfired from the broadcast HandlerThread before main even sees the eventensureCoverOnTopmost: reparent (topmost=DecorView)— fires when a Modal Dialog opens and we proactively re-parent the coverRelated issues
Closes #1
What was wrong (for reviewers / future readers)
The cover was being mounted from
ACTION_CLOSE_SYSTEM_DIALOGS(home/recents/assist broadcast) viaview.alpha = 1on a pre-mounted FrameLayout. That toggle has to:ViewRootImpltraversal → render → buffer queued to SurfaceFlinger,That's ~2 vsyncs (≈33 ms at 60 Hz) of latency, and on some devices the OS captures the Recents thumbnail within a single vsync of the user-leave signal (we measured 7–18 ms between broadcast and
onPauseon the affected device). The toggle finishes too late, the snapshot grabs the alpha=0 frame, the cover is "missing".Compounding it: when a React Native
<Modal>was open, the cover (a sub-window of activity main) was z-ordered below the modal at compose time, so even on devices where the alpha toggle did land in time, the modal would occlude it on the first leave. The reactive re-attach insideaddCoverwas correct logic, but landed after the snapshot was already taken.What the fix does
1.
SurfaceControlViewHost+ directSurfaceControl.TransactionOn API 30+, the cover content's
FrameLayout(color / image / blur — unchanged rendering logic,RenderEffectblur still works) is hosted inside aSurfaceControlViewHostthat we own. The cover Window's root is now aSurfaceViewthat reparents the SCVH'sSurfacePackage. The SCVH'sSurfaceControlis captured at attach time.When the leave broadcast fires, the receiver, running on its own
HandlerThread, does:apply()is thread-safe, applied atomically at the next SurfaceFlinger compose. No main-thread hop, noViewRootImpltraversal, no buffer re-render. The buffer was rendered once at attach whenenable()ran, withview.alpha = 1pinned. Visibility is now a pure SurfaceFlinger-level alpha multiplier on a pre-rendered layer. The pipeline shrinks from ~33 ms to ~16 ms (one compose vsync), which fits inside the observed snapshot window.(Animated
show()/hide()use aValueAnimator+ per-frameTransaction.setAlpha; ~9 binders over a 150 ms fade.)The older
view.alphapath is kept as a fallback for API <30 and for the case where SCVH creation fails. Strictly a fast-path addition, no regression.2. Blur capture must skip the cover's own
SurfaceViewCoverBlurRenderer.rendercallsCoverWindowAttachment.topmostHostViewFor(activity, exclude = target.rootView)to pick the source view to capture-and-blur. With SCVH,target.rootViewis the SCVH-internalFrameLayoutwhich is not inWindowManagerGlobal.mViews, but our wrappingSurfaceViewIS. The exclude no longer matched anything from the cover, the top-most view found was our ownSurfaceView, and software-drawing aSurfaceViewproduces a transparent bitmap (its content lives in a separate hardware surface). Result: the blur ImageView got an empty source →RenderEffectblurred nothing → only the tint showed up → app content was readable straight through the "blur".Fix:
topmostHostViewFornow accepts a secondexclude2parameter,CoverBlurRenderer.rendertakes analsoExclude: View?, andrefreshBlurIfActivepassescoverView(theSurfaceViewon SCVH path; same astarget.rootViewon legacy path, harmless duplicate).3. Proactive modal-token reparenting
The pre-mounted cover lives on the activity's main window token. When a
<Modal>opens, RN creates a separate top-levelDialogwindow with its own token; the cover (a sub-window of activity main) ends up z-ordered below the modal in the SurfaceFlinger composition. The first leave broadcast fires while the cover is still on the wrong token, alpha=1 is applied correctly, but the modal layer occludes it. By the timeaddCoverruns on main and re-attaches the cover to the modal's token, the snapshot is already captured.Fix: install a
ViewTreeObserver.OnWindowFocusChangeListeneron the activity's decor view at pre-mount time. Whenever focus flips on the activity window (which happens when a Modal opens, closes, or any other top-level view in our process steals focus), callensureCoverOnTopmost():This pre-reparents the (still invisible) cover onto whichever window is currently on top. By the time the user actually backgrounds, the cover is already above the modal. The broadcast's fast SCVH alpha-toggle lands on a layer that's already in front of everything. First home-press with a modal open now works.
Filter-safe:
topmostHostViewForonly readsWindowManagerGlobal.mViews(our process's view list). System dialogs from other processes (permission prompts, notification shade) don't show up there, so they don't trigger a re-attach.Smaller helpers also in this change
pendingUserLeaveMountflag + synchronous mount insideonActivityPaused— closes the secondary race where the broadcast'spostAtFrontOfQueueis stuck behind a long-running main-thread message.SurfaceControlAccessreflection helper best-effort attempt to grab the legacyViewRootImpl.mSurfaceControlso the same fast-path technique can engage on API 29 (no SCVH) and on devices that allow the reflection. No-ops gracefully on devices with strict hidden-API enforcement.params.preferredRefreshRate = display.supportedModes.max(refreshRate)on the cover window — asks the OS to drive the display at its highest rate while the cover is attached, shrinking vsync interval where supported.Log.i/Log.wmarkers taggedCoverat every entry point on the leave path, withSystemClock.uptimeMillis()timestamps — makes future device-specific regressions diagnosable from a singleadb logcat -s Cover:I Cover:W.