Skip to content

fix: [SDK-4736] move Flutter bridge off main thread#1155

Merged
fadi-george merged 5 commits into
mainfrom
fadi/sdk-4736
Jun 9, 2026
Merged

fix: [SDK-4736] move Flutter bridge off main thread#1155
fadi-george merged 5 commits into
mainfrom
fadi/sdk-4736

Conversation

@fadi-george

Copy link
Copy Markdown
Collaborator

One Line Summary

Move Android Flutter bridge calls that touch OneSignal SDK accessors off the main thread to avoid startup ANRs while preserving notification response timing.

Motivation

Android MethodChannel handlers run on the platform thread, and OneSignal SDK accessors can block in waitForInit() during initialization. Moving SDK-facing bridge work to a shared background executor prevents that wait from blocking the UI thread.

Scope

  • Adds a shared serial background executor for Android bridge handlers that call into the native SDK.
  • Keeps cache-only notification response paths immediate so foreground notification handling is not queued behind init-sensitive work.
  • Makes notification event caches and click listener state safe across bridge/lifecycle threads.
  • Includes committed demo compatibility updates needed by the branch.

Testing

  • flutter analyze
  • dart run rps format-check
  • dart run rps test (257 tests, 98.6% coverage)
  • flutter build apk --debug from examples/demo
  • Manual verifier run on Android_16 and iPhone 17 with fresh app installs and notification permissions accepted:
    • Android permission changed to true and push subscription updated to optedIn=true / SUBSCRIBED.
    • iOS permission changed to true and push subscription updated to optedIn=true.
    • No Android waitForInit, runBlocking, fatal exception, or ANR signal under the Flutter plugin path.

Affected Code Checklist

  • Android native bridge
  • Flutter example app
  • Dart public API
  • iOS SDK bridge

Checklist

  • Branch targets main.
  • Unrelated untracked verifier script is not included in the PR.
  • Manual permission prompt regression testing completed on emulator/simulator.

Made with Cursor

@fadi-george
fadi-george requested a review from a team as a code owner June 5, 2026 00:55
@fadi-george

Copy link
Copy Markdown
Collaborator Author

@claude review

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find any bugs, but this is a substantive threading change that re-routes every Android Dart→Native call through a shared background executor and also migrates the demo iOS app to the scene-based lifecycle — worth a human pass.

Extended reasoning...

Overview

This PR has two distinct sets of changes:

  1. Android bridge threading (SDK code) — Adds a static single-thread BACKGROUND_EXECUTOR in FlutterMessengerResponder and routes onMethodCall through it for OneSignalPlugin, OneSignalInAppMessages, OneSignalLocation, OneSignalPushSubscription, OneSignalSession, OneSignalUser, and most of OneSignalNotifications. OneSignalNotifications keeps three cache-only paths (displayNotification, preventDefault, proceedWithWillDisplay) synchronous to preserve foreground notification timing. Cache maps are switched to ConcurrentHashMap and clickListenerRequested to volatile.

  2. Demo iOS scene-based lifecycle migrationAppDelegate now conforms to FlutterImplicitEngineDelegate with plugin registration moved to didInitializeImplicitFlutterEngine; Info.plist adds a UIApplicationSceneManifest with FlutterSceneDelegate; AppFrameworkInfo.plist drops the explicit MinimumOSVersion. Demo gradle.properties also adds Flutter-migrator compat flags.

Security risks

Nothing security-sensitive (no auth, crypto, network, or permission changes in the SDK code). The threading change does not affect what is sent to or received from native; it only changes which thread invokes the SDK accessors.

Level of scrutiny

Higher than a typical mechanical refactor. Every Dart→Native bridge call on Android now hops to a background thread instead of running synchronously on the platform thread, which is a semantic change for ordering relative to the platform thread and to native SDK callbacks. The single-thread executor preserves intra-bridge call ordering, and the carve-outs for the willDisplay cache paths look right (they read/write the now-concurrent caches without touching init-sensitive accessors). Still, threading changes in shared bridge code are the kind of thing where regressions surface only under real-world timing, and a human reviewer who knows the Flutter plugin lifecycle on this codebase is better positioned to sign off than I am.

Other factors

  • Bug hunting system found no bugs.
  • PR description notes manual ANR/permission testing on Android 16 and iPhone 17, plus the existing unit suite (257 tests).
  • The demo iOS scene migration is example-app only, so blast radius for that piece is limited to the example, but it's still a non-trivial Flutter version compatibility change worth a glance.
  • Builds on top of recently merged #1138 and #1149 changes touching the same files; a reviewer familiar with that thread will catch interaction issues faster than I can.

@fadi-george
fadi-george merged commit bc45b8c into main Jun 9, 2026
4 checks passed
@fadi-george
fadi-george deleted the fadi/sdk-4736 branch June 9, 2026 22:53
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