From e7c195d9105d457abe3a9bba0cc7f525230233a5 Mon Sep 17 00:00:00 2001 From: Eric Kuck Date: Thu, 15 Dec 2016 15:11:25 -0600 Subject: [PATCH] Now internally ensures that ControllerChangeHandlers aren't reused, unless they're specifically designed to be safe for reuse Made ControllerChangeHandler's copy() method public Fixes #179 --- .../conductor/ControllerChangeHandler.java | 27 ++++++++++++++----- .../com/bluelinelabs/conductor/Router.java | 13 ++++----- .../AutoTransitionChangeHandler.java | 7 +++++ .../changehandler/FadeChangeHandler.java | 8 ++++++ .../HorizontalChangeHandler.java | 8 ++++++ .../SimpleSwapChangeHandler.java | 6 +++++ .../TransitionChangeHandler.java | 1 + .../TransitionChangeHandlerCompat.java | 9 +++++++ .../changehandler/VerticalChangeHandler.java | 7 +++++ .../internal/NoOpControllerChangeHandler.java | 5 ++++ .../demo/controllers/TextController.java | 14 +++++++--- 11 files changed, 87 insertions(+), 18 deletions(-) diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/ControllerChangeHandler.java b/conductor/src/main/java/com/bluelinelabs/conductor/ControllerChangeHandler.java index f2196ac7..c8538452 100644 --- a/conductor/src/main/java/com/bluelinelabs/conductor/ControllerChangeHandler.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/ControllerChangeHandler.java @@ -29,6 +29,8 @@ public abstract class ControllerChangeHandler { private boolean forceRemoveViewOnPush; + boolean hasBeenUsed; + /** * Responsible for swapping Views from one Controller to another. * @@ -73,6 +75,16 @@ public void onAbortPush(@NonNull ControllerChangeHandler newHandler, @Nullable C */ public void completeImmediately() { } + /** + * Returns a copy of this ControllerChangeHandler. This method is internally used by the library, so + * ensure it will return an exact copy of your handler if overriding. If not overriding, the handler + * will be saved and restored from the Bundle format. + */ + @NonNull + public ControllerChangeHandler copy() { + return fromBundle(toBundle()); + } + @NonNull final Bundle toBundle() { Bundle bundle = new Bundle(); @@ -85,11 +97,6 @@ final Bundle toBundle() { return bundle; } - @NonNull - final ControllerChangeHandler copy() { - return fromBundle(toBundle()); - } - private void ensureDefaultConstructor() { try { getClass().getConstructor(); @@ -135,7 +142,15 @@ public static void executeChange(@Nullable final Controller to, @Nullable final public static void executeChange(@Nullable final Controller to, @Nullable final Controller from, final boolean isPush, @Nullable final ViewGroup container, @Nullable final ControllerChangeHandler inHandler, @NonNull final List listeners) { if (container != null) { - final ControllerChangeHandler handler = inHandler != null ? inHandler : new SimpleSwapChangeHandler(); + final ControllerChangeHandler handler; + if (inHandler == null) { + handler = new SimpleSwapChangeHandler(); + } else if (inHandler.hasBeenUsed) { + handler = inHandler.copy(); + } else { + handler = inHandler; + } + handler.hasBeenUsed = true; if (isPush && to != null) { inProgressPushHandlers.put(to.getInstanceId(), handler); diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/Router.java b/conductor/src/main/java/com/bluelinelabs/conductor/Router.java index 1375f3f2..f4774845 100755 --- a/conductor/src/main/java/com/bluelinelabs/conductor/Router.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/Router.java @@ -172,7 +172,7 @@ public void replaceTopController(@NonNull RouterTransaction transaction) { final boolean newHandlerRemovesViews = handler == null || handler.removesFromViewOnPush(); if (!oldHandlerRemovedViews && newHandlerRemovesViews) { for (RouterTransaction visibleTransaction : getVisibleTransactions(backstack.iterator())) { - performControllerChange(null, visibleTransaction.controller, true, handler != null ? handler.copy() : new SimpleSwapChangeHandler()); + performControllerChange(null, visibleTransaction.controller, true, handler); } } } @@ -368,22 +368,19 @@ public void setBackstack(@NonNull List newBackstack, @Nullabl } if (visibleTransactionsChanged) { - ControllerChangeHandler handler = changeHandler != null ? changeHandler : new SimpleSwapChangeHandler(); - Controller rootController = oldVisibleTransactions.size() > 0 ? oldVisibleTransactions.get(0).controller : null; - performControllerChange(newVisibleTransactions.get(0).controller, rootController, true, handler); + performControllerChange(newVisibleTransactions.get(0).controller, rootController, true, changeHandler); for (int i = oldVisibleTransactions.size() - 1; i > 0; i--) { RouterTransaction transaction = oldVisibleTransactions.get(i); - ControllerChangeHandler localHandler = handler.copy(); + ControllerChangeHandler localHandler = changeHandler != null ? changeHandler.copy() : new SimpleSwapChangeHandler(); localHandler.setForceRemoveViewOnPush(true); performControllerChange(null, transaction.controller, true, localHandler); } for (int i = 1; i < newVisibleTransactions.size(); i++) { RouterTransaction transaction = newVisibleTransactions.get(i); - handler = transaction.pushChangeHandler() != null ? transaction.pushChangeHandler().copy() : new SimpleSwapChangeHandler(); - performControllerChange(transaction.controller, newVisibleTransactions.get(i - 1).controller, true, handler); + performControllerChange(transaction.controller, newVisibleTransactions.get(i - 1).controller, true, transaction.pushChangeHandler()); } } } @@ -631,7 +628,7 @@ private void performControllerChange(@Nullable RouterTransaction to, @Nullable R } else if (from != null) { changeHandler = from.popChangeHandler(); } else { - changeHandler = new SimpleSwapChangeHandler(); + changeHandler = null; } Controller toController = to != null ? to.controller : null; diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/AutoTransitionChangeHandler.java b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/AutoTransitionChangeHandler.java index 72a47913..6584b842 100755 --- a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/AutoTransitionChangeHandler.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/AutoTransitionChangeHandler.java @@ -9,6 +9,8 @@ import android.view.View; import android.view.ViewGroup; +import com.bluelinelabs.conductor.ControllerChangeHandler; + /** * A change handler that will use an AutoTransition. */ @@ -20,4 +22,9 @@ protected Transition getTransition(@NonNull ViewGroup container, @Nullable View return new AutoTransition(); } + @Override @NonNull + public ControllerChangeHandler copy() { + return new AutoTransitionChangeHandler(); + } + } diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/FadeChangeHandler.java b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/FadeChangeHandler.java index beba8665..eabc06a0 100755 --- a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/FadeChangeHandler.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/FadeChangeHandler.java @@ -8,6 +8,8 @@ import android.view.View; import android.view.ViewGroup; +import com.bluelinelabs.conductor.ControllerChangeHandler; + /** * An {@link AnimatorChangeHandler} that will cross fade two views */ @@ -45,4 +47,10 @@ protected Animator getAnimator(@NonNull ViewGroup container, @Nullable View from protected void resetFromView(@NonNull View from) { from.setAlpha(1); } + + @Override @NonNull + public ControllerChangeHandler copy() { + return new FadeChangeHandler(getAnimationDuration(), removesFromViewOnPush()); + } + } diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/HorizontalChangeHandler.java b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/HorizontalChangeHandler.java index 06f1d438..c5099de0 100755 --- a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/HorizontalChangeHandler.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/HorizontalChangeHandler.java @@ -8,6 +8,8 @@ import android.view.View; import android.view.ViewGroup; +import com.bluelinelabs.conductor.ControllerChangeHandler; + /** * An {@link AnimatorChangeHandler} that will slide the views left or right, depending on if it's a push or pop. */ @@ -54,4 +56,10 @@ protected Animator getAnimator(@NonNull ViewGroup container, @Nullable View from protected void resetFromView(@NonNull View from) { from.setTranslationX(0); } + + @Override @NonNull + public ControllerChangeHandler copy() { + return new HorizontalChangeHandler(getAnimationDuration(), removesFromViewOnPush()); + } + } diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/SimpleSwapChangeHandler.java b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/SimpleSwapChangeHandler.java index b39a5b38..7a27f1d9 100755 --- a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/SimpleSwapChangeHandler.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/SimpleSwapChangeHandler.java @@ -102,4 +102,10 @@ public void onViewAttachedToWindow(@NonNull View v) { @Override public void onViewDetachedFromWindow(@NonNull View v) { } + + @Override @NonNull + public ControllerChangeHandler copy() { + return new SimpleSwapChangeHandler(removesFromViewOnPush()); + } + } diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/TransitionChangeHandler.java b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/TransitionChangeHandler.java index ebdb416a..021179ce 100644 --- a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/TransitionChangeHandler.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/TransitionChangeHandler.java @@ -81,4 +81,5 @@ public void onTransitionResume(Transition transition) { } public final boolean removesFromViewOnPush() { return true; } + } diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/TransitionChangeHandlerCompat.java b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/TransitionChangeHandlerCompat.java index 6a00d179..07338e97 100644 --- a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/TransitionChangeHandlerCompat.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/TransitionChangeHandlerCompat.java @@ -69,4 +69,13 @@ public boolean removesFromViewOnPush() { return changeHandler.removesFromViewOnPush(); } + @Override @NonNull + public ControllerChangeHandler copy() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { + return new TransitionChangeHandlerCompat((TransitionChangeHandler)changeHandler.copy(), null); + } else { + return new TransitionChangeHandlerCompat(null, changeHandler.copy()); + } + } + } diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/VerticalChangeHandler.java b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/VerticalChangeHandler.java index 7322858b..fd937c2e 100755 --- a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/VerticalChangeHandler.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/VerticalChangeHandler.java @@ -8,6 +8,8 @@ import android.view.View; import android.view.ViewGroup; +import com.bluelinelabs.conductor.ControllerChangeHandler; + import java.util.ArrayList; import java.util.List; @@ -49,4 +51,9 @@ protected Animator getAnimator(@NonNull ViewGroup container, @Nullable View from @Override protected void resetFromView(@NonNull View from) { } + @Override @NonNull + public ControllerChangeHandler copy() { + return new VerticalChangeHandler(getAnimationDuration(), removesFromViewOnPush()); + } + } diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/internal/NoOpControllerChangeHandler.java b/conductor/src/main/java/com/bluelinelabs/conductor/internal/NoOpControllerChangeHandler.java index 77d7fd44..b429ac6c 100644 --- a/conductor/src/main/java/com/bluelinelabs/conductor/internal/NoOpControllerChangeHandler.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/internal/NoOpControllerChangeHandler.java @@ -14,4 +14,9 @@ public void performChange(@NonNull ViewGroup container, @Nullable View from, @Nu changeListener.onChangeCompleted(); } + @NonNull + @Override + public ControllerChangeHandler copy() { + return new NoOpControllerChangeHandler(); + } } \ No newline at end of file diff --git a/demo/src/main/java/com/bluelinelabs/conductor/demo/controllers/TextController.java b/demo/src/main/java/com/bluelinelabs/conductor/demo/controllers/TextController.java index fbfc1896..f824eea6 100755 --- a/demo/src/main/java/com/bluelinelabs/conductor/demo/controllers/TextController.java +++ b/demo/src/main/java/com/bluelinelabs/conductor/demo/controllers/TextController.java @@ -1,5 +1,6 @@ package com.bluelinelabs.conductor.demo.controllers; +import android.os.Bundle; import android.support.annotation.NonNull; import android.view.LayoutInflater; import android.view.View; @@ -8,6 +9,7 @@ import com.bluelinelabs.conductor.demo.R; import com.bluelinelabs.conductor.demo.controllers.base.BaseController; +import com.bluelinelabs.conductor.demo.util.BundleBuilder; import butterknife.BindView; @@ -18,10 +20,14 @@ public class TextController extends BaseController { @BindView(R.id.text_view) TextView textView; public TextController(String text) { -// this(new BundleBuilder(new Bundle()) -// .putString(KEY_TEXT, text) -// .build() -// ); + this(new BundleBuilder(new Bundle()) + .putString(KEY_TEXT, text) + .build() + ); + } + + public TextController(Bundle args) { + super(args); } @NonNull