bluelinelabs / Conductor

A small, yet full-featured framework that allows building View-based Android applications
Apache License 2.0
3.9k stars 343 forks source link

onRestoreInstanceState never called #71

Closed sdrygalski closed 8 years ago

sdrygalski commented 8 years ago

I tested this on my personal project and also on demo project - in both cases I didn't get onRestoreInstanceState called, even once, no matter what I try to save in onSaveInstanceState.

sdrygalski commented 8 years ago

What I found in the guts of Controller is only newInstance call controller.restoreInstanceState(bundle). When I rotate emulator I got different path: setRouter and than immediately performOnRestoreInstanceState - without the chance to properly load mSavedInstanceState from previous Bundle. So when mSavedInstanceState is always null performOnRestoreInstanceState will never call onRestoreInstanceState. Hope it helps. :)

EricKuck commented 8 years ago

It sounds like this is working properly. onRestoreInstanceState is called when your controller has been removed from memory and needs to be restored to its previous state. In your tests this likely never happens. Do you have reason to believe the controller is being destroyed and not properly restored?

adipascu commented 8 years ago

You can not "force a save" , a save is caused by android when it thinks it is required, this mostly happens when the app is going to the background (the decision process is much more complex and differs in android versions).

Can you find a scenario where the containing activity runs onSaveInstanceState but the controller doesn't run onSaveInstanceState? They should run together. Edit: Ignore the last part, I am wrong. Just listen to @EricKuck

sdrygalski commented 8 years ago

Maybe I don't understand something, but in your demo in TargetDisplayController when onRestoreInstanceState should be called? When device is rotated, on configuration change? When I rotate device onRestoreInstanceState is not called.

sdrygalski commented 8 years ago

But onSaveInstanceState is called on every rotation.

sdrygalski commented 8 years ago

Basically I just want to do something like this here: https://github.com/h6ah4i/android-advancedrecyclerview/blob/122fbd6261c95cde70a5c65eeaa686a6b9de5a48/example/src/main/java/com/h6ah4i/android/example/advrecyclerview/demo_e_basic/ExpandableExampleFragment.java

When rotating device, use onSaveInstanceState, save some important data and after rotation read them in onRestoreInstanceState.

Because according to diagrams onSaveInstanceState is called after onDestroyView I use onSaveViewState to save data to field and use it later in onSaveInstanceState. That way I can restore data before onCreateView - in onRestoreInstanceState.

I'm sorry if something is not clear, it's 3:43 am here. ;)

EricKuck commented 8 years ago

If onRestoreInstanceState is not called, that's because your controller still exists. There would be no reason to restore a controller that still exists and has all its data. If you look at the diagram again, you'll see there is a line connecting onDestroyView() to onCreateView() for cases where the app process isn't killed, which is exactly what's happening for you.

sdrygalski commented 8 years ago

So I have few questions:

  1. What must happen to onRestoreInstanceState be called in your demo in TargetDisplayController?
  2. Is rotation enough to make it called? (according to diagrams - no, it's not enough)
  3. So what is equivalent to accessing bundle when inflating views, like accessing savedInstanceState in Fragments onCreateView and onViewCreated? Do I need to wait for onRestoreViewState?

If I need to wait for onRestoreViewState I think it's less convenient than in Fragments, when I got access to bundle in onCreateView and onViewCreated.

EricKuck commented 8 years ago
  1. The app process needs to be killed, as it shows in the diagram.
  2. No.
  3. Why do you want to access a bundle when inflating views? There's no good reason to do this.

If you're used to doing view creation and view restoration in one big method, then yes, it's less convenient. This library encourages splitting up responsibilities. That's not going to change. Let the view creation method create your views and let the restore view method restore the state of your views.

sdrygalski commented 8 years ago

I missed one crucial part in your code:

public LifecycleHandler() { setRetainInstance(true); // ... }

Now I understand your design. And certainly it's not less convenient. :)