From fd23dbbf07475954fa34b6f46c802a026b4e81b8 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 25 Mar 2020 02:09:26 +0900 Subject: [PATCH 1/3] Update the NetworkProvider API for council comments Bug: 152238712 Test: FrameworksNetTests NetworkStackTests Change-Id: I6b086572cfc72a0727f4510351cff0e74cbc4302 --- core/java/android/net/NetworkProvider.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/core/java/android/net/NetworkProvider.java b/core/java/android/net/NetworkProvider.java index 418d6915d4..75086cf82b 100644 --- a/core/java/android/net/NetworkProvider.java +++ b/core/java/android/net/NetworkProvider.java @@ -16,6 +16,7 @@ package android.net; +import android.annotation.IntRange; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.RequiresPermission; @@ -33,8 +34,8 @@ import android.util.Log; * {@link NetworkAgent}s. The networks can then provide connectivity to apps and can be interacted * with via networking APIs such as {@link ConnectivityManager}. * - * Subclasses should implement {@link #onNetworkRequested} and {@link #onRequestWithdrawn} to - * receive {@link NetworkRequest}s sent by the system and by apps. A network that is not the + * Subclasses should implement {@link #onNetworkRequested} and {@link #onNetworkRequestWithdrawn} + * to receive {@link NetworkRequest}s sent by the system and by apps. A network that is not the * best (highest-scoring) network for any request is generally not used by the system, and torn * down. * @@ -77,7 +78,7 @@ public class NetworkProvider { * Constructs a new NetworkProvider. * * @param looper the Looper on which to run {@link #onNetworkRequested} and - * {@link #onRequestWithdrawn}. + * {@link #onNetworkRequestWithdrawn}. * @param name the name of the listener, used only for debugging. * * @hide @@ -94,7 +95,7 @@ public class NetworkProvider { onNetworkRequested((NetworkRequest) m.obj, m.arg1, m.arg2); break; case CMD_CANCEL_REQUEST: - onRequestWithdrawn((NetworkRequest) m.obj); + onNetworkRequestWithdrawn((NetworkRequest) m.obj); break; default: Log.e(mName, "Unhandled message: " + m.what); @@ -142,14 +143,15 @@ public class NetworkProvider { * @hide */ @SystemApi - public void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId) {} + public void onNetworkRequested(@NonNull NetworkRequest request, + @IntRange(from = 0, to = 99) int score, int providerId) {} /** * Called when a NetworkRequest is withdrawn. * @hide */ @SystemApi - public void onRequestWithdrawn(@NonNull NetworkRequest request) {} + public void onNetworkRequestWithdrawn(@NonNull NetworkRequest request) {} /** * Asserts that no provider will ever be able to satisfy the specified request. The provider @@ -157,7 +159,7 @@ public class NetworkProvider { * satisfying this request, and that the request cannot be satisfied. The application filing the * request will receive an {@link NetworkCallback#onUnavailable()} callback. * - * @param request the request that cannot be fulfilled + * @param request the request that permanently cannot be fulfilled * @hide */ @SystemApi From 812e8cb87e72169b71cfa9d7759f0da92529f2e3 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 27 Mar 2020 15:00:38 +0900 Subject: [PATCH 2/3] Address further API council comments. Test: FrameworksNetTests NetworkStackTests Bug: 152238712 Change-Id: I8a785ae0e74e659c317deaaa28c203356c7766ed --- core/java/android/net/NetworkAgent.java | 31 ++++++++++++++-------- core/java/android/net/SocketKeepalive.java | 3 ++- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index 8119df9217..fe90a84579 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/core/java/android/net/NetworkAgent.java @@ -36,6 +36,7 @@ import com.android.internal.util.Protocol; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.time.Duration; import java.util.ArrayList; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; @@ -47,7 +48,7 @@ import java.util.concurrent.atomic.AtomicBoolean; * An agent manages the life cycle of a network. A network starts its * life cycle when {@link register} is called on NetworkAgent. The network * is then connecting. When full L3 connectivity has been established, - * the agent shoud call {@link setConnected} to inform the system that + * the agent shoud call {@link markConnected} to inform the system that * this network is ready to use. When the network disconnects its life * ends and the agent should call {@link unregister}, at which point the * system will clean up and free resources. @@ -503,7 +504,8 @@ public abstract class NetworkAgent { break; } case CMD_START_SOCKET_KEEPALIVE: { - onStartSocketKeepalive(msg.arg1 /* slot */, msg.arg2 /* interval */, + onStartSocketKeepalive(msg.arg1 /* slot */, + Duration.ofSeconds(msg.arg2) /* interval */, (KeepalivePacketData) msg.obj /* packet */); break; } @@ -617,10 +619,10 @@ public abstract class NetworkAgent { * Inform ConnectivityService that this agent has now connected. * Call {@link #unregister} to disconnect. */ - public void setConnected() { + public void markConnected() { if (mIsLegacy) { throw new UnsupportedOperationException( - "Legacy agents can't call setConnected."); + "Legacy agents can't call markConnected."); } mNetworkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTED, null, null); queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, mNetworkInfo); @@ -798,8 +800,8 @@ public abstract class NetworkAgent { * {@code VALIDATION_STATUS_VALID} if Internet connectivity was validated, * {@code VALIDATION_STATUS_NOT_VALID} if Internet connectivity was not validated. * - * This may be called multiple times as network status changes, or if there are multiple - * subsequent attempts to validate connectivity that fail. + * This is guaranteed to be called again when the network status changes, but the system + * may also call this multiple times even if the status does not change. * * @param status one of {@code VALIDATION_STATUS_VALID} or {@code VALIDATION_STATUS_NOT_VALID}. * @param redirectUri If Internet connectivity is being redirected (e.g., on a captive portal), @@ -832,18 +834,25 @@ public abstract class NetworkAgent { * Requests that the network hardware send the specified packet at the specified interval. * * @param slot the hardware slot on which to start the keepalive. - * @param intervalSeconds the interval between packets + * @param interval the interval between packets, between 10 and 3600. Note that this API + * does not support sub-second precision and will round off the request. * @param packet the packet to send. */ // seconds is from SocketKeepalive.MIN_INTERVAL_SEC to MAX_INTERVAL_SEC, but these should // not be exposed as constants because they may change in the future (API guideline 4.8) // and should have getters if exposed at all. Getters can't be used in the annotation, // so the values unfortunately need to be copied. - public void onStartSocketKeepalive(int slot, - @IntRange(from = 10, to = 3600) int intervalSeconds, + public void onStartSocketKeepalive(int slot, @NonNull Duration interval, @NonNull KeepalivePacketData packet) { - Message msg = mHandler.obtainMessage(CMD_START_SOCKET_KEEPALIVE, slot, intervalSeconds, - packet); + final long intervalSeconds = interval.getSeconds(); + if (intervalSeconds < SocketKeepalive.MIN_INTERVAL_SEC + || intervalSeconds > SocketKeepalive.MAX_INTERVAL_SEC) { + throw new IllegalArgumentException("Interval needs to be comprised between " + + SocketKeepalive.MIN_INTERVAL_SEC + " and " + SocketKeepalive.MAX_INTERVAL_SEC + + " but was " + intervalSeconds); + } + final Message msg = mHandler.obtainMessage(CMD_START_SOCKET_KEEPALIVE, slot, + (int) intervalSeconds, packet); startSocketKeepalive(msg); msg.recycle(); } diff --git a/core/java/android/net/SocketKeepalive.java b/core/java/android/net/SocketKeepalive.java index 8ff8f4c48c..46aef10258 100644 --- a/core/java/android/net/SocketKeepalive.java +++ b/core/java/android/net/SocketKeepalive.java @@ -115,7 +115,8 @@ public abstract class SocketKeepalive implements AutoCloseable { SUCCESS, ERROR_INVALID_LENGTH, ERROR_UNSUPPORTED, - ERROR_INSUFFICIENT_RESOURCES + ERROR_INSUFFICIENT_RESOURCES, + ERROR_HARDWARE_UNSUPPORTED }) public @interface KeepaliveEvent {} From 51b8e287ad9964123217deeb8d78973476233874 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 27 Mar 2020 17:57:34 +0900 Subject: [PATCH 3/3] Rename satisfiedBy into canBeSatisfiedBy Test: FrameworksNetTests NetworkStackTests Bug: 152238712 Change-Id: I076876a6662bde143ef7d315ce3767acafff93c1 --- core/java/android/net/NetworkCapabilities.java | 2 +- core/java/android/net/NetworkRequest.java | 2 +- .../java/com/android/server/ConnectivityServiceTest.java | 9 ++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index af9414c566..91ef911ef2 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -1122,7 +1122,7 @@ public final class NetworkCapabilities implements Parcelable { } private boolean satisfiedBySpecifier(NetworkCapabilities nc) { - return mNetworkSpecifier == null || mNetworkSpecifier.satisfiedBy(nc.mNetworkSpecifier) + return mNetworkSpecifier == null || mNetworkSpecifier.canBeSatisfiedBy(nc.mNetworkSpecifier) || nc.mNetworkSpecifier instanceof MatchAllNetworkSpecifier; } diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index 798856d13b..a6bd74a587 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -474,7 +474,7 @@ public class NetworkRequest implements Parcelable { * @param nc Capabilities that should satisfy this NetworkRequest. null capabilities do not * satisfy any request. */ - public boolean satisfiedBy(@Nullable NetworkCapabilities nc) { + public boolean canBeSatisfiedBy(@Nullable NetworkCapabilities nc) { return networkCapabilities.satisfiedByNetworkCapabilities(nc); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 4bfb51bca2..83f4c9817d 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -2896,7 +2896,7 @@ public class ConnectivityServiceTest { class ConfidentialMatchAllNetworkSpecifier extends NetworkSpecifier implements Parcelable { @Override - public boolean satisfiedBy(NetworkSpecifier other) { + public boolean canBeSatisfiedBy(NetworkSpecifier other) { return true; } @@ -2924,7 +2924,7 @@ public class ConnectivityServiceTest { } @Override - public boolean satisfiedBy(NetworkSpecifier other) { + public boolean canBeSatisfiedBy(NetworkSpecifier other) { if (other instanceof LocalStringNetworkSpecifier) { return TextUtils.equals(mString, ((LocalStringNetworkSpecifier) other).mString); @@ -3045,7 +3045,10 @@ public class ConnectivityServiceTest { }); class NonParcelableSpecifier extends NetworkSpecifier { - public boolean satisfiedBy(NetworkSpecifier other) { return false; } + @Override + public boolean canBeSatisfiedBy(NetworkSpecifier other) { + return false; + } }; class ParcelableSpecifier extends NonParcelableSpecifier implements Parcelable { @Override public int describeContents() { return 0; }