Repository navigation
Fix Android headless background fetch lifecycle - #1153
MangelSpec wants to merge 3 commits into
Conversation
|
Thanks for the PR! I'll have to split this up a bit and merge some fixes individually. |
| width: null, | ||
| ), | ||
| () { | ||
| if (!context.mounted) return; |
There was a problem hiding this comment.
I know this was a bit hacky but this is by design. Because the route can be login screen -> loading screen -> login screen when entering a wrong password, where the context would change and no snackbar would be shown if we checked that it is still mounted.
There was a problem hiding this comment.
That makes sense, and you’re right that the mounted check alone would drop the snackbar after the login route is recreated. I kinda didn't think of that.
The existing callback is still unsafe, though, because AppLocalizations.of, ScaffoldMessenger.of, and Theme.of perform ancestor lookups using the old login page context after that route has been disposed. That is the exception I reproduced and why I even wanted to fix it.
I think the proper fix is to use a root ScaffoldMessengerKey, which survives the login -> loading -> login transition and can show the snackbar on the current Scaffold without using the deactivated context.
| } | ||
|
|
||
| @override | ||
| Future<void> close() { |
There was a problem hiding this comment.
I've merged this seperatly, thanks!
| localizationsDelegates: | ||
| AppLocalizations.localizationsDelegates + | ||
| GlobalMaterialLocalizations.delegates + | ||
| AppLocalizations.localizationsDelegates + |
There was a problem hiding this comment.
Hm, this shouldn't be necessary. If I look at the code for the localizations delegate:
static const List<LocalizationsDelegate<dynamic>> localizationsDelegates =
<LocalizationsDelegate<dynamic>>[
delegate,
GlobalMaterialLocalizations.delegate,
GlobalCupertinoLocalizations.delegate,
GlobalWidgetsLocalizations.delegate,
];
It already contains material and cupertino delegates
There was a problem hiding this comment.
The generated delegates here come from package:flutter_localizations, while AppBar now comes from package:material_ui. After the package split, those MaterialLocalizations types are different.
The material_ui migration docs specifically recommend GlobalMaterialLocalizations.delegates for apps migrating from flutter_localizations. Without it, I reproduced the No MaterialLocalizations red screen on a German device, and the added widget test covers that case:
https://pub.dev/packages/material_ui/versions/1.4.0#migrating-existing-code-to-this-package
Thanks for the review. Splitting this up makes sense. Sorry, I was a bit lazy and bundled a few unrelated fixes into one PR ;) |
- Drop the localization and login changes for separate follow-ups. - Remove the duplicate auth listener cleanup now present upstream.
What was happening
Android kept starting KitchenOwl's scheduled background job while the app was closed. The captured ADB output contained the same unhandled exception 14 times:
ADB stack trace
The headless callback created an
AuthCubit, whose constructor started asynchronous setup. The task could finish and close the cubit before setup completed, so its API listener later tried to emit on the closed cubit.What changed
AuthCubit.finally.Verification
flutter test --no-pub test/background_fetch_headless_task_test.dart(3 tests passed)flutter analyze(no issues)