commit be11abeb5d6eb8cacfd38486379d17f464c009d8
author Yang Sun <sunytt@google.com> Mon May 27 21:21:16 2024 +0800
committer Yang Sun <sunytt@google.com> Tue May 28 14:55:50 2024 +0800
tree f29473506a1cde0f8217f9abdde638d6781178f3
parent 22c6b6c9d7553e0e444f78bc68b220d939783e5d

Handle enabledState change in onStateChanged().

Only call onThreadEnableStateChanged() when the callback is registered
or the state has changed.

Bug: 342999416

Test: atest CtsThreadNetworkTestCases:android.net.thread.cts.ThreadNetworkControllerTest#subscribeThreadEnableState_getActiveDataset_onThreadEnableStateChangedNotCalled

Change-Id: I822d2b98b221c3312ff472b82ebb3e7e88e313fe

thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java

diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
index 6a9ae71..af9abdf 100644
--- a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
+++ b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
@@ -1387,14 +1387,6 @@
             }
         }
 
-        private void notifyThreadEnabledUpdated(IStateCallback callback, int enabledState) {
-            try {
-                callback.onThreadEnableStateChanged(enabledState);
-            } catch (RemoteException ignored) {
-                // do nothing if the client is dead
-            }
-        }
-
         public void unregisterStateCallback(IStateCallback callback) {
             checkOnHandlerThread();
             if (!mStateCallbacks.containsKey(callback)) {
@@ -1461,15 +1453,19 @@
             }
         }
 
-        @Override
-        public void onThreadEnabledChanged(int state) {
-            mHandler.post(() -> onThreadEnabledChangedInternal(state));
-        }
-
-        private void onThreadEnabledChangedInternal(int state) {
+        private void onThreadEnabledChanged(int state, long listenerId) {
             checkOnHandlerThread();
-            for (IStateCallback callback : mStateCallbacks.keySet()) {
-                notifyThreadEnabledUpdated(callback, otStateToAndroidState(state));
+            boolean stateChanged = (mState == null || mState.threadEnabled != state);
+
+            for (var callbackEntry : mStateCallbacks.entrySet()) {
+                if (!stateChanged && callbackEntry.getValue().id != listenerId) {
+                    continue;
+                }
+                try {
+                    callbackEntry.getKey().onThreadEnableStateChanged(otStateToAndroidState(state));
+                } catch (RemoteException ignored) {
+                    // do nothing if the client is dead
+                }
             }
         }
 
@@ -1496,6 +1492,7 @@
             onInterfaceStateChanged(newState.isInterfaceUp);
             onDeviceRoleChanged(newState.deviceRole, listenerId);
             onPartitionIdChanged(newState.partitionId, listenerId);
+            onThreadEnabledChanged(newState.threadEnabled, listenerId);
             mState = newState;
 
             ActiveOperationalDataset newActiveDataset;

thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java

diff --git a/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java b/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java
index dea4279..11c4819 100644
--- a/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java
+++ b/thread/tests/cts/src/android/net/thread/cts/ThreadNetworkControllerTest.java
@@ -43,7 +43,6 @@
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
 
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
 
@@ -174,6 +173,17 @@
     }
 
     @Test
+    public void subscribeThreadEnableState_getActiveDataset_onThreadEnableStateChangedNotCalled()
+            throws Exception {
+        EnabledStateListener listener = new EnabledStateListener(mController);
+        listener.expectThreadEnabledState(STATE_ENABLED);
+
+        getActiveOperationalDataset(mController);
+
+        listener.expectCallbackNotCalled();
+    }
+
+    @Test
     public void registerStateCallback_returnsUpdatedEnabledStates() throws Exception {
         CompletableFuture<Void> setFuture1 = new CompletableFuture<>();
         CompletableFuture<Void> setFuture2 = new CompletableFuture<>();
@@ -1016,7 +1026,11 @@
         }
 
         public void expectThreadEnabledState(int enabled) {
-            assertNotNull(mReadHead.poll(ENABLED_TIMEOUT_MILLIS, e -> (e == enabled)));
+            assertThat(mReadHead.poll(ENABLED_TIMEOUT_MILLIS, e -> (e == enabled))).isNotNull();
+        }
+
+        public void expectCallbackNotCalled() {
+            assertThat(mReadHead.poll(CALLBACK_TIMEOUT_MILLIS, e -> true)).isNull();
         }
 
         public void unregisterStateCallback() {