Merge "[MS68.3] Address comments at aosp/1958144"

packages/ConnectivityT/framework-t/src/android/app/usage/NetworkStatsManager.java

@@ -744,8 +744,9 @@ public class NetworkStatsManager {
      * {@link #unregisterUsageCallback} is called.
      *
      * @param template Template used to match networks. See {@link NetworkTemplate}.
-     * @param thresholdBytes Threshold in bytes to be notified on. The provided value that lower
-     *                       than 2MiB will be clamped for non-privileged callers.
+     * @param thresholdBytes Threshold in bytes to be notified on. Provided values lower than 2MiB
+     *                       will be clamped for callers except callers with the NETWORK_STACK
+     *                       permission.
      * @param executor The executor on which callback will be invoked. The provided {@link Executor}
      *                 must run callback sequentially, otherwise the order of callbacks cannot be
      *                 guaranteed.
@@ -754,6 +755,9 @@ public class NetworkStatsManager {
      * @hide
      */
     @SystemApi(client = MODULE_LIBRARIES)
+    @RequiresPermission(anyOf = {
+            NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
+            android.Manifest.permission.NETWORK_STACK}, conditional = true)
     public void registerUsageCallback(@NonNull NetworkTemplate template, long thresholdBytes,
             @NonNull @CallbackExecutor Executor executor, @NonNull UsageCallback callback) {
         Objects.requireNonNull(template, "NetworkTemplate cannot be null");

packages/ConnectivityT/service/src/com/android/server/net/NetworkStatsObservers.java

@@ -19,8 +19,11 @@ package com.android.server.net;
 import static android.app.usage.NetworkStatsManager.MIN_THRESHOLD_BYTES;
 
 import android.app.usage.NetworkStatsManager;
+import android.content.Context;
+import android.content.pm.PackageManager;
 import android.net.DataUsageRequest;
 import android.net.NetworkIdentitySet;
+import android.net.NetworkStack;
 import android.net.NetworkStats;
 import android.net.NetworkStatsAccess;
 import android.net.NetworkStatsCollection;
@@ -74,9 +77,9 @@ class NetworkStatsObservers {
      *
      * @return the normalized request wrapped within {@link RequestInfo}.
      */
-    public DataUsageRequest register(DataUsageRequest inputRequest, IUsageCallback callback,
-            int callingUid, @NetworkStatsAccess.Level int accessLevel) {
-        DataUsageRequest request = buildRequest(inputRequest, callingUid);
+    public DataUsageRequest register(Context context, DataUsageRequest inputRequest,
+            IUsageCallback callback, int callingUid, @NetworkStatsAccess.Level int accessLevel) {
+        DataUsageRequest request = buildRequest(context, inputRequest, callingUid);
         RequestInfo requestInfo = buildRequestInfo(request, callback, callingUid,
                 accessLevel);
 
@@ -194,10 +197,13 @@ class NetworkStatsObservers {
         }
     }
 
-    private DataUsageRequest buildRequest(DataUsageRequest request, int callingUid) {
-        // For non-system uid, cap the minimum threshold to a safe default to avoid too
-        // many callbacks.
-        long thresholdInBytes = (callingUid == Process.SYSTEM_UID ? request.thresholdInBytes
+    private DataUsageRequest buildRequest(Context context, DataUsageRequest request,
+                int callingUid) {
+        // For non-NETWORK_STACK permission uid, cap the minimum threshold to a safe default to
+        // avoid too many callbacks.
+        final long thresholdInBytes = (context.checkPermission(
+                NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, Process.myPid(), callingUid)
+                == PackageManager.PERMISSION_GRANTED ? request.thresholdInBytes
                 : Math.max(MIN_THRESHOLD_BYTES, request.thresholdInBytes));
         if (thresholdInBytes > request.thresholdInBytes) {
             Log.w(TAG, "Threshold was too low for " + request

packages/ConnectivityT/service/src/com/android/server/net/NetworkStatsService.java

@@ -1285,7 +1285,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         DataUsageRequest normalizedRequest;
         final long token = Binder.clearCallingIdentity();
         try {
-            normalizedRequest = mStatsObservers.register(
+            normalizedRequest = mStatsObservers.register(mContext,
                     request, callback, callingUid, accessLevel);
         } finally {
             Binder.restoreCallingIdentity(token);

services/core/java/com/android/server/net/NetworkPolicyManagerService.java

@@ -281,6 +281,7 @@ import java.util.List;
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
 import java.util.function.IntConsumer;
 
@@ -1003,10 +1004,11 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
             mContext.registerReceiver(mUserReceiver, userFilter, null, mHandler);
 
             // listen for stats updated callbacks for interested network types.
+            final Executor executor = new HandlerExecutor(mHandler);
             mNetworkStats.registerUsageCallback(new NetworkTemplate.Builder(MATCH_MOBILE).build(),
-                    0 /* thresholdBytes */, new HandlerExecutor(mHandler), mStatsCallback);
+                    0 /* thresholdBytes */, executor, mStatsCallback);
             mNetworkStats.registerUsageCallback(new NetworkTemplate.Builder(MATCH_WIFI).build(),
-                    0 /* thresholdBytes */, new HandlerExecutor(mHandler), mStatsCallback);
+                    0 /* thresholdBytes */, executor, mStatsCallback);
 
             // listen for restrict background changes from notifications
             final IntentFilter allowFilter = new IntentFilter(ACTION_ALLOW_BACKGROUND);
@@ -1234,6 +1236,12 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
          * Used to determine if NetworkStatsService is ready.
          */
         public boolean isAnyCallbackReceived() {
+            // Warning : threading for this member is broken. It should only be read
+            // and written on the handler thread ; furthermore, the constructor
+            // is called on a different thread, so this stops working if the default
+            // value is not false or if this member ever goes back to false after
+            // being set to true.
+            // TODO : fix threading for this member.
             return mIsAnyCallbackReceived;
         }
     };

services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java

@@ -504,6 +504,7 @@ public class NetworkPolicyManagerServiceTest {
                 ArgumentCaptor.forClass(NetworkStatsManager.UsageCallback.class);
         verify(mStatsManager, times(2))
                 .registerUsageCallback(any(), anyLong(), any(), usageObserver.capture());
+        // It doesn't matter which of the observers is returned here.
         usageObserver.getValue().onThresholdReached(
                 new NetworkTemplate.Builder(MATCH_MOBILE).build());