[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
(cherry picked from commit 413a047)

Author: drchen

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

@@ -54,6 +54,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;
@@ -151,6 +152,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;
@@ -251,6 +253,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
@@ -707,17 +719,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/chip/ChipGroupTest.java

@@ -15,15 +15,18 @@
  */
 package com.google.android.material.chip;
 
-import com.google.android.material.R;
+import android.widget.CompoundButton.OnCheckedChangeListener;
+import com.google.android.material.test.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 +199,41 @@ 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(
+        new OnCheckedChangeListener() {
+          @Override
+          public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
+            onChipCheckedStateChanged(buttonView, isChecked);
+          }
+        });
+
+    Chip second = (Chip) chipgroup.getChildAt(1);
+    second.setOnCheckedChangeListener(
+        new OnCheckedChangeListener() {
+          @Override
+          public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
+            onChipCheckedStateChanged(buttonView, isChecked);
+          }
+        });
+
+    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);
@@ -260,4 +298,9 @@ public void isNotSingleLine_initializesAccessibilityNodeInfo() {
     assertEquals(1, itemInfo.getRowIndex());
     assertTrue(itemInfo.isSelected());
   }
+
+  private void onChipCheckedStateChanged(CompoundButton chip, boolean checked) {
+    checkedChangeCallCount++;
+    checkedIds = chipgroup.getCheckedChipIds();
+  }
 }