Merge pull request #1620 from lonnyzhang423/master

Avoid all @OnClick events may not working
JakeWharton · Aug 3, 2020 · 2723228 · 2723228
2 parents e82a906 + 51e872d
commit 2723228
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 89 deletions.
diff --git a/...integration-test/src/androidTest/java/com/example/butterknife/functional/OnClickTest.java b/...integration-test/src/androidTest/java/com/example/butterknife/functional/OnClickTest.java
@@ -1,13 +1,13 @@
 package com.example.butterknife.functional;
 
+import android.app.Instrumentation;
 import android.content.Context;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.Button;
 import android.widget.FrameLayout;
 import android.widget.TextView;
 import androidx.test.InstrumentationRegistry;
-import androidx.test.annotation.UiThreadTest;
 import butterknife.ButterKnife;
 import butterknife.OnClick;
 import butterknife.Optional;
@@ -21,6 +21,8 @@
 
 @SuppressWarnings("unused") // Used reflectively / by code gen.
 public final class OnClickTest {
+  private final Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation();
+
   static final class Simple {
     int clicks = 0;
 
@@ -29,7 +31,6 @@ static final class Simple {
     }
   }
 
-  @UiThreadTest
   @Test public void simple() {
     View tree = ViewTree.create(1);
     View view1 = tree.findViewById(1);
@@ -38,12 +39,16 @@ static final class Simple {
     Unbinder unbinder = ButterKnife.bind(target, tree);
     assertEquals(0, target.clicks);
 
-    view1.performClick();
-    assertEquals(1, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      view1.performClick();
+      assertEquals(1, target.clicks);
+    });
 
-    unbinder.unbind();
-    view1.performClick();
-    assertEquals(1, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      unbinder.unbind();
+      view1.performClick();
+      assertEquals(1, target.clicks);
+    });
   }
 
   static final class MultipleBindings {
@@ -58,7 +63,6 @@ static final class MultipleBindings {
     }
   }
 
-  @UiThreadTest
   @Test public void multipleBindings() {
     assumeFalse("Not implemented", BuildConfig.FLAVOR.equals("reflect")); // TODO
 
@@ -69,12 +73,16 @@ static final class MultipleBindings {
     Unbinder unbinder = ButterKnife.bind(target, tree);
     assertEquals(0, target.clicks);
 
-    view1.performClick();
-    assertEquals(2, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      view1.performClick();
+      assertEquals(2, target.clicks);
+    });
 
-    unbinder.unbind();
-    view1.performClick();
-    assertEquals(2, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      unbinder.unbind();
+      view1.performClick();
+      assertEquals(2, target.clicks);
+    });
   }
 
   static final class Visibilities {
@@ -93,7 +101,6 @@ static final class Visibilities {
     }
   }
 
-  @UiThreadTest
   @Test public void visibilities() {
     View tree = ViewTree.create(1, 2, 3);
     View view1 = tree.findViewById(1);
@@ -104,14 +111,20 @@ static final class Visibilities {
     ButterKnife.bind(target, tree);
     assertEquals(0, target.clicks);
 
-    view1.performClick();
-    assertEquals(1, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      view1.performClick();
+      assertEquals(1, target.clicks);
+    });
 
-    view2.performClick();
-    assertEquals(2, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      view2.performClick();
+      assertEquals(2, target.clicks);
+    });
 
-    view3.performClick();
-    assertEquals(3, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      view3.performClick();
+      assertEquals(3, target.clicks);
+    });
   }
 
   static final class MultipleIds {
@@ -122,7 +135,6 @@ static final class MultipleIds {
     }
   }
 
-  @UiThreadTest
   @Test public void multipleIds() {
     View tree = ViewTree.create(1, 2);
     View view1 = tree.findViewById(1);
@@ -132,16 +144,22 @@ static final class MultipleIds {
     Unbinder unbinder = ButterKnife.bind(target, tree);
     assertEquals(0, target.clicks);
 
-    view1.performClick();
-    assertEquals(1, target.clicks);
-
-    view2.performClick();
-    assertEquals(2, target.clicks);
-
-    unbinder.unbind();
-    view1.performClick();
-    view2.performClick();
-    assertEquals(2, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      view1.performClick();
+      assertEquals(1, target.clicks);
+    });
+
+    instrumentation.runOnMainSync(() -> {
+      view2.performClick();
+      assertEquals(2, target.clicks);
+    });
+
+    instrumentation.runOnMainSync(() -> {
+      unbinder.unbind();
+      view1.performClick();
+      view2.performClick();
+      assertEquals(2, target.clicks);
+    });
   }
 
   static final class OptionalId {
@@ -152,7 +170,6 @@ static final class OptionalId {
     }
   }
 
-  @UiThreadTest
   @Test public void optionalIdPresent() {
     View tree = ViewTree.create(1);
     View view1 = tree.findViewById(1);
@@ -161,15 +178,18 @@ static final class OptionalId {
     Unbinder unbinder = ButterKnife.bind(target, tree);
     assertEquals(0, target.clicks);
 
-    view1.performClick();
-    assertEquals(1, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      view1.performClick();
+      assertEquals(1, target.clicks);
+    });
 
-    unbinder.unbind();
-    view1.performClick();
-    assertEquals(1, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      unbinder.unbind();
+      view1.performClick();
+      assertEquals(1, target.clicks);
+    });
   }
 
-  @UiThreadTest
   @Test public void optionalIdAbsent() {
     View tree = ViewTree.create(2);
     View view2 = tree.findViewById(2);
@@ -178,12 +198,16 @@ static final class OptionalId {
     Unbinder unbinder = ButterKnife.bind(target, tree);
     assertEquals(0, target.clicks);
 
-    view2.performClick();
-    assertEquals(0, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      view2.performClick();
+      assertEquals(0, target.clicks);
+    });
 
-    unbinder.unbind();
-    view2.performClick();
-    assertEquals(0, target.clicks);
+    instrumentation.runOnMainSync(() -> {
+      unbinder.unbind();
+      view2.performClick();
+      assertEquals(0, target.clicks);
+    });
   }
 
   static final class ArgumentCast {
@@ -208,18 +232,11 @@ interface MyInterface {}
     }
   }
 
-  @UiThreadTest
   @Test public void argumentCast() {
     class MyView extends Button implements ArgumentCast.MyInterface {
       MyView(Context context) {
         super(context);
       }
-
-      @Override public boolean post(Runnable action) {
-        // Because of DebouncingOnClickListener, we run any posted Runnables synchronously.
-        action.run();
-        return true;
-      }
     }
 
     View view1 = new MyView(InstrumentationRegistry.getContext());
@@ -239,16 +256,24 @@ class MyView extends Button implements ArgumentCast.MyInterface {
     ArgumentCast target = new ArgumentCast();
     ButterKnife.bind(target, tree);
 
-    view1.performClick();
-    assertSame(view1, target.last);
-
-    view2.performClick();
-    assertSame(view2, target.last);
-
-    view3.performClick();
-    assertSame(view3, target.last);
-
-    view4.performClick();
-    assertSame(view4, target.last);
+    instrumentation.runOnMainSync(() -> {
+      view1.performClick();
+      assertSame(view1, target.last);
+    });
+
+    instrumentation.runOnMainSync(() -> {
+      view2.performClick();
+      assertSame(view2, target.last);
+    });
+
+    instrumentation.runOnMainSync(() -> {
+      view3.performClick();
+      assertSame(view3, target.last);
+    });
+
+    instrumentation.runOnMainSync(() -> {
+      view4.performClick();
+      assertSame(view4, target.last);
+    });
   }
 }
diff --git a/...fe-integration-test/src/androidTest/java/com/example/butterknife/functional/ViewTree.java b/...fe-integration-test/src/androidTest/java/com/example/butterknife/functional/ViewTree.java
@@ -17,36 +17,20 @@ static View create(Class<? extends View> cls, int... ids) {
     ViewGroup group = new FrameLayout(context);
     for (int id : ids) {
       View view;
-      if (cls == View.class) {
-        view = new NoPostView(context);
-      } else {
-        try {
-          view = cls.getConstructor(Context.class).newInstance(context);
-        } catch (IllegalAccessException | InstantiationException | NoSuchMethodException e) {
-          throw new RuntimeException(e);
-        } catch (InvocationTargetException e) {
-          Throwable cause = e.getCause();
-          if (cause instanceof RuntimeException) throw (RuntimeException) cause;
-          if (cause instanceof Error) throw (Error) cause;
-          throw new RuntimeException(cause);
-        }
+      try {
+        view = cls.getConstructor(Context.class).newInstance(context);
+      } catch (IllegalAccessException | InstantiationException | NoSuchMethodException e) {
+        throw new RuntimeException(e);
+      } catch (InvocationTargetException e) {
+        Throwable cause = e.getCause();
+        if (cause instanceof RuntimeException) throw (RuntimeException) cause;
+        if (cause instanceof Error) throw (Error) cause;
+        throw new RuntimeException(cause);
       }
 
       view.setId(id);
       group.addView(view);
     }
     return group;
   }
-
-  private static final class NoPostView extends View {
-    NoPostView(Context context) {
-      super(context);
-    }
-
-    @Override public boolean post(Runnable action) {
-      // Because of DebouncingOnClickListener, we run any posted Runnables synchronously.
-      action.run();
-      return true;
-    }
-  }
 }
diff --git a/butterknife-runtime/src/main/java/butterknife/internal/DebouncingOnClickListener.java b/butterknife-runtime/src/main/java/butterknife/internal/DebouncingOnClickListener.java
@@ -1,20 +1,28 @@
 package butterknife.internal;
 
+import android.os.Handler;
+import android.os.Looper;
 import android.view.View;
 
 /**
  * A {@linkplain View.OnClickListener click listener} that debounces multiple clicks posted in the
  * same frame. A click on one button disables all buttons for that frame.
  */
 public abstract class DebouncingOnClickListener implements View.OnClickListener {
-  static boolean enabled = true;
-
   private static final Runnable ENABLE_AGAIN = () -> enabled = true;
+  private static final Handler MAIN = new Handler(Looper.getMainLooper());
+
+  static boolean enabled = true;
 
   @Override public final void onClick(View v) {
     if (enabled) {
       enabled = false;
-      v.post(ENABLE_AGAIN);
+
+      // Post to the main looper directly rather than going through the view.
+      // Ensure that ENABLE_AGAIN will be executed, avoid static field {@link #enabled}
+      // staying in false state.
+      MAIN.post(ENABLE_AGAIN);
+
       doClick(v);
     }
   }