ReactContext: currentActivity is null when onActivityResult is called

[fab1an]

I use addActivityEventListener to listen for the result of an Intent.

1. when I start the Intent RN calls onHostPause and releases the reference mCurrentActivity in ReactContext.java.
2. my onActivityResult fires, but there is no currentActivity set yet, because onHostResume is called afterwards.

0.29 / Android / Mac

[charpeni]

Can you provide some code?

[fab1an]

Yes, but it's in Kotlin, do you mind?

[charpeni]

I'm not familiar with Kotlin, but for other people who will check this that will be better than nothing I guess.

[fab1an]

Stripped version:

package at.feibra.mobileqm

class ControllerModule(ctx: ReactApplicationContext) : ReactContextBaseJavaModule(ctx) {

    // ~ Constructors ----------------------------------------------------------------------------------

    init {
        /* forward activity event to controller */
        ctx.addActivityEventListener { requestCode, resultCode, data ->
            L.info("activity result: requestCode: {}, resultCode: {}, data: {}", requestCode, resultCode, data);
        }

        /* forward resume, pause, destroy to controller */
        ctx.addLifecycleEventListener(object : LifecycleEventListener {
            override fun onHostResume() {
                L.info("host resumed")
            }

            override fun onHostPause() {
                L.info("host paused")
            }

            override fun onHostDestroy() {
            }
        })
    }
}

[fab1an]

This line: https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java#L157

is called AFTER this: https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java#L208

[satya164]

I can confirm this that I've run into this issue.

cc @foghina

[foghina]

Yeah, this is how Android works, unfortunately. If you have a plain Activity you will see the same thing: onActivityResult is called before onResume. We're not going to make hacks to work around this at the platform level (since we want to mimic Android as much as possible), but feel free to work around it yourself by e.g. storing the result and dealing with it in onHostResume. Even better, you can handle both cases easily, like this:

@Override
public void onHostResume() {
  if (mActivityResult != null) {
    // getCurrentActivity() will never return null here
    handleActivityResult(getCurrentActivity(), mActivityResult);
    mActivityResult = null;
  }
}

@Override
public void onActivityResult(int requestCode, int resultCode, Intent data) {
  final Activity activity = getCurrentActivity();
  if (activity != null) {
    handleActivityResult(activity, data);
  } else {
    mActivityResult = data;
  }
}

private void handleActivityResult(Activity activity, Intent result) {
  // insert business logic
}

I'd write this in Kotlin but I'm pretty rusty 🙃

[fab1an]

I've worked around it like that already.

I still think this needs fixing: Usually, without RN, people would override onActivityResult and have their Activity around using this.

Please reopen.

[fab1an]

@foghina Please reopen this issue. You said RN wants to mimic Android as much as possible. In a regular Android application you have access to your Activity on onActivityResult. In RN you don't…

[foghina]

Fine. Feel free to send a PR that e.g. adds an Activity arg to onActivityResult().

[fab1an]

@foghina Awesome thanks. I can do that, but I noticed another problem which might be related. I worked around like you said by storing the result and waiting for the Activity to return.

Now I tested my app for low-memory using "Don't keep activities". When using that onActivityResult is actually never called at all, because apparently the Host is destroyed?

[foghina]

Ah yes, the problem there is the ReactInstanceManager gets destroyed when the Activity is destroyed. I might fix that soon anyway as part of a different thing I'm working on.

[fab1an]

@foghina Ok. I would help, but I don't know what the "structural plan" is. One ReactInstanceManager and one ReactNativeHost per Android Application, is that right? The ReactRootView is destroyed and recreated along with Activities?

Is the actual JS structure (the component-tree, the callbacks) tied to the ReactRootView too or can it be reused across Activities?

[fab1an]

Has this be changed with 0.31? Thanks!

[fab1an]

Any idea when this will be fixed?

[foghina]

This should be fixed by 3c4fd4.

However, note that if the activity returning a result is also a RN activity, it will be returned by getCurrentReactActivity() during onActivityResult(), instead of the activity receiving the result.

If the activity returning the result is not RN (e.g. camera app), current activity will be correct.

Also, running with don't keep activities is probably still broken.

Working on a fix for these two issues.

[fab1an]

@foghina So does this work now when not keeping activities? If not, should I open a separate bug-report?

[foghina]

It should work, I've tested a bunch of scenarios but not onActivityResult specifically. Feel free to try it out (e.g. using master) and open a new bugreport if it still doesn't work (and cc me).

[fab1an]

@foghina I'll test it when it's released (in 0.32?)

[foghina]

Probably 0.33-rc and then 0.33, not sure though. @bestander?

[bestander]

0.33-rc.0 next Monday unless this is critical to get as a patch into 0.32.0.

[fab1an]

@bestander It is not critical to me, but, since Android is allowed to kill Activities any time, this can lead to curious bugs for inexperienced developers, which in turn creates new issues for this already overwhelmed project…

[bestander]

@fab1an, 0.32 becomes stable early next week and 0.33-rc is released.
This issue has been there for a while so I don't think we should rush and risk 0.32 stable and release it without a good 2 weeks of release-candidate testing.
But thanks for caring and feel free to discuss more.

[conover]

This is actually a big deal. This patch seems to fix a whole class of issues for us. We will be testing .33-rc over the weekend.

[fab1an]

@conover Did it fix your issues?

[fab1an]

I can confirm this fixed as of version 0.34, I did not try 0.33.

[gold-duo]

I hava the same issues. android version 0.36
when i used getCurrentActivity in ReactContextBaseJavaModule some times return null.
I create ReactInstanceManager not in onCreate method，But in the onCreate after a period of time。
So there is no onHostResume method is called, passing the current activity to ReactContext.
My solution is to create a ReactInstanceManager called after ReactInstanceManager.onHostResume.