Fixed image picker crash when used with android alarm manager #1125
Conversation
|
test? |
|
|
| @@ -21,6 +21,9 @@ | |||
| private final ImagePickerDelegate delegate; | |||
|
|
|||
| public static void registerWith(PluginRegistry.Registrar registrar) { | |||
| if (registrar.activity() == null) { | |||
There was a problem hiding this comment.
This will prevent the crash but IIUC leaves us with an unregistered plugin, as even if the activity asks the GeneratedPluginRegistrant to register again it will short circuit.
Unless I'm missing something this will trade the startup crash with a crash the first time the plugin is used (as the method channel isn't registered).
I believe the only eager use of activity is getExternalFilesDir and this can be done with the application context which is guaranteed to be available.
It looks like ImagePickerDelegate is caching the activity that's being passed to it from here, I believe instead of doing that we can make ImagePickerDelegate always ask the activity from the registrar when it needs an activity.
@cyanglaz @Hixie am I getting this wrong? is the current fix actually leaving the picker in a working state? are you interested in merging this for now just to prevent the early crash or doing a little more refactoring here?
There was a problem hiding this comment.
I don't know how to test if the picker was in a working state with this fix -- the problem I was seeing was that when the app was starting in the background (for the alarm manager), nothing was working, because the plugin would crash and prevent anything else from happening. In that case, I don't actually care about the image picker.
I've no idea if it's possible for the app to then get re-used in the foreground. If so, then I agree that this wouldn't work.
There was a problem hiding this comment.
I see your point for the scenario that the app is executed in the background and then quits (and the Android APP is destroyed) it's ok to live with the plugin unregistered. So this fix at least improves some cases, we can land it now. We should probably follow-up and figure what happens if e.g the app's background code is calling startActivity and starts an activity of the same (Android) application with a FlutterView.
After an offline discussion, we verified that the registering process that crashed the image picker came from a background flutter view that is used by the Android Alarm Manager. And a foreground registration will happen with a different registry. Image Picker only needs the foreground registry so the fix should not cause any issue with the Image Picker. |
…1125) Fixing [flutter/flutter#21972](flutter/flutter#21972) Android Alarm Manager will create a background flutter view to register plugins. When that happens, image_picker will be registered without an activity, which then caused crash. So we escape the registration process when there is no activity in the registrar.
…lutter#1125)" This reverts commit f45ca68.
…1125) Fixing [flutter/flutter#21972](flutter/flutter#21972) Android Alarm Manager will create a background flutter view to register plugins. When that happens, image_picker will be registered without an activity, which then caused crash. So we escape the registration process when there is no activity in the registrar.
…1125) Fixing [flutter/flutter#21972](flutter/flutter#21972) Android Alarm Manager will create a background flutter view to register plugins. When that happens, image_picker will be registered without an activity, which then caused crash. So we escape the registration process when there is no activity in the registrar.
Background FlutterViews do not have an activity, the Maps and Camera plugins are foreground only and assumed and activity is available which can result in a crash when the plugin is registered by a background FlutterView. We workaround by just not registering the plugins if there is no activity. Similar to flutter#1125
#1255) Background FlutterViews do not have an activity, the Maps and Camera plugins are foreground only and assumed and activity is available which can result in a crash when the plugin is registered by a background FlutterView. We workaround by just not registering the plugins if there is no activity. Similar to #1125
…. (#1255) Background FlutterViews do not have an activity, the Maps and Camera plugins are foreground only and assumed and activity is available which can result in a crash when the plugin is registered by a background FlutterView. We workaround by just not registering the plugins if there is no activity. Similar to flutter/plugins#1125
Fixing flutter/flutter#21972