Zhuinden / flowless

[DEPRECATED] Based on Flow 1.0-alpha. To keep your life simple, use zhuinden/simple-stack instead.
Apache License 2.0
141 stars 7 forks source link

Possible randomly occurring freeze where `pendingTraversal != null, state == DISPATCHED` #8

Closed Zhuinden closed 8 years ago

Zhuinden commented 8 years ago

There can be a traversal in which nextHistory == null and state == DISPATCHED.

Image

Flowless1.0-alpha1.1 does not handle this special case

    void setDispatcher(@NonNull Dispatcher dispatcher, boolean created) {
        this.dispatcher = checkNotNull(dispatcher, "dispatcher");

        if(pendingTraversal == null && created) {
            // initialization should occur for current state if views are created or re-created
            move(new PendingTraversal() {
                @Override
                void doExecute() {
                    bootstrap(history);
                }
            });
        } else if(pendingTraversal == null) { // no-op if the view still exists and nothing to be done
            return;
        } else if(pendingTraversal.state == TraversalState.DISPATCHED) { // a traversal is in progress
            // do nothing, pending traversal will finish

created == true, but pendingTraversal != null, but the pending traversal will never finish.

The newly added fix for the midflight reentrance test might help

  if((pendingTraversal == null && created) || (pendingTraversal != null && pendingTraversal.state == TraversalState.DISPATCHED && pendingTraversal.next == null)) {

However, the traversal would only call traversal.next if onTraversalCompleted() is called, which in this case might be never.

This special case must be analyzed further and fixed.

Zhuinden commented 8 years ago

Hopefully fixed with 1.0-alpha1.2

Zhuinden commented 8 years ago

Not fixed. Attempted fixes failed, so 1.0-alpha1.5 might still have this.

Will need to reproduce through test, and eliminate the error, rather than just sharpshooting.

This probably came over from Flow 1.0-alpha itself.

Zhuinden commented 8 years ago

The error is not in Flow 1.0-alpha, it's in SingleRootDispatcher.

If two rotations are done during a state transition, onMeasure() is never called.

    ViewUtils.waitForMeasure(newView, new ViewUtils.OnMeasuredCallback() {
        @Override
        public void onMeasured(View view, int width, int height) {
            Animator animator = DispatcherUtils.createAnimatorForViews(animatedKey, previousView, newView, direction);
            if(animator != null) {
                animator.addListener(new AnimatorListenerAdapter() {
                    @Override
                    public void onAnimationEnd(Animator animation) {
                        finishTransition(previousView, root, callback);
                    }
                });
                animator.start();
            } else {
                finishTransition(previousView, root, callback);
            }
        }
    });

Sometimes, onMeasured() is never called, and as a result, the state transition will never finish.

Zhuinden commented 8 years ago

Well... technically there is more to it than that.

Apparently the issue is that the dispatcher can be removed midway, and the onMeasure() won't be called.

There is another issue that the InternalLifecycleIntegration fragment isn't properly bound by application.registerLifecycleCallbacks() if the screen is rotated while the app is still starting up.. but I haven't been able to reproduce it reliably.

Zhuinden commented 8 years ago

Fixed in 1.0-alpha1.6