From 9bd55639ba67c8da80ea6d136c128d313a6c23f5 Mon Sep 17 00:00:00 2001 From: David Su Date: Tue, 31 Mar 2020 23:33:26 -0700 Subject: Clean up WifiTrafficPoller Previously, WifiTrafficPoller only triggers callbacks if the new data activity is different from the last one. However, this can cause some unexpected behaviors. For example, if the data activity is constantly DATA_ACTIVITY_INOUT i.e. there's always inbound & outbound traffic, then if a new callback is registered, it wouldn't be triggered until the data activity changed, which could be a long time after. But, the client would reasonably expect to be notified for the first time soon after registration. Modified WifiTrafficPoller to instead keep a isFirstInvocation flag for each callback, and always trigger the callback if it is the first invocation, no matter if the data activity changed or not. Also cleaned up this class. Bug: 152633766 Test: atest WifiTrafficPollerTest Change-Id: Ide4d040bf8602d9e8b6b9b3715d8d6e58618b408 --- .../android/server/wifi/WifiTrafficPollerTest.java | 68 +++++++++++++++++++++- 1 file changed, 65 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/wifitests/src/com/android/server/wifi/WifiTrafficPollerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiTrafficPollerTest.java index 1c7afef54..ecdf490c4 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiTrafficPollerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiTrafficPollerTest.java @@ -19,6 +19,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import android.net.wifi.ITrafficStateCallback; @@ -30,8 +31,6 @@ import android.os.test.TestLooper; import androidx.test.filters.SmallTest; -import com.android.server.wifi.util.ExternalCallbackTracker; - import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -50,10 +49,13 @@ public class WifiTrafficPollerTest extends WifiBaseTest { private final static long TX_PACKET_COUNT = 40; private final static long RX_PACKET_COUNT = 50; private static final int TEST_TRAFFIC_STATE_CALLBACK_IDENTIFIER = 14; + private static final int TEST_TRAFFIC_STATE_CALLBACK_IDENTIFIER2 = 42; @Mock IBinder mAppBinder; @Mock ITrafficStateCallback mTrafficStateCallback; - @Mock ExternalCallbackTracker mCallbackTracker; + + @Mock IBinder mAppBinder2; + @Mock ITrafficStateCallback mTrafficStateCallback2; /** * Called before each test @@ -148,4 +150,64 @@ public class WifiTrafficPollerTest extends WifiBaseTest { // Client should not get any message callback add failed. verify(mTrafficStateCallback, never()).onStateChanged(anyInt()); } + + /** Test that if the data activity didn't change, the client is not notified. */ + @Test + public void unchangedDataActivityNotNotified() throws Exception { + mWifiTrafficPoller.addCallback( + mAppBinder, mTrafficStateCallback, TEST_TRAFFIC_STATE_CALLBACK_IDENTIFIER); + mWifiTrafficPoller.notifyOnDataActivity(TX_PACKET_COUNT, RX_PACKET_COUNT); + + verify(mTrafficStateCallback).onStateChanged( + WifiManager.TrafficStateCallback.DATA_ACTIVITY_INOUT); + + // since TX and RX both increased, should still be INOUT. But since it's the same data + // activity as before, the callback should not be triggered again. + mWifiTrafficPoller.notifyOnDataActivity(TX_PACKET_COUNT + 1, RX_PACKET_COUNT + 1); + + // still only called once + verify(mTrafficStateCallback).onStateChanged(anyInt()); + } + + /** + * If there are 2 callbacks, but the data activity only changed for one of them, only that + * callback should be triggered. + */ + @Test + public void multipleCallbacksOnlyChangedNotified() throws Exception { + mWifiTrafficPoller.addCallback( + mAppBinder, mTrafficStateCallback, TEST_TRAFFIC_STATE_CALLBACK_IDENTIFIER); + mWifiTrafficPoller.notifyOnDataActivity(TX_PACKET_COUNT, RX_PACKET_COUNT); + + verify(mTrafficStateCallback).onStateChanged( + WifiManager.TrafficStateCallback.DATA_ACTIVITY_INOUT); + verify(mTrafficStateCallback2, never()).onStateChanged(anyInt()); + + mWifiTrafficPoller.addCallback( + mAppBinder2, mTrafficStateCallback2, TEST_TRAFFIC_STATE_CALLBACK_IDENTIFIER2); + mWifiTrafficPoller.notifyOnDataActivity(TX_PACKET_COUNT + 1, RX_PACKET_COUNT + 1); + + // still only called once + verify(mTrafficStateCallback).onStateChanged(anyInt()); + // called for the first time with INOUT + verify(mTrafficStateCallback2) + .onStateChanged(WifiManager.TrafficStateCallback.DATA_ACTIVITY_INOUT); + // not called with anything else + verify(mTrafficStateCallback2).onStateChanged(anyInt()); + + // now only TX increased + mWifiTrafficPoller.notifyOnDataActivity(TX_PACKET_COUNT + 2, RX_PACKET_COUNT + 1); + + // called once with OUT + verify(mTrafficStateCallback) + .onStateChanged(WifiManager.TrafficStateCallback.DATA_ACTIVITY_OUT); + // called twice total + verify(mTrafficStateCallback, times(2)).onStateChanged(anyInt()); + + // called once with OUT + verify(mTrafficStateCallback2) + .onStateChanged(WifiManager.TrafficStateCallback.DATA_ACTIVITY_OUT); + // called twice total + verify(mTrafficStateCallback2, times(2)).onStateChanged(anyInt()); + } } -- cgit v1.2.3