GitHub/Conductor
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Change handlers that occur after a deferred change handler are now deferred until the preceding handler completes to ensure correct ordering. Handlers will only be deferred when the containing view isn’t yet fully attached and they could cause a NPE in ViewGroup.java.

Browse Source
This commit is contained in:
Eric Kuck
2017-05-18 07:36:02 -05:00
parent a84b2fb4c4
commit d260dfcf12
4 changed files with 110 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
conductor/src/main/java/com/bluelinelabs/conductor/ControllerChangeHandler.java
+22 -4
View File
@@ -145,11 +145,11 @@ public abstract class ControllerChangeHandler {
}
}
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<ControllerChangeListener> listeners) {
if (isPush && to != null && to.isDestroyed()) {
throw new IllegalStateException("Trying to push a controller that has already been destroyed. (" + to.getClass().getSimpleName() + ")");
}
static void executeChange(@NonNull final ChangeTransaction transaction) {
executeChange(transaction.to, transaction.from, transaction.isPush, transaction.container, transaction.changeHandler, transaction.listeners);
}
private 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<ControllerChangeListener> listeners) {
if (container != null) {
final ControllerChangeHandler handler;
if (inHandler == null) {
@@ -231,6 +231,24 @@ public abstract class ControllerChangeHandler {
forceRemoveViewOnPush = force;
}
static class ChangeTransaction {
@Nullable final Controller to;
@Nullable final Controller from;
final boolean isPush;
@Nullable final ViewGroup container;
@Nullable final ControllerChangeHandler changeHandler;
@NonNull final List<ControllerChangeListener> listeners;
public ChangeTransaction(@Nullable Controller to, @Nullable Controller from, boolean isPush, @Nullable ViewGroup container, @Nullable ControllerChangeHandler changeHandler, @NonNull List<ControllerChangeListener> listeners) {
this.to = to;
this.from = from;
this.isPush = isPush;
this.container = container;
this.changeHandler = changeHandler;
this.listeners = listeners;
}
}
/**
* A listener interface useful for allowing external classes to be notified of change events.
*/
conductor/src/main/java/com/bluelinelabs/conductor/Router.java
+32 -9
View File
@@ -14,6 +14,7 @@ import android.view.View;
import android.view.ViewGroup;
import com.bluelinelabs.conductor.Controller.LifecycleListener;
import com.bluelinelabs.conductor.ControllerChangeHandler.ChangeTransaction;
import com.bluelinelabs.conductor.ControllerChangeHandler.ControllerChangeListener;
import com.bluelinelabs.conductor.changehandler.SimpleSwapChangeHandler;
import com.bluelinelabs.conductor.internal.NoOpControllerChangeHandler;
@@ -35,8 +36,9 @@ public abstract class Router {
private static final String KEY_BACKSTACK = "Router.backstack";
private static final String KEY_POPS_LAST_VIEW = "Router.popsLastView";
protected final Backstack backstack = new Backstack();
final Backstack backstack = new Backstack();
private final List<ControllerChangeListener> changeListeners = new ArrayList<>();
private final List<ChangeTransaction> pendingControllerChanges = new ArrayList<>();
final List<Controller> destroyingControllers = new ArrayList<>();
private boolean popsLastView = false;
@@ -726,8 +728,8 @@ public abstract class Router {
to.ensureValidIndex(getTransactionIndexer());
setControllerRouter(toController);
} else if (backstack.size() == 0 && !popsLastView) {
// We're emptying out the backstack. Views get weird if you transition them out, so just no-op it. The hosting
// Activity should be handling this by finishing or at least hiding this view.
// We're emptying out the backstack. Views get weird if you transition them out, so just no-op it. The host
// Activity or controller should be handling this by finishing or at least hiding this view.
changeHandler = new NoOpControllerChangeHandler();
}
@@ -735,18 +737,39 @@ public abstract class Router {
}
private void performControllerChange(@Nullable final Controller to, @Nullable final Controller from, final boolean isPush, @Nullable final ControllerChangeHandler changeHandler) {
// If the change handler will remove the from view, we have to make sure the container is fully attached first so we avoid NPEs
// within ViewGroup (details on issue #287). Post this to the container to ensure the attach is complete before we try to remove
// anything.
if (from != null && (changeHandler == null || changeHandler.removesFromViewOnPush()) && !containerFullyAttached) {
if (isPush && to != null && to.isDestroyed()) {
throw new IllegalStateException("Trying to push a controller that has already been destroyed. (" + to.getClass().getSimpleName() + ")");
}
final ChangeTransaction transaction = new ChangeTransaction(to, from, isPush, container, changeHandler, changeListeners);
if (pendingControllerChanges.size() > 0) {
// If we already have changes queued up (awaiting full container attach), queue this one up as well so they don't happen
// out of order.
pendingControllerChanges.add(transaction);
} else if (from != null && (changeHandler == null || changeHandler.removesFromViewOnPush()) && !containerFullyAttached) {
// If the change handler will remove the from view, we have to make sure the container is fully attached first so we avoid NPEs
// within ViewGroup (details on issue #287). Post this to the container to ensure the attach is complete before we try to remove
// anything.
pendingControllerChanges.add(transaction);
container.post(new Runnable() {
@Override
public void run() {
ControllerChangeHandler.executeChange(to, from, isPush, container, changeHandler, changeListeners);
performPendingControllerChanges();
}
});
} else {
ControllerChangeHandler.executeChange(to, from, isPush, container, changeHandler, changeListeners);
ControllerChangeHandler.executeChange(transaction);
}
}
void performPendingControllerChanges() {
// We're intentionally using dynamic size checking (list.size()) here so we can account for changes
// that occur during this loop (ex: if a controller is popped from within onAttach)
for (int i = 0; i < pendingControllerChanges.size(); i++) {
ControllerChangeHandler.executeChange(pendingControllerChanges.get(i));
}
}
conductor/src/main/java/com/bluelinelabs/conductor/changehandler/AnimatorChangeHandler.java
+55 -18
View File
@@ -21,6 +21,7 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler {
private static final String KEY_DURATION = "AnimatorChangeHandler.duration";
private static final String KEY_REMOVES_FROM_ON_PUSH = "AnimatorChangeHandler.removesFromViewOnPush";
@SuppressWarnings("WeakerAccess")
public static final long DEFAULT_ANIMATION_DURATION = -1;
private long animationDuration;
@@ -29,15 +30,19 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler {
private boolean needsImmediateCompletion;
private boolean completed;
private Animator animator;
private OnAnimationReadyOrAbortedListener onAnimationReadyOrAbortedListener;
@SuppressWarnings("WeakerAccess")
public AnimatorChangeHandler() {
this(DEFAULT_ANIMATION_DURATION, true);
}
@SuppressWarnings("WeakerAccess")
public AnimatorChangeHandler(boolean removesFromViewOnPush) {
this(DEFAULT_ANIMATION_DURATION, removesFromViewOnPush);
}
@SuppressWarnings("WeakerAccess")
public AnimatorChangeHandler(long duration) {
this(duration, true);
}
@@ -68,6 +73,8 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler {
canceled = true;
if (animator != null) {
animator.cancel();
} else if (onAnimationReadyOrAbortedListener != null) {
onAnimationReadyOrAbortedListener.onReadyOrAborted();
}
}
@@ -78,6 +85,8 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler {
needsImmediateCompletion = true;
if (animator != null) {
animator.end();
} else if (onAnimationReadyOrAbortedListener != null) {
onAnimationReadyOrAbortedListener.onReadyOrAborted();
}
}
@@ -121,24 +130,8 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler {
if (to.getWidth() <= 0 && to.getHeight() <= 0) {
readyToAnimate = false;
to.getViewTreeObserver().addOnPreDrawListener(new ViewTreeObserver.OnPreDrawListener() {
boolean hasRun;
@Override
public boolean onPreDraw() {
final ViewTreeObserver observer = to.getViewTreeObserver();
if (observer.isAlive()) {
observer.removeOnPreDrawListener(this);
}
// Apparently this gets called multiple times, even if removeOnPreDrawListener is called successfully.
if (!hasRun) {
hasRun = true;
performAnimation(container, from, to, isPush, addingToView, changeListener);
}
return true;
}
});
onAnimationReadyOrAbortedListener = new OnAnimationReadyOrAbortedListener(container, from, to, isPush, true, changeListener);
to.getViewTreeObserver().addOnPreDrawListener(onAnimationReadyOrAbortedListener);
}
}
@@ -160,6 +153,8 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler {
animator.cancel();
animator = null;
}
onAnimationReadyOrAbortedListener = null;
}
private void performAnimation(@NonNull final ViewGroup container, @Nullable final View from, @Nullable final View to, final boolean isPush, final boolean toAddedToContainer, @NonNull final ControllerChangeCompletedListener changeListener) {
@@ -213,4 +208,46 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler {
animator.start();
}
private class OnAnimationReadyOrAbortedListener implements ViewTreeObserver.OnPreDrawListener {
@NonNull final ViewGroup container;
@Nullable final View from;
@Nullable final View to;
final boolean isPush;
final boolean addingToView;
@NonNull final ControllerChangeCompletedListener changeListener;
private boolean hasRun;
OnAnimationReadyOrAbortedListener(@NonNull ViewGroup container, @Nullable View from, @Nullable View to, boolean isPush, boolean addingToView, @NonNull ControllerChangeCompletedListener changeListener) {
this.container = container;
this.from = from;
this.to = to;
this.isPush = isPush;
this.addingToView = addingToView;
this.changeListener = changeListener;
}
@Override
public boolean onPreDraw() {
onReadyOrAborted();
return true;
}
void onReadyOrAborted() {
if (!hasRun) {
hasRun = true;
if (to != null) {
final ViewTreeObserver observer = to.getViewTreeObserver();
if (observer.isAlive()) {
observer.removeOnPreDrawListener(this);
}
}
performAnimation(container, from, to, isPush, addingToView, changeListener);
}
}
}
}
demo/src/main/java/com/bluelinelabs/conductor/demo/changehandler/SharedElementDelayingChangeHandler.java
+1
View File
@@ -70,6 +70,7 @@ public class SharedElementDelayingChangeHandler extends ArcFadeMoveChangeHandler
if (waitForTransitionNames.size() == 0) {
to.getViewTreeObserver().removeOnPreDrawListener(onPreDrawListener);
onPreDrawListener = null;
to.setVisibility(View.INVISIBLE);