wix / react-native-navigation

A complete native navigation solution for React Native
https://wix.github.io/react-native-navigation/
MIT License
13.04k stars 2.67k forks source link

[Android][V1] App doesn't load when restored from minimized/background state #3011

Closed arash8m closed 6 years ago

arash8m commented 6 years ago

Issue Description

Two of my apps currently in production sometimes don't load when restored from app switcher. After some digging, I was able to reproduce it.

Steps to Reproduce / Code Snippets / Screenshots


Environment

FransTwisk commented 6 years ago

I'm experiencing the same when app is inactive in background and reopened again: the app stays on the splashscreen.

ericketts commented 6 years ago

Try this (and note that you should definitely not ship this code, because I only just found this and am totally unsure of the ramifications):

In NavigationActivity.java, look for the onDestroy() method, and comment out the call to destroyJsIfNeeded(). This should leave you with something roughly like this:

@Override
protected void onDestroy() {
    destroyLayouts();
    // destroyJsIfNeeded();
    NavigationApplication.instance.getActivityCallbacks().onActivityDestroyed(this);
    super.onDestroy();
}

Does it work for you now?

Here's my present understanding of why:

When coming from backgrounded state, the app initially starts the NavigationActivity (rather than the SplashActivity created on a cold boot), which checks whether the React context is initialized, which it isn't because we're coming from background and nothing has initialized it yet.

When it sees React isn't initialized, it starts a SplashActivity; within the onResume() of the SplashActivity, a call is made to start initialization of the React context.

However, the onDestroy() of the NavigationActivity that presented the SplashActivity is called after the onResume() starts creating the React context, and in this onDestroy() as we've seen, the React context is told to destroy.

So when the context created event starts up another NavigationActivity after the call to destroy the context, the NavigationActivity sees that the context is not created and creates another SplashActivity, which starts this whole song and dance over again...

I've not yet nailed down why this doesn't repeat ad infinitum, I think its something to do with trying to post runners to a dead thread, since I see an error about that in the console.

Sorry for the wall of text here, I've spent all day trying to debug this issue, and I wanted to get my cursory results jotted down somewhere they could be shared with others who might be afflicted by this bug!

arash8m commented 6 years ago

@ericketts thanks. Looking at what you explained, I found this https://github.com/wix/react-native-navigation/issues/1022#issuecomment-329432373 I think overriding clearHostOnActivityDestroy is a higher level solution to what you're suggesting. I tested it and it seems to solve the issue. Although I should mention the way I'm starting the app on the javascript side is the same as what guyca is suggesting. Please let me know if his solution works for you too.

ericketts commented 6 years ago

@arash8m thank you, I've actually seen that link previously, but (I think, since I don't fully remember all that I tried) even with that change made I was still experiencing the splash screen "loop" when booting with a saved background state.

@guyca can you possibly chime in if you've got a minute on what the potential ramifications would be of not destroying React context in NavigationActivity#onDestroy()?

guyca commented 6 years ago

Hey guys. In older RN versions (0.2x) react context was bound to the activity, therefore it was cleared when the activity was destroyed. Later on it was decoupled from the activity, and handled solely by the Application. You're actually not supposed to clear the host at all, we've stopped doing so in the Wix app for quite some time now and are not clearing it in v2 as well.

peterp commented 6 years ago

To clarify this means that we should definitely return false, and use the events, as documented here

ericketts commented 6 years ago

@peterp As noted elsewhere, the changes at your posted link cause exiting the app via the back button to cease functioning correctly;

to "fix" that, I forked & replaced NavigationActivity#invokeDefaultOnBackPressed with the following code (shared in case anyone else needs the back button to not render their app useless until a hard restart):

@Override
public void invokeDefaultOnBackPressed() {
  if (layout == null) {
    if (!NavigationApplication.instance.clearHostOnActivityDestroy()) {
      this.moveTaskToBack(true);
    }
    return;
  }
  if (layout.onBackPressed()) {
    return;
  }
  if (!NavigationApplication.instance.clearHostOnActivityDestroy()) {
    this.moveTaskToBack(true);
    return;
  }
  super.onBackPressed();
}

This moves the app to the background without finishing the NavigationActivity, assuming you've overridden the your MainApplication#clearHostOnActivityDestroy to return false.

(As an aside, I sort of feel like the if on line 4 should have an else block that does the super call, but this is written to behave as the code it was replacing did, where if the layout was null, no super call).

xvonabur commented 6 years ago

@guyca can you approve @ericketts solution? @ericketts can you send a PR?

ericketts commented 6 years ago

@xvonabur I'm not sure that this behavior is necessarily something that the Wix folks would want to merge into the master branch, since the default (and therefor expected) behavior of the back button when there's no activity stack is to finish the root activity and close the app.

That said, if @guyca thinks this is something that would make sense as a behavior and is likely to get merged, I'd be happy to put together a PR for it.

Even if the activity started up again correctly, by moving it to the back instead of finishing it, you get a far faster start time the next time you go to use the app (since its already created and on the stack, assuming the OS doesn't kill it in the interim period).

As a humorous aside, I'm well on my way to becoming a react-native-navigation expert, if wix is hiring... 😁

xvonabur commented 6 years ago

@ericketts I totally agree, that default back behaviour is close an app. But without this "fix" I see white screen after pressing back button and reopening an app. App just doesn't load.

SuperKirik commented 6 years ago

@guyca, my application need in this solution https://github.com/wix/react-native-navigation/issues/1022#issuecomment-329432373 but it in turn causes the same troubles as @xvonabur has. @ericketts's workaround handle with this.

So shouldn't it be merged if clearHostOnActivityDestroy should definitely return false ? Or there's some other workaround with back button troubles ?

davelnewton commented 6 years ago

FWIW, I ended up not needing any of these solutions. I ended up blowing away node_modules for an unrelated issue (thanks, misuse of .yarnclean), checking a couple of version numbers, and things started working again.

For reference:

  "dependencies": {
    "react": "16.3.1",
    "react-native": "~0.55.4",
    "react-native-navigation": "^1.1.466"
  }

Noting that trying 16.4.0 was a no-go at this time.

xvonabur commented 6 years ago

@davelnewton Thanks for the 16.4.0 part. I didn't try it yet. But why are you using two navigation library in your project? react-navigation and react-native-navigation

davelnewton commented 6 years ago

@xvonabur I'm not; it was a remnant pre-switch to react-native-navigation :)

hugows commented 6 years ago

So is there any fix?

xvonabur commented 6 years ago

@hugows Fix requires editing source code of react-native-navigation. Unfortunately v1 is freezed and team doesn't accept any pull requests for it. v2 still misses some features from v1, so you have 2 options:

  1. Use my (or somebody else) fork of react-native-navigation;
  2. Make your own fork with @ericketts changes and use it instead.
GioLogist commented 5 years ago

Still experiencing this issue w/v2

GioLogist commented 5 years ago

Update: My issue was due to not wrapping the app in registerAppLaunchedListener as mentioned here 🤪

vipu-sa commented 3 years ago

Android - The previous state of stripe payment screen not getting When app reopens click of app icon after minimizing.

when I am performing 3d secure payment using stripe payment screen.When I minimize the application in between this process and, reopen it by clicking on the app icon at that time I am not getting the previous state of stripe payment screen specific for android. In ios, it is working.