Skip to content

fix: scope pendingResult to the call that defers it - #188

Open
MB-Hilo wants to merge 1 commit into
X-SLAYER:mainfrom
MB-Hilo:fix/pending-result-lifecycle
Open

MB-Hilo wants to merge 1 commit into
X-SLAYER:mainfrom
MB-Hilo:fix/pending-result-lifecycle

Conversation

@MB-Hilo

@MB-Hilo MB-Hilo commented Aug 15, 2026

Copy link
Copy Markdown

Follows up #187, which asked for a release containing the addActivityResultListener fix already on main. These are the two smaller pendingResult problems I mentioned there — both still present on main, and independent of that release.

1. pendingResult was claimed by every call

public void onMethodCall(@NonNull MethodCall call, @NonNull Result result) {
    pendingResult = result;   // every method, not just the one that defers

requestPermission is the only branch that defers its Result — it starts the settings intent and lets onActivityResult complete it later. Every other branch completes its own Result inline and has no use for the field.

So any call arriving while the settings screen is open — isOverlayActive, getOverlayPosition, moveOverlay — overwrites pendingResult. When the user returns, onActivityResult completes that call with a permission boolean, and the original requestPermission future never completes.

This is easy to hit in practice: apps commonly poll isOverlayActive() from a lifecycle observer, which fires exactly when the user comes back from the settings screen.

Fixed by assigning pendingResult only in the branch that defers.

2. onActivityResult dereferenced it unguarded

if (requestCode == REQUEST_CODE_FOR_OVERLAY_PERMISSION) {
    pendingResult.success(checkOverlayPermission());   // NPE when null

An activity result can arrive with nothing pending — most plausibly when the activity is recreated while the settings screen is up, so the plugin re-attaches with a fresh pendingResult of null. Added a null check, and cleared the field after use so the same Result cannot be completed twice.

I kept the existing behaviour of reporting checkOverlayPermission() rather than resultCode, and added a comment for why — ACTION_MANAGE_OVERLAY_PERMISSION returns RESULT_CANCELED even when the user grants, so resultCode would be wrong here.

Notes

  • One file, two hunks, no API or behaviour change for correct callers — a requestPermission() that already worked still resolves the same way.

  • Not fixed here, to keep the diff focused, but noticed nearby in onMethodCall:

    • closeOverlay returns without completing its Result when OverlayService.isRunning is false, so that call hangs.
    • The isOverlayActive branch is duplicated — the second is unreachable.

    Happy to send either as a separate PR if you'd like.

Tested against a real app on Android 16 (API 36), Pixel 9 Pro: revoke the overlay permission, request it, grant, return — the future resolves and the overlay starts without an app restart.

pendingResult was assigned on every onMethodCall. requestPermission is the
only branch that defers its Result to onActivityResult, so any call landing
between the intent launch and the activity result — isOverlayActive,
getOverlayPosition, moveOverlay — overwrote it. onActivityResult would then
complete that unrelated call with a permission boolean, and the original
requestPermission Future would never complete.

Assign pendingResult only in the branch that defers, and null-guard
onActivityResult so an unpaired activity result (e.g. the activity is
recreated while the settings screen is up) no longer throws an NPE. Clear
the field after use so it cannot be completed twice.

Checking the live permission state rather than resultCode is preserved and
now documented: ACTION_MANAGE_OVERLAY_PERMISSION returns RESULT_CANCELED
even when the user grants.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants