Merge "[SP16] Address comments on aosp/1172143"

This commit is contained in:
Junyu Lai
2020-02-15 02:21:30 +00:00
committed by Gerrit Code Review
6 changed files with 28 additions and 24 deletions

View File

@@ -526,15 +526,17 @@ public class NetworkStatsManager {
} }
/** /**
* Registers a custom provider of {@link android.net.NetworkStats} to combine the network * Registers a custom provider of {@link android.net.NetworkStats} to provide network statistics
* statistics that cannot be seen by the kernel to system. To unregister, invoke * to the system. To unregister, invoke {@link NetworkStatsProviderCallback#unregister()}.
* {@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 tag a human readable identifier of the custom network stats provider. This is only
* @param provider a custom implementation of {@link AbstractNetworkStatsProvider} that needs to * used for debugging.
* be registered to the system. * @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 * @return a {@link NetworkStatsProviderCallback}, which can be used to report events to the
* system. * system or unregister the provider.
* @hide * @hide
*/ */
@SystemApi @SystemApi

View File

@@ -130,7 +130,7 @@ public abstract class NetworkPolicyManagerInternal {
Set<String> packageNames, int userId); 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)}. * which was set through {@link AbstractNetworkStatsProvider#setLimit(String, long)}.
* *
* @param tag the human readable identifier of the custom network stats provider. * @param tag the human readable identifier of the custom network stats provider.

View File

@@ -4613,13 +4613,13 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
final long quota = ((long) msg.arg1 << 32) | (msg.arg2 & 0xFFFFFFFFL); final long quota = ((long) msg.arg1 << 32) | (msg.arg2 & 0xFFFFFFFFL);
removeInterfaceQuota(iface); removeInterfaceQuota(iface);
setInterfaceQuota(iface, quota); setInterfaceQuota(iface, quota);
mNetworkStats.setStatsProviderLimit(iface, quota); mNetworkStats.setStatsProviderLimitAsync(iface, quota);
return true; return true;
} }
case MSG_REMOVE_INTERFACE_QUOTA: { case MSG_REMOVE_INTERFACE_QUOTA: {
final String iface = (String) msg.obj; final String iface = (String) msg.obj;
removeInterfaceQuota(iface); removeInterfaceQuota(iface);
mNetworkStats.setStatsProviderLimit(iface, QUOTA_UNLIMITED); mNetworkStats.setStatsProviderLimitAsync(iface, QUOTA_UNLIMITED);
return true; return true;
} }
case MSG_RESET_FIREWALL_RULES_BY_UID: { case MSG_RESET_FIREWALL_RULES_BY_UID: {

View File

@@ -40,5 +40,5 @@ public abstract class NetworkStatsManagerInternal {
* Set the quota limit to all registered custom network stats providers. * Set the quota limit to all registered custom network stats providers.
* Note that invocation of any interface will be sent to all 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);
} }

View File

@@ -257,7 +257,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
} }
private final Object mStatsLock = new Object(); private final Object mStatsLock = new Object();
private final Object mStatsProviderLock = new Object();
/** Set of currently active ifaces. */ /** Set of currently active ifaces. */
@GuardedBy("mStatsLock") @GuardedBy("mStatsLock")
@@ -1521,8 +1520,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
} }
@Override @Override
public void setStatsProviderLimit(@NonNull String iface, long quota) { public void setStatsProviderLimitAsync(@NonNull String iface, long quota) {
Slog.v(TAG, "setStatsProviderLimit(" + iface + "," + quota + ")"); Slog.v(TAG, "setStatsProviderLimitAsync(" + iface + "," + quota + ")");
invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.setLimit(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. * {@code unregister()} of the returned callback.
* *
* @param tag a human readable identifier of the custom network stats provider. * @param tag a human readable identifier of the custom network stats provider.
* @param provider the binder interface of * @param provider the {@link INetworkStatsProvider} binder corresponding to the
* {@link android.net.netstats.provider.AbstractNetworkStatsProvider} that * {@link android.net.netstats.provider.AbstractNetworkStatsProvider} to be
* needs to be registered to the system. * registered.
* *
* @return a binder interface of * @return a binder interface of
* {@link android.net.netstats.provider.NetworkStatsProviderCallback}, which can be * {@link android.net.netstats.provider.NetworkStatsProviderCallback}, which can be
@@ -1844,7 +1843,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
private void invokeForAllStatsProviderCallbacks( private void invokeForAllStatsProviderCallbacks(
@NonNull ThrowingConsumer<NetworkStatsProviderCallbackImpl, RemoteException> task) { @NonNull ThrowingConsumer<NetworkStatsProviderCallbackImpl, RemoteException> task) {
synchronized (mStatsProviderCbList) { synchronized (mStatsLock) {
final int length = mStatsProviderCbList.beginBroadcast(); final int length = mStatsProviderCbList.beginBroadcast();
try { try {
for (int i = 0; i < length; i++) { for (int i = 0; i < length; i++) {
@@ -1865,14 +1864,16 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
private static class NetworkStatsProviderCallbackImpl extends INetworkStatsProviderCallback.Stub private static class NetworkStatsProviderCallbackImpl extends INetworkStatsProviderCallback.Stub
implements IBinder.DeathRecipient { implements IBinder.DeathRecipient {
@NonNull final String mTag; @NonNull final String mTag;
@NonNull private final Object mProviderStatsLock = new Object();
@NonNull final INetworkStatsProvider mProvider; @NonNull final INetworkStatsProvider mProvider;
@NonNull private final Semaphore mSemaphore; @NonNull private final Semaphore mSemaphore;
@NonNull final INetworkManagementEventObserver mAlertObserver; @NonNull final INetworkManagementEventObserver mAlertObserver;
@NonNull final RemoteCallbackList<NetworkStatsProviderCallbackImpl> mStatsProviderCbList; @NonNull final RemoteCallbackList<NetworkStatsProviderCallbackImpl> mStatsProviderCbList;
@NonNull private final Object mProviderStatsLock = new Object();
@GuardedBy("mProviderStatsLock") @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); private final NetworkStats mIfaceStats = new NetworkStats(0L, 0);
@GuardedBy("mProviderStatsLock") @GuardedBy("mProviderStatsLock")
private final NetworkStats mUidStats = new NetworkStats(0L, 0); private final NetworkStats mUidStats = new NetworkStats(0L, 0);
@@ -1905,7 +1906,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
default: default:
throw new IllegalArgumentException("Invalid type: " + how); 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(); return stats.clone();
} }
} }

View File

@@ -1702,7 +1702,7 @@ public class NetworkPolicyManagerServiceTest {
// Get active mobile network in place // Get active mobile network in place
expectMobileDefaults(); expectMobileDefaults();
mService.updateNetworks(); mService.updateNetworks();
verify(mStatsService).setStatsProviderLimit(TEST_IFACE, Long.MAX_VALUE); verify(mStatsService).setStatsProviderLimitAsync(TEST_IFACE, Long.MAX_VALUE);
// Set limit to 10KB. // Set limit to 10KB.
setNetworkPolicies(new NetworkPolicy( setNetworkPolicies(new NetworkPolicy(
@@ -1711,7 +1711,7 @@ public class NetworkPolicyManagerServiceTest {
postMsgAndWaitForCompletion(); postMsgAndWaitForCompletion();
// Verifies that remaining quota is set to providers. // Verifies that remaining quota is set to providers.
verify(mStatsService).setStatsProviderLimit(TEST_IFACE, 10000L - 4999L); verify(mStatsService).setStatsProviderLimitAsync(TEST_IFACE, 10000L - 4999L);
reset(mStatsService); reset(mStatsService);
@@ -1733,7 +1733,7 @@ public class NetworkPolicyManagerServiceTest {
postMsgAndWaitForCompletion(); postMsgAndWaitForCompletion();
verify(mStatsService).forceUpdate(); verify(mStatsService).forceUpdate();
postMsgAndWaitForCompletion(); postMsgAndWaitForCompletion();
verify(mStatsService).setStatsProviderLimit(TEST_IFACE, 10000L - 4999L - 1999L); verify(mStatsService).setStatsProviderLimitAsync(TEST_IFACE, 10000L - 4999L - 1999L);
} }
/** /**