Use ActivityAware to get the Activity context

[nan-li]

Description

One Line Summary

Implement the Flutter ActivityAware interface to pass the Activity context instead of Application context to setAppId.

Details

Motivation

Fixes #509. What we were experiencing was that on the first run of the app, the current activity would be null so that IAMs don't display and the app is sensed as in the background even when in the foreground. Backgrounding the app and returning resumed normal behavior.

Why? After the changes to support Android V2 embedding, the plugin is set up in onAttachedToEngine and the Application Context is not an instance of Activity.

When this context is eventually passed to setAppId, we end up with the current activity is null which leads to some problems. Note that this is only a problem on the first run of the app.

This PR implements the ActivityAware interface and in onAttachedToActivity, updates the existing context to the activity received, following some Flutter documentation here.

In addition, use activeContext() instead of context() for the registrar for Flutter's v1 embedding to get the application context. Doing some testing shows the former provides the MainActivity while the latter provides FlutterApplication.

Scope

During testing, onAttachedToActivity is called right after onAttachedToEngine so we expect to get the activity context immediately.

There are 3 other ActivityAware methods are not implemented as they don't appear to be needed. We only need the first call to Flutter's onAttachedToActivity during the first run of the app. After that, the OneSignal ActivityLifecycleHandler should be triggered and set the activities.

Testing

Unit testing

N/A

Manual testing

Tested on Pixel 5 emulator on API 30 and checking logs for onAttachedToEngine and onAttachedToActivity and what context is passed to setAppId. Also put logging for the other 3 methods in ActivityAware and they are never logged.

• Fresh install of the app
• Backgrounding the app and returning
• Kill the app and reopen
• ❗ Not tested: navigating between multiple activities within the app, the example app is only 1 screen.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself, ensuring it meets each checklist item

---

This change is 

[jkasten2]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @emawby, @Jeasmine, @nan-li, and @tanaynigam)

---

android/src/main/java/com/onesignal/flutter/OneSignalPlugin.java, line 109 at r1 (raw file):

  public void onAttachedToActivity(@NonNull ActivityPluginBinding binding) {
    // onAttachedToEngine is called first that sets the BinaryMessenger so we can pass null here.
    init(

onAttachedToEngine already calls init, adding this means are will be calling init more than once. Some of the logic such as channel = new MethodChannel(this.messenger, "OneSignal"); doesn't look like it should be called more than once in there.

Some options the come to mind however are check if init is being called more than once, or to split up this context gathering and calling OneSignal.initWithContext.

[nan-li]

android/src/main/java/com/onesignal/flutter/OneSignalPlugin.java, line 109 at r1 (raw file):

  public void onAttachedToActivity(@NonNull ActivityPluginBinding binding) {
    // onAttachedToEngine is called first that sets the BinaryMessenger so we can pass null here.
    init(

onAttachedToEngine already calls init, adding this means are will be calling init more than once. Some of the logic such as channel = new MethodChannel(this.messenger, "OneSignal"); doesn't look like it should be called more than once in there.

Some options the come to mind however are check if init is being called more than once, or to split up this context gathering and calling OneSignal.initWithContext.

I think it suffices to update this.context with the application context when we receive onAttachedToActivity. As far as I can tell, the this.context is only ever used by initWithContext() which should happen later. In some simple testing, onAttachedToActivity is called immediately after onAttachedToEngine.

[jkasten2]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @emawby, @Jeasmine, and @tanaynigam)

---

android/src/main/java/com/onesignal/flutter/OneSignalPlugin.java, line 109 at r1 (raw file):

Previously, nan-li (Nan) wrote…

I think it suffices to update this.context with the application context when we receive onAttachedToActivity. As far as I can tell, the this.context is only ever used by initWithContext() which should happen later. In some simple testing, onAttachedToActivity is called immediately after onAttachedToEngine.

Good idea, and is less code too.

[westy92]

Should our Activity reference be cleared here?

[nan-li]

Thanks for your input @westy92,
Since we only need to get the activity on initialization and use the activity only for setAppId, we don't to track it after.

[tanaynigam]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @emawby, @Jeasmine, and @nan-li)