[ChipGroup] Fix ChipGroup.getCheckedChipIds() returns wrong state

In the Chip implementation, onCheckedChangeListener was called before onCheckedChangeListenerInternal. This causes an issue that in onCheckedChangeListener's callback, the checkable group's checked state is not updated yet, therefore ChipGroup.getCheckedChipIds() will return the outdated checked state.

Fixes this by overriding Chip.setOnCheckedChangeListener to get full control of the execution order between onCheckedChangeListener and onCheckedChangeListenerInternal.

Resolves #2691

PiperOrigin-RevId: 449100861

Author: drchen

lib/java/com/google/android/material/chip/Chip.java

@@ -55,6 +55,7 @@
 import android.view.ViewParent;
 import android.view.accessibility.AccessibilityEvent;
 import android.view.accessibility.AccessibilityNodeInfo;
+import android.widget.CompoundButton;
 import androidx.annotation.AnimatorRes;
 import androidx.annotation.BoolRes;
 import androidx.annotation.CallSuper;
@@ -152,6 +153,7 @@ public class Chip extends AppCompatCheckBox
   @Nullable private RippleDrawable ripple;
 
   @Nullable private OnClickListener onCloseIconClickListener;
+  @Nullable private CompoundButton.OnCheckedChangeListener onCheckedChangeListener;
   @Nullable private MaterialCheckable.OnCheckedChangeListener<Chip> onCheckedChangeListenerInternal;
   private boolean deferredCheckedValue;
   private boolean closeIconPressed;
@@ -250,6 +252,16 @@ public Chip(Context context, AttributeSet attrs, int defStyleAttr) {
       setMinHeight(minTouchTargetSize);
     }
     lastLayoutDirection = ViewCompat.getLayoutDirection(this);
+
+    super.setOnCheckedChangeListener(
+        (buttonView, isChecked) -> {
+          if (onCheckedChangeListenerInternal != null) {
+            onCheckedChangeListenerInternal.onCheckedChanged(Chip.this, isChecked);
+          }
+          if (onCheckedChangeListener != null) {
+            onCheckedChangeListener.onCheckedChanged(buttonView, isChecked);
+          }
+        });
   }
 
   @Override
@@ -706,17 +718,17 @@ public void setChecked(boolean checked) {
       // Defer the setChecked() call until after initialization.
       deferredCheckedValue = checked;
     } else if (chipDrawable.isCheckable()) {
-      boolean wasChecked = isChecked();
       super.setChecked(checked);
-
-      if (wasChecked != checked) {
-        if (onCheckedChangeListenerInternal != null) {
-          onCheckedChangeListenerInternal.onCheckedChanged(this, checked);
-        }
-      }
     }
   }
 
+  @Override
+  public void setOnCheckedChangeListener(
+      @Nullable CompoundButton.OnCheckedChangeListener listener) {
+    // Do not call super here - the wrapped listener set in the constructor will call the listener.
+    onCheckedChangeListener = listener;
+  }
+
   /** Register a callback to be invoked when the close icon is clicked. */
   public void setOnCloseIconClickListener(OnClickListener listener) {
     this.onCloseIconClickListener = listener;

lib/javatests/com/google/android/material/appbar/MaterialToolbarTest.java

@@ -15,7 +15,7 @@
  */
 package com.google.android.material.appbar;
 
-import com.google.android.material.test.R;
+import com.google.android.material.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

lib/javatests/com/google/android/material/badge/BadgeDrawableTest.java

@@ -15,7 +15,7 @@
  */
 package com.google.android.material.badge;
 
-import com.google.android.material.test.R;
+import com.google.android.material.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

lib/javatests/com/google/android/material/badge/BadgeUtilsTest.java

@@ -15,7 +15,7 @@
  */
 package com.google.android.material.badge;
 
-import com.google.android.material.test.R;
+import com.google.android.material.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

lib/javatests/com/google/android/material/bottomnavigation/BottomNavigationViewTest.java

@@ -15,7 +15,7 @@
  */
 package com.google.android.material.bottomnavigation;
 
-import com.google.android.material.test.R;
+import com.google.android.material.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

lib/javatests/com/google/android/material/bottomsheet/BottomSheetBehaviorTest.java

@@ -15,7 +15,7 @@
  */
 package com.google.android.material.bottomsheet;
 
-import com.google.android.material.test.R;
+import com.google.android.material.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

lib/javatests/com/google/android/material/button/MaterialButtonTest.java

@@ -15,7 +15,7 @@
  */
 package com.google.android.material.button;
 
-import com.google.android.material.test.R;
+import com.google.android.material.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

lib/javatests/com/google/android/material/button/MaterialButtonToggleGroupTest.java

@@ -16,7 +16,7 @@
 
 package com.google.android.material.button;
 
-import com.google.android.material.test.R;
+import com.google.android.material.R;
 
 import static android.view.View.GONE;
 import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;

lib/javatests/com/google/android/material/checkbox/MaterialCheckBoxTest.java

@@ -16,7 +16,7 @@
 
 package com.google.android.material.checkbox;
 
-import com.google.android.material.test.R;
+import com.google.android.material.R;
 
 import static com.google.common.truth.Truth.assertThat;
 

lib/javatests/com/google/android/material/chip/ChipGroupTest.java

@@ -15,15 +15,17 @@
  */
 package com.google.android.material.chip;
 
-import com.google.android.material.test.R;
+import com.google.android.material.R;
 
+import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import android.content.Context;
 import androidx.appcompat.app.AppCompatActivity;
 import android.view.View;
+import android.widget.CompoundButton;
 import androidx.core.view.ViewCompat;
 import androidx.core.view.accessibility.AccessibilityNodeInfoCompat;
 import androidx.core.view.accessibility.AccessibilityNodeInfoCompat.CollectionInfoCompat;
@@ -196,6 +198,29 @@ public void onCheckedChanged(ChipGroup group, List<Integer> checkedIds) {
     assertThat(checkedIds).contains(second.getId());
   }
 
+  @Test
+  public void multipleSelection_chipListener() {
+    chipgroup.setSingleSelection(false);
+
+    Chip first = (Chip) chipgroup.getChildAt(0);
+    first.setOnCheckedChangeListener(this::onChipCheckedStateChanged);
+
+    Chip second = (Chip) chipgroup.getChildAt(1);
+    second.setOnCheckedChangeListener(this::onChipCheckedStateChanged);
+
+    first.performClick();
+    getInstrumentation().waitForIdleSync();
+
+    assertThat(checkedChangeCallCount).isEqualTo(1);
+    assertThat(checkedIds).containsExactly(first.getId());
+
+    second.performClick();
+    getInstrumentation().waitForIdleSync();
+
+    assertThat(checkedChangeCallCount).isEqualTo(2);
+    assertThat(checkedIds).containsExactly(first.getId(), second.getId());
+  }
+
   @Test
   public void multiSelection_withSelectionRequired_unSelectsIfTwo() {
     chipgroup.setSingleSelection(false);
@@ -273,4 +298,9 @@ public void getChipAccessibilityClassName_singleChecked_radioButtonName() {
     Chip chip = (Chip) chipgroup.getChildAt(0);
     assertEquals("android.widget.RadioButton", chip.getAccessibilityClassName().toString());
   }
+
+  private void onChipCheckedStateChanged(CompoundButton chip, boolean checked) {
+    checkedChangeCallCount++;
+    checkedIds = chipgroup.getCheckedChipIds();
+  }
 }