[in_app_purchases] Android: make sure properly disconnect the billing client object. #2509
Conversation
cyanglaz
changed the title
| @@ -81,7 +85,7 @@ public void onReattachedToActivityForConfigChanges(ActivityPluginBinding binding | |||
|
|
|||
| @Override | |||
| public void onDetachedFromActivityForConfigChanges() { | |||
| onDetachedFromActivity(); | |||
| methodCallHandler.setActivity(null); | |||
There was a problem hiding this comment.
Why call this and not methodCallHandler.onDetachedFromActivity()?
There was a problem hiding this comment.
I think it is not necessary to disconnect the billing client when activity detached from config changes (e.g. when the phone is rotated).
There was a problem hiding this comment.
Makes sense, could you also document that here? Everywhere else we treat these callbacks as equivalent.
| @Override | ||
| public void onActivityDestroyed(Activity activity) { | ||
| if (this.activity == activity) { | ||
| if (this.applicationContext != null) { |
There was a problem hiding this comment.
style nit: I would combine this with the outer if just to avoid an extra level of indentation.
|
Whoops, I forgot about tests. Please add in a unit test before merging. |
|
@mklim Updated with tests |
| @@ -81,7 +85,7 @@ public void onReattachedToActivityForConfigChanges(ActivityPluginBinding binding | |||
|
|
|||
| @Override | |||
| public void onDetachedFromActivityForConfigChanges() { | |||
| onDetachedFromActivity(); | |||
| methodCallHandler.setActivity(null); | |||
There was a problem hiding this comment.
Makes sense, could you also document that here? Everywhere else we treat these callbacks as equivalent.
| @@ -66,6 +69,37 @@ void setActivity(@Nullable Activity activity) { | |||
| this.activity = activity; | |||
| } | |||
|
|
|||
| // Activity Life Cycle callbacks | |||
There was a problem hiding this comment.
nit: I think this comment is a little redundant. If this is more of an organizational thing I'd rather put the callbacks in an inner final class and reference that instead.
Description
Fixes a bug that the plugin disconnect the billing client at an inappropriate place. Also fixed some minor compiling warnings.
Related Issues
flutter/flutter#33210
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?