From e52df5b668125f69af33afe1ffd7e7e2d88bd4e4 Mon Sep 17 00:00:00 2001
From: lucaslin <lucaslin@google.com>
Date: Fri, 19 Mar 2021 19:11:18 +0800
Subject: Remove the usage of hidden API and simplify the design

- ConnectivityManager will be a part of mainline module, so its
hidden API cannot be called by external caller.
- There is no need to listen onNetworkSuspended() and
onNetworkResumed() because if the network is suspended, then
NET_CAPABILITY_NOT_SUSPENDED will be removed and
onCapabilitiesChanged() will be called and the network capabilities
will be different from the previous one. And when the network is
resumed, NET_CAPABILITY_NOT_SUSPENDED will be added back to the
network capabilities. So the current design can be simplified.

Bug: 182963397
Test: atest FrameworksVcnTests
Change-Id: Ib8f8ea1c95503c946b8229670512af2db8be3165
---
 .../server/vcn/UnderlyingNetworkTracker.java       | 30 +---------------------
 .../server/vcn/UnderlyingNetworkTrackerTest.java   | 16 +++++++++---
 2 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java b/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java
index 8dcc547508ec..6ff8d6f8ba94 100644
--- a/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java
+++ b/services/core/java/com/android/server/vcn/UnderlyingNetworkTracker.java
@@ -290,25 +290,6 @@ public class UnderlyingNetworkTracker {
         maybeNotifyCallback();
     }
 
-    private void handleNetworkSuspended(@NonNull Network network, boolean isSuspended) {
-        mVcnContext.ensureRunningOnLooperThread();
-
-        if (!isSameNetwork(mRecordInProgress, network)) {
-            Slog.wtf(TAG, "Invalid update to isSuspended");
-            return;
-        }
-
-        final NetworkCapabilities newCaps =
-                new NetworkCapabilities(mRecordInProgress.getNetworkCapabilities());
-        if (isSuspended) {
-            newCaps.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED);
-        } else {
-            newCaps.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED);
-        }
-
-        handleCapabilitiesChanged(network, newCaps);
-    }
-
     private void handlePropertiesChanged(
             @NonNull Network network, @NonNull LinkProperties linkProperties) {
         mVcnContext.ensureRunningOnLooperThread();
@@ -366,19 +347,10 @@ public class UnderlyingNetworkTracker {
         @Override
         public void onCapabilitiesChanged(
                 @NonNull Network network, @NonNull NetworkCapabilities networkCapabilities) {
+            if (networkCapabilities.equals(mRecordInProgress.getNetworkCapabilities())) return;
             handleCapabilitiesChanged(network, networkCapabilities);
         }
 
-        @Override
-        public void onNetworkSuspended(@NonNull Network network) {
-            handleNetworkSuspended(network, true /* isSuspended */);
-        }
-
-        @Override
-        public void onNetworkResumed(@NonNull Network network) {
-            handleNetworkSuspended(network, false /* isSuspended */);
-        }
-
         @Override
         public void onLinkPropertiesChanged(
                 @NonNull Network network, @NonNull LinkProperties linkProperties) {
diff --git a/tests/vcn/java/com/android/server/vcn/UnderlyingNetworkTrackerTest.java b/tests/vcn/java/com/android/server/vcn/UnderlyingNetworkTrackerTest.java
index 1ef1a61f17ea..631c862d2bfd 100644
--- a/tests/vcn/java/com/android/server/vcn/UnderlyingNetworkTrackerTest.java
+++ b/tests/vcn/java/com/android/server/vcn/UnderlyingNetworkTrackerTest.java
@@ -328,7 +328,7 @@ public class UnderlyingNetworkTrackerTest {
     public void testRecordTrackerCallbackNotifiedForNetworkSuspended() {
         RouteSelectionCallback cb = verifyRegistrationOnAvailableAndGetCallback();
 
-        cb.onNetworkSuspended(mNetwork);
+        cb.onCapabilitiesChanged(mNetwork, SUSPENDED_NETWORK_CAPABILITIES);
 
         UnderlyingNetworkRecord expectedRecord =
                 new UnderlyingNetworkRecord(
@@ -336,7 +336,11 @@ public class UnderlyingNetworkTrackerTest {
                         SUSPENDED_NETWORK_CAPABILITIES,
                         INITIAL_LINK_PROPERTIES,
                         false /* isBlocked */);
-        verify(mNetworkTrackerCb).onSelectedUnderlyingNetworkChanged(eq(expectedRecord));
+        verify(mNetworkTrackerCb, times(1)).onSelectedUnderlyingNetworkChanged(eq(expectedRecord));
+        // onSelectedUnderlyingNetworkChanged() won't be fired twice if network capabilities doesn't
+        // change.
+        cb.onCapabilitiesChanged(mNetwork, SUSPENDED_NETWORK_CAPABILITIES);
+        verify(mNetworkTrackerCb, times(1)).onSelectedUnderlyingNetworkChanged(eq(expectedRecord));
     }
 
     @Test
@@ -344,7 +348,7 @@ public class UnderlyingNetworkTrackerTest {
         RouteSelectionCallback cb =
                 verifyRegistrationOnAvailableAndGetCallback(SUSPENDED_NETWORK_CAPABILITIES);
 
-        cb.onNetworkResumed(mNetwork);
+        cb.onCapabilitiesChanged(mNetwork, INITIAL_NETWORK_CAPABILITIES);
 
         UnderlyingNetworkRecord expectedRecord =
                 new UnderlyingNetworkRecord(
@@ -352,7 +356,11 @@ public class UnderlyingNetworkTrackerTest {
                         INITIAL_NETWORK_CAPABILITIES,
                         INITIAL_LINK_PROPERTIES,
                         false /* isBlocked */);
-        verify(mNetworkTrackerCb).onSelectedUnderlyingNetworkChanged(eq(expectedRecord));
+        verify(mNetworkTrackerCb, times(1)).onSelectedUnderlyingNetworkChanged(eq(expectedRecord));
+        // onSelectedUnderlyingNetworkChanged() won't be fired twice if network capabilities doesn't
+        // change.
+        cb.onCapabilitiesChanged(mNetwork, INITIAL_NETWORK_CAPABILITIES);
+        verify(mNetworkTrackerCb, times(1)).onSelectedUnderlyingNetworkChanged(eq(expectedRecord));
     }
 
     @Test
-- 
cgit v1.2.3-59-g8ed1b