search

lyft / scoop

:icecream: micro framework for building view based modular Android applications.
Apache License 2.0
1.03k stars 79 forks source link

Back button handler needs debounce logic #4

Closed elijahz closed 8 years ago

elijahz commented 8 years ago

Steps to recreate with scoop-basics:

  1. Open scoop-basics app on device
  2. Click "CUSTOM TRANSITION" button
  3. Click "Next" and "Back" buttons ("@+id/next_button" and "@+id/back_button" respectively) a bunch of times, until the MainActivity shows again.
  4. Click "WIZARD SAMPLE"

Result: App crashes. Below is logcat.

MainUiContainer  D  Scoop changed:AutoTransitionEndController
  5489        MainUiContainer  D  Scoop changed:AutoTransitionEndController
  5489        MainUiContainer  D  Scoop changed:AutoTransitionEndController
  5489        MainUiContainer  D  Scoop changed:AutoTransitionStartController
  5489        MainUiContainer  D  Scoop changed:DemosController
  5489        MainUiContainer  D  Scoop changed:AutoTransitionEndController
  5489        MainUiContainer  D  Scoop changed:DemosController
  5489         AndroidRuntime  D  Shutting down VM
  5489               dalvikvm  W  threadid=1: thread exiting with uncaught exception (group=0x41708c80)
  5489         AndroidRuntime  E  FATAL EXCEPTION: main
  5489         AndroidRuntime  E  Process: com.scooptest PID: 5489
  5489         AndroidRuntime  E  java.lang.NullPointerException
  5489         AndroidRuntime  E  at com.scooptest.scoop.DaggerInjector.fromScoop(DaggerInjector.java:30)
  5489         AndroidRuntime  E  at com.scooptest.scoop.DaggerScreenScooper.addServices(DaggerScreenScooper.java:11)
  5489         AndroidRuntime  E  at com.lyft.scoop.ScreenScooper.createScreenScoop(ScreenScooper.java:10)
  5489         AndroidRuntime  E  at com.lyft.scoop.Router.goTo(Router.java:56)
  5489         AndroidRuntime  E  at com.scooptest.ui.DemosController.goToWizardSample(DemosController.java:82)
  5489         AndroidRuntime  E  at com.scooptest.ui.DemosController$$ViewBinder$5.doClick(DemosController$$ViewBinder.java:53)
  5489         AndroidRuntime  E  at butterknife.internal.DebouncingOnClickListener.onClick(DebouncingOnClickListener.java:22)
  5489         AndroidRuntime  E  at android.view.View.performClick(View.java:4768)
  5489         AndroidRuntime  E  at android.view.View$PerformClick.run(View.java:19073)
  5489         AndroidRuntime  E  at android.os.Handler.handleCallback(Handler.java:755)
  5489         AndroidRuntime  E  at android.os.Handler.dispatchMessage(Handler.java:95)
  5489         AndroidRuntime  E  at android.os.Looper.loop(Looper.java:145)
  5489         AndroidRuntime  E  at android.app.ActivityThread.main(ActivityThread.java:5266)
  5489         AndroidRuntime  E  at java.lang.reflect.Method.invokeNative(Native Method)
  5489         AndroidRuntime  E  at java.lang.reflect.Method.invoke(Method.java:515)
  5489         AndroidRuntime  E  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:826)
  5489         AndroidRuntime  E  at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:642)
  5489         AndroidRuntime  E  at dalvik.system.NativeStart.main(Native Method)
  5489         AndroidRuntime  I  To Report FATAL to activityManagerService
 16024        ActivityManager  I  handleApplicationCrash
 16024        ActivityManager  W  Force finishing activity com.scooptest/.MainActivity
  5489         AndroidRuntime  I  Finished reporting FATAL to activityManagerService
  5489                Process  I  Sending signal. PID: 5489 SIG: 9
elijahz commented 8 years ago

Note, the crash doesn't seem to happen when I modify ui/customtransition/AutoTransitionEndController.java as follows:

    @OnClick(R.id.back_button)
    public void goNext(Button button) {
        button.setEnabled(false);
        appRouter.goBack();
    }

But there is a curious/serious side-effect: the MainActivity DemosController screen still appears when you hit "@+id/next_button" and "@+id/back_button" rapidly in "CUSTOM TRANSITION" in the demo.

This leads me to believe perhaps it isn't (entirely) the back button handler, but the MainActivity DemosController screen is getting focus between transitions if a user is rapidly tapping the screen during a transition.

elijahz commented 8 years ago

I found another way to recreate the issue:

  1. Open scoop-basics app on device
  2. Tap "NAVIGATION SAMPLE" button
  3. Tap "GO TO B"
  4. Tap "GO UP" rapidly (2+ taps per second)

Result: App crashes. Below is the logcat.

Note: If you rapidly tap "GO TO A|B|C" instead, the crash does not happen.

MainUiContainer  D  Scoop changed:AController
 13190        MainUiContainer  D  Scoop changed:BController
 13190        MainUiContainer  D  Scoop changed:AController
 13190         AndroidRuntime  D  Shutting down VM
 13190               dalvikvm  W  threadid=1: thread exiting with uncaught exception (group=0x41708c80)
 13190         AndroidRuntime  E  FATAL EXCEPTION: main
 13190         AndroidRuntime  E  Process: com.scooptest, PID: 13190
 13190         AndroidRuntime  E  java.lang.RuntimeException: ParentController annotation not specified for this controller: AController
 13190         AndroidRuntime  E  at com.lyft.scoop.Router.goUp(Router.java:43)
 13190         AndroidRuntime  E  at com.scooptest.ui.navigationsample.BController.goUp(BController.java:66)
 13190         AndroidRuntime  E  at com.scooptest.ui.navigationsample.BController$$ViewBinder$1.doClick(BController$$ViewBinder.j
                               ava:17)
 13190         AndroidRuntime  E  at butterknife.internal.DebouncingOnClickListener.onClick(DebouncingOnClickListener.java:22)
 13190         AndroidRuntime  E  at android.view.View.performClick(View.java:4768)
 13190         AndroidRuntime  E  at android.view.View$PerformClick.run(View.java:19073)
 13190         AndroidRuntime  E  at android.os.Handler.handleCallback(Handler.java:755)
 13190         AndroidRuntime  E  at android.os.Handler.dispatchMessage(Handler.java:95)
 13190         AndroidRuntime  E  at android.os.Looper.loop(Looper.java:145)
 13190         AndroidRuntime  E  at android.app.ActivityThread.main(ActivityThread.java:5266)
 13190         AndroidRuntime  E  at java.lang.reflect.Method.invokeNative(Native Method)
 13190         AndroidRuntime  E  at java.lang.reflect.Method.invoke(Method.java:515)
 13190         AndroidRuntime  E  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:826)
 13190         AndroidRuntime  E  at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:642)
 13190         AndroidRuntime  E  at dalvik.system.NativeStart.main(Native Method)
 13190         AndroidRuntime  I  To Report FATAL to activityManagerService
lexer commented 8 years ago

@elijahz Thanks, I will look into it.

lexer commented 8 years ago

@eveliotc @buildbreaker guess we need queue in uicontainer. Also single instance default will make it less noticeable.