[SP16] Address comments on aosp/1172143
Test: atest FrameworksNetTests
Test: atest NetworkPolicyManagerServiceTest
Test: m doc-comment-check-docs
Bug: 130855321
Change-Id: Iccaab09f5b9668ec4a7249737c64a69cecb08d15
diff --git a/core/java/android/app/usage/NetworkStatsManager.java b/core/java/android/app/usage/NetworkStatsManager.java
index 9c4a8f4..5b98188 100644
--- a/core/java/android/app/usage/NetworkStatsManager.java
+++ b/core/java/android/app/usage/NetworkStatsManager.java
@@ -526,15 +526,17 @@
}
/**
- * 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 b24a938..563dcf7 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 @@
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 9760185..10cf250 100644
--- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
+++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
@@ -4613,13 +4613,13 @@
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 6d72cb5..0cb0bc2c 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 @@
* 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 415ccb8..2af6e34 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -255,7 +255,6 @@
}
private final Object mStatsLock = new Object();
- private final Object mStatsProviderLock = new Object();
/** Set of currently active ifaces. */
@GuardedBy("mStatsLock")
@@ -1501,8 +1500,8 @@
}
@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));
}
}
@@ -1783,9 +1782,9 @@
* {@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
@@ -1823,7 +1822,7 @@
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++) {
@@ -1844,13 +1843,15 @@
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 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);
@@ -1881,7 +1882,8 @@
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 2e58ad6..ece937a 100644
--- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java
@@ -1702,7 +1702,7 @@
// 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(
@@ -1711,7 +1711,7 @@
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);
@@ -1733,7 +1733,7 @@
postMsgAndWaitForCompletion();
verify(mStatsService).forceUpdate();
postMsgAndWaitForCompletion();
- verify(mStatsService).setStatsProviderLimit(TEST_IFACE, 10000L - 4999L - 1999L);
+ verify(mStatsService).setStatsProviderLimitAsync(TEST_IFACE, 10000L - 4999L - 1999L);
}
/**