From 2e98f6cd57b36ec4916fdd39e36fcffdf927569a Mon Sep 17 00:00:00 2001 From: Chiachang Wang Date: Tue, 20 Apr 2021 15:41:24 +0800 Subject: [PATCH] Address API review feedback Address API review feedback to: - Rename NetworkAgent#setTeardownDelayMs to NetworkAgent#setTeardownDelayMillis - Use getters instead of fields in VpnTransportInfo - Rename registerDefaultNetworkCallbackAsUid to registerDefaultNetworkCallbackForUid in ConnectiivityManager Bug: 183972850 Bug: 185246410 Fix: 184735863 Test: make update-api Test: atest FrameworksNetTests Test: atest CtsNetTestCasesLatestSdk Test: atest FrameworksMockingServicesTests Change-Id: I5e8c4bed8bda40d507afa894c359b5e24ee5d868 --- framework/api/module-lib-current.txt | 6 +-- framework/api/system-current.txt | 2 +- .../src/android/net/ConnectivityManager.java | 4 +- framework/src/android/net/NetworkAgent.java | 8 ++-- .../src/android/net/VpnTransportInfo.java | 37 +++++++++++++------ .../android/server/ConnectivityService.java | 2 +- .../android/net/ConnectivityManagerTest.java | 2 +- .../android/net/VpnTransportInfoTest.java | 2 +- .../server/ConnectivityServiceTest.java | 22 +++++------ .../android/server/connectivity/VpnTest.java | 2 +- 10 files changed, 51 insertions(+), 36 deletions(-) diff --git a/framework/api/module-lib-current.txt b/framework/api/module-lib-current.txt index c8b04a3f63..a8e2517a5e 100644 --- a/framework/api/module-lib-current.txt +++ b/framework/api/module-lib-current.txt @@ -11,7 +11,7 @@ package android.net { method @Nullable public android.net.ProxyInfo getGlobalProxy(); method @NonNull public static android.util.Range getIpSecNetIdRange(); method @NonNull public static String getPrivateDnsMode(@NonNull android.content.Context); - method @RequiresPermission(anyOf={android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_SETTINGS}) public void registerDefaultNetworkCallbackAsUid(int, @NonNull android.net.ConnectivityManager.NetworkCallback, @NonNull android.os.Handler); + method @RequiresPermission(anyOf={android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_SETTINGS}) public void registerDefaultNetworkCallbackForUid(int, @NonNull android.net.ConnectivityManager.NetworkCallback, @NonNull android.os.Handler); method @RequiresPermission(anyOf={android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_SETTINGS}) public void registerSystemDefaultNetworkCallback(@NonNull android.net.ConnectivityManager.NetworkCallback, @NonNull android.os.Handler); method @RequiresPermission(anyOf={android.Manifest.permission.NETWORK_SETTINGS, android.Manifest.permission.NETWORK_STACK, android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK}) public void requestBackgroundNetwork(@NonNull android.net.NetworkRequest, @NonNull android.net.ConnectivityManager.NetworkCallback, @NonNull android.os.Handler); method @Deprecated public boolean requestRouteToHostAddress(int, java.net.InetAddress); @@ -166,11 +166,11 @@ package android.net { public final class VpnTransportInfo implements android.os.Parcelable android.net.TransportInfo { ctor public VpnTransportInfo(int, @Nullable String); method public int describeContents(); + method @Nullable public String getSessionId(); + method public int getType(); method @NonNull public android.net.VpnTransportInfo makeCopy(long); method public void writeToParcel(@NonNull android.os.Parcel, int); field @NonNull public static final android.os.Parcelable.Creator CREATOR; - field @Nullable public final String sessionId; - field public final int type; } } diff --git a/framework/api/system-current.txt b/framework/api/system-current.txt index 3ca74756df..52673c9267 100644 --- a/framework/api/system-current.txt +++ b/framework/api/system-current.txt @@ -240,7 +240,7 @@ package android.net { method public final void sendSocketKeepaliveEvent(int, int); method @Deprecated public void setLegacySubtype(int, @NonNull String); method public void setLingerDuration(@NonNull java.time.Duration); - method public void setTeardownDelayMs(@IntRange(from=0, to=0x1388) int); + method public void setTeardownDelayMillis(@IntRange(from=0, to=0x1388) int); method public final void setUnderlyingNetworks(@Nullable java.util.List); method public void unregister(); field public static final int VALIDATION_STATUS_NOT_VALID = 2; // 0x2 diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java index b3c1997725..f116db52ba 100644 --- a/framework/src/android/net/ConnectivityManager.java +++ b/framework/src/android/net/ConnectivityManager.java @@ -4462,7 +4462,7 @@ public class ConnectivityManager { @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) public void registerDefaultNetworkCallback(@NonNull NetworkCallback networkCallback, @NonNull Handler handler) { - registerDefaultNetworkCallbackAsUid(Process.INVALID_UID, networkCallback, handler); + registerDefaultNetworkCallbackForUid(Process.INVALID_UID, networkCallback, handler); } /** @@ -4492,7 +4492,7 @@ public class ConnectivityManager { @RequiresPermission(anyOf = { NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_SETTINGS}) - public void registerDefaultNetworkCallbackAsUid(int uid, + public void registerDefaultNetworkCallbackForUid(int uid, @NonNull NetworkCallback networkCallback, @NonNull Handler handler) { CallbackHandler cbHandler = new CallbackHandler(handler); sendRequestForNetwork(uid, null /* need */, networkCallback, 0 /* timeoutMs */, diff --git a/framework/src/android/net/NetworkAgent.java b/framework/src/android/net/NetworkAgent.java index 518d3f39d1..adcf338ba1 100644 --- a/framework/src/android/net/NetworkAgent.java +++ b/framework/src/android/net/NetworkAgent.java @@ -892,11 +892,11 @@ public abstract class NetworkAgent { * This method may be called at any time while the network is connected. It has no effect if * the network is already disconnected and the teardown delay timer is running. * - * @param teardownDelayMs the teardown delay to set, or 0 to disable teardown delay. + * @param teardownDelayMillis the teardown delay to set, or 0 to disable teardown delay. */ - public void setTeardownDelayMs( - @IntRange(from = 0, to = MAX_TEARDOWN_DELAY_MS) int teardownDelayMs) { - queueOrSendMessage(reg -> reg.sendTeardownDelayMs(teardownDelayMs)); + public void setTeardownDelayMillis( + @IntRange(from = 0, to = MAX_TEARDOWN_DELAY_MS) int teardownDelayMillis) { + queueOrSendMessage(reg -> reg.sendTeardownDelayMs(teardownDelayMillis)); } /** diff --git a/framework/src/android/net/VpnTransportInfo.java b/framework/src/android/net/VpnTransportInfo.java index efd3363771..4071c9ab71 100644 --- a/framework/src/android/net/VpnTransportInfo.java +++ b/framework/src/android/net/VpnTransportInfo.java @@ -40,10 +40,10 @@ import java.util.Objects; @SystemApi(client = MODULE_LIBRARIES) public final class VpnTransportInfo implements TransportInfo, Parcelable { /** Type of this VPN. */ - public final int type; + private final int mType; @Nullable - public final String sessionId; + private final String mSessionId; @Override public @RedactionType long getApplicableRedactions() { @@ -55,13 +55,28 @@ public final class VpnTransportInfo implements TransportInfo, Parcelable { */ @NonNull public VpnTransportInfo makeCopy(@RedactionType long redactions) { - return new VpnTransportInfo(type, - ((redactions & REDACT_FOR_NETWORK_SETTINGS) != 0) ? null : sessionId); + return new VpnTransportInfo(mType, + ((redactions & REDACT_FOR_NETWORK_SETTINGS) != 0) ? null : mSessionId); } public VpnTransportInfo(int type, @Nullable String sessionId) { - this.type = type; - this.sessionId = sessionId; + this.mType = type; + this.mSessionId = sessionId; + } + + /** + * Returns the session Id of this VpnTransportInfo. + */ + @Nullable + public String getSessionId() { + return mSessionId; + } + + /** + * Returns the type of this VPN. + */ + public int getType() { + return mType; } @Override @@ -69,17 +84,17 @@ public final class VpnTransportInfo implements TransportInfo, Parcelable { if (!(o instanceof VpnTransportInfo)) return false; VpnTransportInfo that = (VpnTransportInfo) o; - return (this.type == that.type) && TextUtils.equals(this.sessionId, that.sessionId); + return (this.mType == that.mType) && TextUtils.equals(this.mSessionId, that.mSessionId); } @Override public int hashCode() { - return Objects.hash(type, sessionId); + return Objects.hash(mType, mSessionId); } @Override public String toString() { - return String.format("VpnTransportInfo{type=%d, sessionId=%s}", type, sessionId); + return String.format("VpnTransportInfo{type=%d, sessionId=%s}", mType, mSessionId); } @Override @@ -89,8 +104,8 @@ public final class VpnTransportInfo implements TransportInfo, Parcelable { @Override public void writeToParcel(@NonNull Parcel dest, int flags) { - dest.writeInt(type); - dest.writeString(sessionId); + dest.writeInt(mType); + dest.writeString(mSessionId); } public static final @NonNull Creator CREATOR = diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 809ef41b4d..8de74e1f21 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -8724,7 +8724,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (vpn == null) return VpnManager.TYPE_VPN_NONE; final TransportInfo ti = vpn.networkCapabilities.getTransportInfo(); if (!(ti instanceof VpnTransportInfo)) return VpnManager.TYPE_VPN_NONE; - return ((VpnTransportInfo) ti).type; + return ((VpnTransportInfo) ti).getType(); } /** diff --git a/tests/net/java/android/net/ConnectivityManagerTest.java b/tests/net/java/android/net/ConnectivityManagerTest.java index 19f884346e..591e0cc350 100644 --- a/tests/net/java/android/net/ConnectivityManagerTest.java +++ b/tests/net/java/android/net/ConnectivityManagerTest.java @@ -379,7 +379,7 @@ public class ConnectivityManagerTest { eq(testPkgName), eq(testAttributionTag)); reset(mService); - manager.registerDefaultNetworkCallbackAsUid(42, callback, handler); + manager.registerDefaultNetworkCallbackForUid(42, callback, handler); verify(mService).requestNetwork(eq(42), eq(null), eq(TRACK_DEFAULT.ordinal()), any(), anyInt(), any(), eq(TYPE_NONE), anyInt(), eq(testPkgName), eq(testAttributionTag)); diff --git a/tests/net/java/android/net/VpnTransportInfoTest.java b/tests/net/java/android/net/VpnTransportInfoTest.java index fee65f06bc..ccaa5cf7e9 100644 --- a/tests/net/java/android/net/VpnTransportInfoTest.java +++ b/tests/net/java/android/net/VpnTransportInfoTest.java @@ -63,6 +63,6 @@ public class VpnTransportInfoTest { assertEquals(v31, v32); assertEquals(v11.hashCode(), v13.hashCode()); assertEquals(REDACT_FOR_NETWORK_SETTINGS, v32.getApplicableRedactions()); - assertEquals(session1, v15.makeCopy(REDACT_NONE).sessionId); + assertEquals(session1, v15.makeCopy(REDACT_NONE).getSessionId()); } } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 7ebed39675..0aa2e9469e 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -1387,7 +1387,7 @@ public class ConnectivityServiceTest { final TransportInfo ti = nc.getTransportInfo(); assertTrue("VPN TransportInfo is not a VpnTransportInfo: " + ti, ti instanceof VpnTransportInfo); - assertEquals(type, ((VpnTransportInfo) ti).type); + assertEquals(type, ((VpnTransportInfo) ti).getType()); } @@ -2896,7 +2896,7 @@ public class ConnectivityServiceTest { callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); // Set teardown delay and make sure CS has processed it. - mWiFiNetworkAgent.getNetworkAgent().setTeardownDelayMs(300); + mWiFiNetworkAgent.getNetworkAgent().setTeardownDelayMillis(300); waitForIdle(); // Post the duringTeardown lambda to the handler so it fires while teardown is in progress. @@ -4253,7 +4253,7 @@ public class ConnectivityServiceTest { () -> mCm.registerSystemDefaultNetworkCallback(callback, handler)); callback.assertNoCallback(); assertThrows(SecurityException.class, - () -> mCm.registerDefaultNetworkCallbackAsUid(APP1_UID, callback, handler)); + () -> mCm.registerDefaultNetworkCallbackForUid(APP1_UID, callback, handler)); callback.assertNoCallback(); mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED); @@ -4261,7 +4261,7 @@ public class ConnectivityServiceTest { callback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); mCm.unregisterNetworkCallback(callback); - mCm.registerDefaultNetworkCallbackAsUid(APP1_UID, callback, handler); + mCm.registerDefaultNetworkCallbackForUid(APP1_UID, callback, handler); callback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); mCm.unregisterNetworkCallback(callback); } @@ -5685,7 +5685,7 @@ public class ConnectivityServiceTest { for (int i = 0; i < SYSTEM_ONLY_MAX_REQUESTS - 1; i++) { NetworkCallback cb = new NetworkCallback(); if (i % 2 == 0) { - mCm.registerDefaultNetworkCallbackAsUid(1000000 + i, cb, handler); + mCm.registerDefaultNetworkCallbackForUid(1000000 + i, cb, handler); } else { mCm.registerNetworkCallback(networkRequest, cb); } @@ -5694,7 +5694,7 @@ public class ConnectivityServiceTest { waitForIdle(); assertThrows(TooManyRequestsException.class, () -> - mCm.registerDefaultNetworkCallbackAsUid(1001042, new NetworkCallback(), + mCm.registerDefaultNetworkCallbackForUid(1001042, new NetworkCallback(), handler)); assertThrows(TooManyRequestsException.class, () -> mCm.registerNetworkCallback(networkRequest, new NetworkCallback())); @@ -5747,7 +5747,7 @@ public class ConnectivityServiceTest { withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, () -> { for (int i = 0; i < MAX_REQUESTS; i++) { NetworkCallback networkCallback = new NetworkCallback(); - mCm.registerDefaultNetworkCallbackAsUid(1000000 + i, networkCallback, + mCm.registerDefaultNetworkCallbackForUid(1000000 + i, networkCallback, new Handler(ConnectivityThread.getInstanceLooper())); mCm.unregisterNetworkCallback(networkCallback); } @@ -7831,7 +7831,7 @@ public class ConnectivityServiceTest { registerDefaultNetworkCallbackAsUid(vpnUidDefaultCallback, VPN_UID); final TestNetworkCallback vpnDefaultCallbackAsUid = new TestNetworkCallback(); - mCm.registerDefaultNetworkCallbackAsUid(VPN_UID, vpnDefaultCallbackAsUid, + mCm.registerDefaultNetworkCallbackForUid(VPN_UID, vpnDefaultCallbackAsUid, new Handler(ConnectivityThread.getInstanceLooper())); final int uid = Process.myUid(); @@ -10993,7 +10993,7 @@ public class ConnectivityServiceTest { final TestNetworkCallback otherUidDefaultCallback = new TestNetworkCallback(); withPermission(NETWORK_SETTINGS, () -> - mCm.registerDefaultNetworkCallbackAsUid(TEST_PACKAGE_UID, otherUidDefaultCallback, + mCm.registerDefaultNetworkCallbackForUid(TEST_PACKAGE_UID, otherUidDefaultCallback, new Handler(ConnectivityThread.getInstanceLooper()))); // Setup the test process to use networkPref for their default network. @@ -11041,7 +11041,7 @@ public class ConnectivityServiceTest { final TestNetworkCallback otherUidDefaultCallback = new TestNetworkCallback(); withPermission(NETWORK_SETTINGS, () -> - mCm.registerDefaultNetworkCallbackAsUid(TEST_PACKAGE_UID, otherUidDefaultCallback, + mCm.registerDefaultNetworkCallbackForUid(TEST_PACKAGE_UID, otherUidDefaultCallback, new Handler(ConnectivityThread.getInstanceLooper()))); // Bring up ethernet with OEM_PAID. This will satisfy NET_CAPABILITY_OEM_PAID. @@ -11083,7 +11083,7 @@ public class ConnectivityServiceTest { final TestNetworkCallback otherUidDefaultCallback = new TestNetworkCallback(); withPermission(NETWORK_SETTINGS, () -> - mCm.registerDefaultNetworkCallbackAsUid(TEST_PACKAGE_UID, otherUidDefaultCallback, + mCm.registerDefaultNetworkCallbackForUid(TEST_PACKAGE_UID, otherUidDefaultCallback, new Handler(ConnectivityThread.getInstanceLooper()))); // Setup a process different than the test process to use the default network. This means diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 6ad4900989..b725b826b1 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -1023,7 +1023,7 @@ public class VpnTest { assertNotNull(nc); VpnTransportInfo ti = (VpnTransportInfo) nc.getTransportInfo(); assertNotNull(ti); - assertEquals(type, ti.type); + assertEquals(type, ti.getType()); } public void startRacoon(final String serverAddr, final String expectedAddr)