fix: [SDK-4736] move Flutter bridge off main thread#1155
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@claude review |
There was a problem hiding this comment.
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:
-
Android bridge threading (SDK code) — Adds a static single-thread
BACKGROUND_EXECUTORinFlutterMessengerResponderand routesonMethodCallthrough it forOneSignalPlugin,OneSignalInAppMessages,OneSignalLocation,OneSignalPushSubscription,OneSignalSession,OneSignalUser, and most ofOneSignalNotifications.OneSignalNotificationskeeps three cache-only paths (displayNotification,preventDefault,proceedWithWillDisplay) synchronous to preserve foreground notification timing. Cache maps are switched toConcurrentHashMapandclickListenerRequestedtovolatile. -
Demo iOS scene-based lifecycle migration —
AppDelegatenow conforms toFlutterImplicitEngineDelegatewith plugin registration moved todidInitializeImplicitFlutterEngine;Info.plistadds aUIApplicationSceneManifestwithFlutterSceneDelegate;AppFrameworkInfo.plistdrops the explicitMinimumOSVersion. Demogradle.propertiesalso 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.
Co-authored-by: Cursor <cursoragent@cursor.com>
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
Testing
flutter analyzedart run rps format-checkdart run rps test(257 tests, 98.6% coverage)flutter build apk --debugfromexamples/demoAndroid_16andiPhone 17with fresh app installs and notification permissions accepted:waitForInit,runBlocking, fatal exception, or ANR signal under the Flutter plugin path.Affected Code Checklist
Checklist
main.Made with Cursor