Merge "[SP16] Address comments on aosp/1172143" am: fc9e4fbc08 am: dfb6a0fdb9 am: a08cdb380e

Change-Id: I4cff4d096f375210585ceae5eb27002b97a35226

6 files changed, 28 insertions, 24 deletions

diff --git a/core/java/android/app/usage/NetworkStatsManager.java b/core/java/android/app/usage/NetworkStatsManager.java
index 9c4a8f4fbe27..5b98188300c9 100644
--- a/core/java/android/app/usage/NetworkStatsManager.java
+++ b/core/java/android/app/usage/NetworkStatsManager.java
@@ -526,15 +526,17 @@ public class NetworkStatsManager {
     }
 
     /**
-     * Registers a custom provider of {@link android.net.NetworkStats} to combine the network
-     * statistics that cannot be seen by the kernel to system. To unregister, invoke
-     * {@link NetworkStatsProviderCallback#unregister()}.
+     * Registers a custom provider of {@link android.net.NetworkStats} to provide network statistics
+     * to the system. To unregister, invoke {@link NetworkStatsProviderCallback#unregister()}.
+     * Note that no de-duplication of statistics between providers is performed, so each provider
+     * must only report network traffic that is not being reported by any other provider.
      *
-     * @param tag a human readable identifier of the custom network stats provider.
-     * @param provider a custom implementation of {@link AbstractNetworkStatsProvider} that needs to
-     *                 be registered to the system.
+     * @param tag a human readable identifier of the custom network stats provider. This is only
+     *            used for debugging.
+     * @param provider the subclass of {@link AbstractNetworkStatsProvider} that needs to be
+     *                 registered to the system.
      * @return a {@link NetworkStatsProviderCallback}, which can be used to report events to the
-     *         system.
+     *         system or unregister the provider.
      * @hide
      */
     @SystemApi
diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerInternal.java b/services/core/java/com/android/server/net/NetworkPolicyManagerInternal.java
index b24a938ef7ea..563dcf7e1156 100644
--- a/services/core/java/com/android/server/net/NetworkPolicyManagerInternal.java
+++ b/services/core/java/com/android/server/net/NetworkPolicyManagerInternal.java
@@ -130,7 +130,7 @@ public abstract class NetworkPolicyManagerInternal {
             Set<String> packageNames, int userId);
 
     /**
-     *  Notifies that any of the {@link AbstractNetworkStatsProvider} has reached its quota
+     *  Notifies that the specified {@link AbstractNetworkStatsProvider} has reached its quota
      *  which was set through {@link AbstractNetworkStatsProvider#setLimit(String, long)}.
      *
      * @param tag the human readable identifier of the custom network stats provider.
diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
index bb954ab2b5e6..aacb46e939f6 100644
--- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
+++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
@@ -4588,13 +4588,13 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                     final long quota = ((long) msg.arg1 << 32) | (msg.arg2 & 0xFFFFFFFFL);
                     removeInterfaceQuota(iface);
                     setInterfaceQuota(iface, quota);
-                    mNetworkStats.setStatsProviderLimit(iface, quota);
+                    mNetworkStats.setStatsProviderLimitAsync(iface, quota);
                     return true;
                 }
                 case MSG_REMOVE_INTERFACE_QUOTA: {
                     final String iface = (String) msg.obj;
                     removeInterfaceQuota(iface);
-                    mNetworkStats.setStatsProviderLimit(iface, QUOTA_UNLIMITED);
+                    mNetworkStats.setStatsProviderLimitAsync(iface, QUOTA_UNLIMITED);
                     return true;
                 }
                 case MSG_RESET_FIREWALL_RULES_BY_UID: {
diff --git a/services/core/java/com/android/server/net/NetworkStatsManagerInternal.java b/services/core/java/com/android/server/net/NetworkStatsManagerInternal.java
index 6d72cb5ee345..0cb0bc2c0896 100644
--- a/services/core/java/com/android/server/net/NetworkStatsManagerInternal.java
+++ b/services/core/java/com/android/server/net/NetworkStatsManagerInternal.java
@@ -40,5 +40,5 @@ public abstract class NetworkStatsManagerInternal {
      * Set the quota limit to all registered custom network stats providers.
      * Note that invocation of any interface will be sent to all providers.
      */
-    public abstract void setStatsProviderLimit(@NonNull String iface, long quota);
+    public abstract void setStatsProviderLimitAsync(@NonNull String iface, long quota);
 }
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java
index 9fc2c274ce89..0662400dbb69 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -257,7 +257,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
     }
 
     private final Object mStatsLock = new Object();
-    private final Object mStatsProviderLock = new Object();
 
     /** Set of currently active ifaces. */
     @GuardedBy("mStatsLock")
@@ -1521,8 +1520,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         }
 
         @Override
-        public void setStatsProviderLimit(@NonNull String iface, long quota) {
-            Slog.v(TAG, "setStatsProviderLimit(" + iface + "," + quota + ")");
+        public void setStatsProviderLimitAsync(@NonNull String iface, long quota) {
+            Slog.v(TAG, "setStatsProviderLimitAsync(" + iface + "," + quota + ")");
             invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.setLimit(iface, quota));
         }
     }
@@ -1803,9 +1802,9 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
      * {@code unregister()} of the returned callback.
      *
      * @param tag a human readable identifier of the custom network stats provider.
-     * @param provider the binder interface of
-     *                 {@link android.net.netstats.provider.AbstractNetworkStatsProvider} that
-     *                 needs to be registered to the system.
+     * @param provider the {@link INetworkStatsProvider} binder corresponding to the
+     *                 {@link android.net.netstats.provider.AbstractNetworkStatsProvider} to be
+     *                 registered.
      *
      * @return a binder interface of
      *         {@link android.net.netstats.provider.NetworkStatsProviderCallback}, which can be
@@ -1844,7 +1843,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
 
     private void invokeForAllStatsProviderCallbacks(
             @NonNull ThrowingConsumer<NetworkStatsProviderCallbackImpl, RemoteException> task) {
-        synchronized (mStatsProviderCbList) {
+        synchronized (mStatsLock) {
             final int length = mStatsProviderCbList.beginBroadcast();
             try {
                 for (int i = 0; i < length; i++) {
@@ -1865,14 +1864,16 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
     private static class NetworkStatsProviderCallbackImpl extends INetworkStatsProviderCallback.Stub
             implements IBinder.DeathRecipient {
         @NonNull final String mTag;
-        @NonNull private final Object mProviderStatsLock = new Object();
+
         @NonNull final INetworkStatsProvider mProvider;
         @NonNull private final Semaphore mSemaphore;
         @NonNull final INetworkManagementEventObserver mAlertObserver;
         @NonNull final RemoteCallbackList<NetworkStatsProviderCallbackImpl> mStatsProviderCbList;
 
+        @NonNull private final Object mProviderStatsLock = new Object();
+
         @GuardedBy("mProviderStatsLock")
-        // STATS_PER_IFACE and STATS_PER_UID
+        // Track STATS_PER_IFACE and STATS_PER_UID separately.
         private final NetworkStats mIfaceStats = new NetworkStats(0L, 0);
         @GuardedBy("mProviderStatsLock")
         private final NetworkStats mUidStats = new NetworkStats(0L, 0);
@@ -1905,7 +1906,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
                     default:
                         throw new IllegalArgumentException("Invalid type: " + how);
                 }
-                // Return a defensive copy instead of local reference.
+                // Callers might be able to mutate the returned object. Return a defensive copy
+                // instead of local reference.
                 return stats.clone();
             }
         }
diff --git a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
index a1baf0e9ce05..7e3cfc8fb2a5 100644
--- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
@@ -1779,7 +1779,7 @@ public class NetworkPolicyManagerServiceTest {
         // Get active mobile network in place
         expectMobileDefaults();
         mService.updateNetworks();
-        verify(mStatsService).setStatsProviderLimit(TEST_IFACE, Long.MAX_VALUE);
+        verify(mStatsService).setStatsProviderLimitAsync(TEST_IFACE, Long.MAX_VALUE);
 
         // Set limit to 10KB.
         setNetworkPolicies(new NetworkPolicy(
@@ -1788,7 +1788,7 @@ public class NetworkPolicyManagerServiceTest {
         postMsgAndWaitForCompletion();
 
         // Verifies that remaining quota is set to providers.
-        verify(mStatsService).setStatsProviderLimit(TEST_IFACE, 10000L - 4999L);
+        verify(mStatsService).setStatsProviderLimitAsync(TEST_IFACE, 10000L - 4999L);
 
         reset(mStatsService);
 
@@ -1810,7 +1810,7 @@ public class NetworkPolicyManagerServiceTest {
         postMsgAndWaitForCompletion();
         verify(mStatsService).forceUpdate();
         postMsgAndWaitForCompletion();
-        verify(mStatsService).setStatsProviderLimit(TEST_IFACE, 10000L - 4999L - 1999L);
+        verify(mStatsService).setStatsProviderLimitAsync(TEST_IFACE, 10000L - 4999L - 1999L);
     }
 
     /**