From c51218bc6c03791bf61814199144fef6efc8ec25 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Thu, 2 Apr 2015 13:29:12 -0700 Subject: [PATCH] Fix view state persistence 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(). --- .../java/com/example/flow/MainActivity.java | 9 ++-- .../com/example/flow/view/FriendView.java | 4 +- .../src/main/res/layout/friend_view.xml | 10 +++- .../main/java/flow/ActivityFlowSupport.java | 54 +++++++++---------- flow/src/main/java/flow/Backstack.java | 43 ++++++++++++--- 5 files changed, 74 insertions(+), 46 deletions(-) diff --git a/flow-sample/src/main/java/com/example/flow/MainActivity.java b/flow-sample/src/main/java/com/example/flow/MainActivity.java index 99be801..a16e80f 100644 --- a/flow-sample/src/main/java/com/example/flow/MainActivity.java +++ b/flow-sample/src/main/java/com/example/flow/MainActivity.java @@ -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) { @@ -83,7 +80,7 @@ protected void onCreate(Bundle savedInstanceState) { @Override protected void onResume() { super.onResume(); - flowSupport.onResume(this); + flowSupport.onResume(); } @Override protected void onPause() { diff --git a/flow-sample/src/main/java/com/example/flow/view/FriendView.java b/flow-sample/src/main/java/com/example/flow/view/FriendView.java index 41586dd..6a9d7bb 100644 --- a/flow-sample/src/main/java/com/example/flow/view/FriendView.java +++ b/flow-sample/src/main/java/com/example/flow/view/FriendView.java @@ -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; @@ -30,7 +30,7 @@ import java.util.List; import javax.inject.Inject; -public class FriendView extends FrameLayout { +public class FriendView extends LinearLayout { @Inject List friends; private final User friend; diff --git a/flow-sample/src/main/res/layout/friend_view.xml b/flow-sample/src/main/res/layout/friend_view.xml index 24c08d1..f6baa1b 100644 --- a/flow-sample/src/main/res/layout/friend_view.xml +++ b/flow-sample/src/main/res/layout/friend_view.xml @@ -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" > + diff --git a/flow/src/main/java/flow/ActivityFlowSupport.java b/flow/src/main/java/flow/ActivityFlowSupport.java index f5d3ce0..876cdda 100644 --- a/flow/src/main/java/flow/ActivityFlowSupport.java +++ b/flow/src/main/java/flow/ActivityFlowSupport.java @@ -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 @@ -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) { @@ -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) { @@ -109,11 +113,11 @@ 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() { @@ -121,9 +125,8 @@ public NonConfigurationInstance onRetainNonConfigurationInstance() { } public void onPause() { - checkState(dispatcher != null, "Did you forget to call onResume()?"); flow.removeDispatcher(dispatcher); - dispatcher = null; + dispatcherSet = false; } /** @@ -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); + } } /** @@ -161,18 +169,4 @@ private static Backstack selectBackstack(Intent intent, Backstack saved, } return defaultBackstack; } - - private static Backstack getBackstackToSave(Backstack backstack) { - Iterator 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(); - } } diff --git a/flow/src/main/java/flow/Backstack.java b/flow/src/main/java/flow/Backstack.java index af7425f..92e8e89 100644 --- a/flow/src/main/java/flow/Backstack.java +++ b/flow/src/main/java/flow/Backstack.java @@ -31,7 +31,9 @@ * Describes the history of a {@link Flow} at a specific point in time. */ public final class Backstack implements Iterable { - private final Deque 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) { @@ -47,18 +49,38 @@ public static Backstack from(Parcelable parcelable, Parceler parceler) { return new Backstack(entries); } + private final Deque backstack; + /** Get a {@link Parcelable} of this backstack using the supplied {@link Parceler}. */ public Parcelable getParcelable(Parceler parceler) { Bundle backstackBundle = new Bundle(); - ArrayList 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 entryBundles = new ArrayList<>(backstack.size()); + Iterator 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; } @@ -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;