History may not be empty

[seligijus]

Hi guys,

I'm facing a problem with Flow(getView()).goBack() method. This crash appears when I go to next screen and then press back button very fast (while animation is still in progress).

java.lang.IllegalArgumentException: History may not be empty                                                                          at flow.Preconditions.checkArgument(Preconditions.java:27)
                           at flow.History.<init>(History.java:48)
                           at flow.History.<init>(History.java:34)
               at flow.History$Builder.build(History.java:185)
                            at flow.Flow.goBack(Flow.java:241)

I guess that the problem might be related to my cusom KeyChanger class implementation as callback.onTraversalCompleted() method is not called until animation is finished.

I'm using the latest Flow version in maven (1.0.0-alpha).

Is this a bug or am I doing something wrong? Thanks.

[Zhuinden]

This happened to me only when callback.onTraversalCompleted() wasn't called properly (it was either called too early or too late or not called at all).

Please check my implementation of the KeyChanger to get a grasp on how to handle animations right.

https://github.com/Zhuinden/First-Flow-Alpha-Project/blob/master/app/src/main/java/com/zhuinden/flow_alpha_project/MainActivity.java#L65-L104

[seligijus]

Hi, thanks for your reply. My animation logic was very similar to yours (just in case I tried to do exactly like you do) but problem is still the same.

[Zhuinden]

@EligijusStarinskas you are correct, I added a line to my transition set.setDuration(2000); to see what happens, and during transition pressing onBackPressed() I get this

03-06 14:21:42.499    9446-9446/com.zhuinden.flow_alpha_project E/InputEventSender﹕ Exception dispatching finished signal.
03-06 14:21:42.499    9446-9446/com.zhuinden.flow_alpha_project E/MessageQueue-JNI﹕ Exception in MessageQueue callback: handleReceiveCallback
03-06 14:21:42.499    9446-9446/com.zhuinden.flow_alpha_project E/MessageQueue-JNI﹕ java.lang.IllegalArgumentException: History may not be empty
            at flow.Preconditions.checkArgument(Preconditions.java:27)
            at flow.History.<init>(History.java:48)
            at flow.History.<init>(History.java:34)
            at flow.History$Builder.build(History.java:185)
            at flow.Flow.goBack(Flow.java:241)
            at com.zhuinden.flow_alpha_project.MainActivity.onBackPressed(MainActivity.java:134)
            at android.app.Activity.onKeyUp(Activity.java:2159)

This is clearly a bug in Flow, as it's supposed to return false and it doesn't

  /**
   * Go back one key.
   *
   * @return false if going back is not possible or a traversal is in progress.
   */
  public boolean goBack() {
    boolean canGoBack = history.size() > 1 || (pendingTraversal != null
        && pendingTraversal.state != TraversalState.FINISHED);
    if (!canGoBack) return false;
    History.Builder builder = history.buildUpon();
    builder.pop();
    final History newHistory = builder.build(); // <--- crash is here
    setHistory(newHistory, Direction.BACKWARD);
    return true;
  }

[Zhuinden]

In the meantime, I snatched the master branch of Flow and modified Flow like this:

  /**
   * Go back one key.
   *
   * @return false if going back is not possible or a traversal is in progress.
   */
  @CheckResult public boolean goBack() {
    boolean traversalInProgress = isTraversalInProgress();
    if(traversalInProgress) {
      return false;
    }
    boolean canGoBack = history.size() > 1;
    if (!canGoBack) return false;
    History.Builder builder = history.buildUpon();
    builder.pop();
    final History newHistory = builder.build();
    setHistory(newHistory, Direction.BACKWARD);
    return true;
  }

  public boolean isTraversalInProgress() {
    return (pendingTraversal != null
            && pendingTraversal.state != TraversalState.FINISHED);
  }

And also modified my MainActivity like this

@Override
public void onBackPressed() {
    Flow flow = Flow.get(this);
    if(flow.isTraversalInProgress()) {
        return;
    } else if(!flow.goBack()) {
        super.onBackPressed();
    }
}

[seligijus]

I think that main activity shouldn't be part of this logic. So here is my suggestion

public boolean goBack() {
    if (history.size() <= 1) return false;
    if(!isTraversalInProgress()){
       History.Builder builder = history.buildUpon();
       builder.pop();
       final History newHistory = builder.build();
       setHistory(newHistory, Direction.BACKWARD);
    }
    return true;
  }

public boolean isTraversalInProgress() {
    return (pendingTraversal != null
            && pendingTraversal.state != TraversalState.FINISHED);
  }

[Zhuinden]

@EligijusStarinskas you're right.

Modified back:

@Override
public void onBackPressed() {
    if(!Flow.get(this).goBack()) {
        super.onBackPressed();
    }
}

And in Flow:

  /**
   * Go back one key.
   *
   * @return false if going back is not possible or a traversal is in progress.
   */
  @CheckResult public boolean goBack() {
    if(pendingTraversal != null && pendingTraversal.state != TraversalState.FINISHED) return true;
    boolean canGoBack = history.size() > 1;
    if (!canGoBack) return false;
    History.Builder builder = history.buildUpon();
    builder.pop();
    final History newHistory = builder.build();
    setHistory(newHistory, Direction.BACKWARD);
    return true;
  }

Essentially, just return true during an active traversal instead of false.

Added a pull request to address this issue.

#176

[seligijus]

👍

[Zhuinden]

...

Well this test fails now
https://github.com/square/flow/blob/master/flow/src/test/java/flow/ReentranceTest.java#L63-L91
I wonder what happens there, the logs only said it verified something else.

[Zhuinden]

@EligijusStarinskas Looking at the test, what happens is that this change would disable the ability to set a different active Key during an active traversal.

I guess my initial change would be the way to go.