Skip to content

Commit

Permalink
Fix view state persistence
Browse files Browse the repository at this point in the history
Thanks to Aleksey Malevaniy for catching these problems and
demonstrating fixes.

This change fixes two problems with view state persistence in
ActivityFlowSupport:

1. It was just throwing view state away when building the Parcelable to
save in the Bundle. The fix is to move filtering into Backstack itself,
so that it can build a Parcelable that includes view state.

2. It wasn (re)constructing the view hierarchy for the first time in
onResume(), which is after restoreHierarchyState() is called.  View
state was therefore not getting properly restored. The fix is to rebuild
the view hierarchy in onCreate(), so that it's there in time for
restoreHierarchyState().
  • Loading branch information
loganj committed Apr 11, 2015
1 parent 12906a5 commit c51218b
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 46 deletions.
9 changes: 3 additions & 6 deletions flow-sample/src/main/java/com/example/flow/MainActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,13 @@ protected void onCreate(Bundle savedInstanceState) {
GsonParceler parceler = new GsonParceler(new Gson());
@SuppressWarnings("deprecation") ActivityFlowSupport.NonConfigurationInstance nonConfig =
(ActivityFlowSupport.NonConfigurationInstance) getLastNonConfigurationInstance();
flowSupport = ActivityFlowSupport.onCreate(nonConfig, getIntent(), savedInstanceState, parceler,
Backstack.single(new Paths.ConversationList()));

final ActionBar actionBar = getActionBar();
actionBar.setDisplayShowHomeEnabled(false);

setContentView(R.layout.root_layout);

container = (PathContainerView) findViewById(R.id.container);
containerAsBackTarget = (HandlesBack) container;
flowSupport = ActivityFlowSupport.onCreate(nonConfig, getIntent(), savedInstanceState, parceler,
Backstack.single(new Paths.ConversationList()), this);
}

@Override protected void onNewIntent(Intent intent) {
Expand All @@ -83,7 +80,7 @@ protected void onCreate(Bundle savedInstanceState) {

@Override protected void onResume() {
super.onResume();
flowSupport.onResume(this);
flowSupport.onResume();
}

@Override protected void onPause() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import android.content.Context;
import android.util.AttributeSet;
import android.widget.FrameLayout;
import android.widget.LinearLayout;
import android.widget.TextView;
import butterknife.ButterKnife;
import butterknife.InjectView;
Expand All @@ -30,7 +30,7 @@
import java.util.List;
import javax.inject.Inject;

public class FriendView extends FrameLayout {
public class FriendView extends LinearLayout {
@Inject List<User> friends;

private final User friend;
Expand Down
10 changes: 9 additions & 1 deletion flow-sample/src/main/res/layout/friend_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,19 @@
xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="vertical"
android:id="@+id/friend_view"
>
<TextView
android:id="@+id/friend_info"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:layout_height="wrap_content"
/>
<EditText
android:id="@+id/friend_note"
android:freezesText="true"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:minLines="2"
/>
</com.example.flow.view.FriendView>
54 changes: 24 additions & 30 deletions flow/src/main/java/flow/ActivityFlowSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@

import android.content.Intent;
import android.os.Bundle;
import java.util.Iterator;
import android.os.Parcelable;

import static flow.Preconditions.checkArgument;
import static flow.Preconditions.checkState;

/**
* Manages a Flow within an Activity. Make sure that each method is called from the corresponding
Expand Down Expand Up @@ -79,14 +78,18 @@ public static void setBackstackExtra(Intent intent, Backstack backstack, Parcele
private final Parceler parceler;
private final Flow flow;
private Flow.Dispatcher dispatcher;
private boolean dispatcherSet;

private ActivityFlowSupport(Flow flow, Parceler parceler) {
private ActivityFlowSupport(Flow flow, Flow.Dispatcher dispatcher, Parceler parceler) {
this.flow = flow;
this.dispatcher = dispatcher;
this.parceler = parceler;
}

/** Immediately starts the Dispatcher, so the dispatcher should be prepared before calling. */
public static ActivityFlowSupport onCreate(NonConfigurationInstance nonConfigurationInstance,
Intent intent, Bundle savedInstanceState, Parceler parceler, Backstack defaultBackstack) {
Intent intent, Bundle savedInstanceState, Parceler parceler, Backstack defaultBackstack,
Flow.Dispatcher dispatcher) {
checkArgument(parceler != null, "parceler may not be null");
final Flow flow;
if (nonConfigurationInstance != null) {
Expand All @@ -98,7 +101,8 @@ public static ActivityFlowSupport onCreate(NonConfigurationInstance nonConfigura
}
flow = new Flow(selectBackstack(intent, savedBackstack, defaultBackstack, parceler));
}
return new ActivityFlowSupport(flow, parceler);
flow.setDispatcher(dispatcher);
return new ActivityFlowSupport(flow, dispatcher, parceler);
}

public void onNewIntent(Intent intent) {
Expand All @@ -109,21 +113,20 @@ public void onNewIntent(Intent intent) {
}
}

public void onResume(Flow.Dispatcher dispatcher) {
checkArgument(dispatcher != null, "dispatcher may not be null");
this.dispatcher = dispatcher;

flow.setDispatcher(dispatcher);
public void onResume() {
if (!dispatcherSet) {
dispatcherSet = true;
flow.setDispatcher(dispatcher);
}
}

public NonConfigurationInstance onRetainNonConfigurationInstance() {
return new NonConfigurationInstance(flow);
}

public void onPause() {
checkState(dispatcher != null, "Did you forget to call onResume()?");
flow.removeDispatcher(dispatcher);
dispatcher = null;
dispatcherSet = false;
}

/**
Expand All @@ -135,10 +138,15 @@ public boolean onBackPressed() {

public void onSaveInstanceState(Bundle outState) {
checkArgument(outState != null, "outState may not be null");
Backstack backstack = getBackstackToSave(flow.getBackstack());
if (backstack == null) return;
//noinspection ConstantConditions
outState.putParcelable(BACKSTACK_KEY, backstack.getParcelable(parceler));
Parcelable parcelable = flow.getBackstack().getParcelable(parceler, new Backstack.Filter() {
@Override public boolean apply(Object state) {
return !state.getClass().isAnnotationPresent(NotPersistent.class);
}
});
if (parcelable != null) {
//noinspection ConstantConditions
outState.putParcelable(BACKSTACK_KEY, parcelable);
}
}

/**
Expand All @@ -161,18 +169,4 @@ private static Backstack selectBackstack(Intent intent, Backstack saved,
}
return defaultBackstack;
}

private static Backstack getBackstackToSave(Backstack backstack) {
Iterator<Object> it = backstack.reverseIterator();
Backstack.Builder save = Backstack.emptyBuilder();
boolean empty = true;
while (it.hasNext()) {
Object state = it.next();
if (!state.getClass().isAnnotationPresent(NotPersistent.class)) {
save.push(state);
empty = false;
}
}
return empty ? null : save.build();
}
}
43 changes: 36 additions & 7 deletions flow/src/main/java/flow/Backstack.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
* Describes the history of a {@link Flow} at a specific point in time.
*/
public final class Backstack implements Iterable<Object> {
private final Deque<Entry> backstack;
public static interface Filter {
boolean apply(Object state);
}

/** Restore a saved backstack from a {@link Parcelable} using the supplied {@link Parceler}. */
public static Backstack from(Parcelable parcelable, Parceler parceler) {
Expand All @@ -47,18 +49,38 @@ public static Backstack from(Parcelable parcelable, Parceler parceler) {
return new Backstack(entries);
}

private final Deque<Entry> backstack;

/** Get a {@link Parcelable} of this backstack using the supplied {@link Parceler}. */
public Parcelable getParcelable(Parceler parceler) {
Bundle backstackBundle = new Bundle();

ArrayList<Bundle> entryBundles = new ArrayList<>(backstack.size());

for (Entry entry : backstack) {
Bundle entryBundle = new Bundle();
entryBundle.putParcelable("OBJECT", parceler.wrap(entry.state));
entryBundle.putSparseParcelableArray("VIEW_STATE", entry.viewState);
entryBundles.add(entryBundle);
entryBundles.add(entry.getBundle(parceler));
}
backstackBundle.putParcelableArrayList("ENTRIES", entryBundles);
return backstackBundle;
}

/**
* Get a {@link Parcelable} of this backstack using the supplied {@link Parceler}, filtered
* by the supplied {@link Filter}.
*
* The filter is invoked on each state in the stack in reverse order
*
* @return null if all states are filtered out.
*/
public Parcelable getParcelable(Parceler parceler, Filter filter) {
Bundle backstackBundle = new Bundle();
ArrayList<Bundle> entryBundles = new ArrayList<>(backstack.size());
Iterator<Entry> it = backstack.descendingIterator();
while (it.hasNext()) {
Entry entry = it.next();
if (filter.apply(entry.state)) {
entryBundles.add(entry.getBundle(parceler));
}
}
Collections.reverse(entryBundles);
backstackBundle.putParcelableArrayList("ENTRIES", entryBundles);
return backstackBundle;
}
Expand Down Expand Up @@ -128,6 +150,13 @@ private static final class Entry implements ViewState {
}
}

Bundle getBundle(Parceler parceler) {
Bundle bundle = new Bundle();
bundle.putParcelable("OBJECT", parceler.wrap(state));
bundle.putSparseParcelableArray("VIEW_STATE", viewState);
return bundle;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand Down

0 comments on commit c51218b

Please sign in to comment.