[SP21] Address comments for API council review about aosp/1172143

Test: atest FrameworksNetTests ImsPhoneCallTrackerTest
Test: atest TetheringTests NetworkStackTests
Test: m doc-comment-check-docs
Fix: 148552904

Change-Id: I141393f229e772d2eb9f7c156849e379bd71b845

core/java/android/app/usage/NetworkStatsManager.java

@@ -31,9 +31,8 @@ import android.net.INetworkStatsService;
 import android.net.NetworkIdentity;
 import android.net.NetworkStack;
 import android.net.NetworkTemplate;
-import android.net.netstats.provider.AbstractNetworkStatsProvider;
+import android.net.netstats.provider.INetworkStatsProviderCallback;
-import android.net.netstats.provider.NetworkStatsProviderCallback;
+import android.net.netstats.provider.NetworkStatsProvider;
-import android.net.netstats.provider.NetworkStatsProviderWrapper;
 import android.os.Binder;
 import android.os.Handler;
 import android.os.Looper;
@@ -528,34 +527,53 @@ public class NetworkStatsManager {
 
     /**
      * Registers a custom provider of {@link android.net.NetworkStats} to provide network statistics
-     * to the system. To unregister, invoke {@link NetworkStatsProviderCallback#unregister()}.
+     * to the system. To unregister, invoke {@link #unregisterNetworkStatsProvider}.
      * 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.
+     * must only report network traffic that is not being reported by any other provider. Also note
+     * that the provider cannot be re-registered after unregistering.
      *
      * @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
+     * @param provider the subclass of {@link NetworkStatsProvider} that needs to be
      *                 registered to the system.
-     * @return a {@link NetworkStatsProviderCallback}, which can be used to report events to the
-     *         system or unregister the provider.
      * @hide
      */
     @SystemApi
     @RequiresPermission(anyOf = {
             android.Manifest.permission.NETWORK_STATS_PROVIDER,
             NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK})
-    @NonNull public NetworkStatsProviderCallback registerNetworkStatsProvider(
+    @NonNull public void registerNetworkStatsProvider(
             @NonNull String tag,
-            @NonNull AbstractNetworkStatsProvider provider) {
+            @NonNull NetworkStatsProvider provider) {
         try {
-            final NetworkStatsProviderWrapper wrapper = new NetworkStatsProviderWrapper(provider);
+            if (provider.getProviderCallbackBinder() != null) {
-            return new NetworkStatsProviderCallback(
+                throw new IllegalArgumentException("provider is already registered");
-                    mService.registerNetworkStatsProvider(tag, wrapper));
+            }
+            final INetworkStatsProviderCallback cbBinder =
+                    mService.registerNetworkStatsProvider(tag, provider.getProviderBinder());
+            provider.setProviderCallbackBinder(cbBinder);
+        } catch (RemoteException e) {
+            e.rethrowAsRuntimeException();
+        }
+    }
+
+    /**
+     * Unregisters an instance of {@link NetworkStatsProvider}.
+     *
+     * @param provider the subclass of {@link NetworkStatsProvider} that needs to be
+     *                 unregistered to the system.
+     * @hide
+     */
+    @SystemApi
+    @RequiresPermission(anyOf = {
+            android.Manifest.permission.NETWORK_STATS_PROVIDER,
+            NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK})
+    @NonNull public void unregisterNetworkStatsProvider(@NonNull NetworkStatsProvider provider) {
+        try {
+            provider.getProviderCallbackBinderOrThrow().unregister();
         } catch (RemoteException e) {
             e.rethrowAsRuntimeException();
         }
-        // Unreachable code, but compiler doesn't know about it.
-        return null;
     }
 
     private static NetworkTemplate createTemplate(int networkType, String subscriberId) {

core/java/android/net/netstats/provider/INetworkStatsProvider.aidl

@@ -22,7 +22,7 @@ package android.net.netstats.provider;
  * @hide
  */
 oneway interface INetworkStatsProvider {
-    void requestStatsUpdate(int token);
+    void onRequestStatsUpdate(int token);
-    void setLimit(String iface, long quotaBytes);
+    void onSetLimit(String iface, long quotaBytes);
-    void setAlert(long quotaBytes);
+    void onSetAlert(long quotaBytes);
 }

core/java/android/net/netstats/provider/INetworkStatsProviderCallback.aidl

@@ -24,8 +24,8 @@ import android.net.NetworkStats;
  * @hide
  */
 oneway interface INetworkStatsProviderCallback {
-    void onStatsUpdated(int token, in NetworkStats ifaceStats, in NetworkStats uidStats);
+    void notifyStatsUpdated(int token, in NetworkStats ifaceStats, in NetworkStats uidStats);
-    void onAlertReached();
+    void notifyAlertReached();
-    void onLimitReached();
+    void notifyLimitReached();
     void unregister();
 }

core/java/android/net/netstats/provider/NetworkStatsProvider.java

@@ -0,0 +1,195 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.net.netstats.provider;
+
+import android.annotation.NonNull;
+import android.annotation.Nullable;
+import android.annotation.SystemApi;
+import android.net.NetworkStats;
+import android.os.RemoteException;
+
+/**
+ * A base class that allows external modules to implement a custom network statistics provider.
+ * @hide
+ */
+@SystemApi
+public abstract class NetworkStatsProvider {
+    /**
+     * A value used by {@link #onSetLimit} and {@link #onSetAlert} indicates there is no limit.
+     */
+    public static final int QUOTA_UNLIMITED = -1;
+
+    @NonNull private final INetworkStatsProvider mProviderBinder =
+            new INetworkStatsProvider.Stub() {
+
+        @Override
+        public void onRequestStatsUpdate(int token) {
+            NetworkStatsProvider.this.onRequestStatsUpdate(token);
+        }
+
+        @Override
+        public void onSetLimit(String iface, long quotaBytes) {
+            NetworkStatsProvider.this.onSetLimit(iface, quotaBytes);
+        }
+
+        @Override
+        public void onSetAlert(long quotaBytes) {
+            NetworkStatsProvider.this.onSetAlert(quotaBytes);
+        }
+    };
+
+    // The binder given by the service when successfully registering. Only null before registering,
+    // never null once non-null.
+    @Nullable
+    private INetworkStatsProviderCallback mProviderCbBinder;
+
+    /**
+     * Return the binder invoked by the service and redirect function calls to the overridden
+     * methods.
+     * @hide
+     */
+    @NonNull
+    public INetworkStatsProvider getProviderBinder() {
+        return mProviderBinder;
+    }
+
+    /**
+     * Store the binder that was returned by the service when successfully registering. Note that
+     * the provider cannot be re-registered. Hence this method can only be called once per provider.
+     *
+     * @hide
+     */
+    public void setProviderCallbackBinder(@NonNull INetworkStatsProviderCallback binder) {
+        if (mProviderCbBinder != null) {
+            throw new IllegalArgumentException("provider is already registered");
+        }
+        mProviderCbBinder = binder;
+    }
+
+    /**
+     * Get the binder that was returned by the service when successfully registering. Or null if the
+     * provider was never registered.
+     *
+     * @hide
+     */
+    @Nullable
+    public INetworkStatsProviderCallback getProviderCallbackBinder() {
+        return mProviderCbBinder;
+    }
+
+    /**
+     * Get the binder that was returned by the service when successfully registering. Throw an
+     * {@link IllegalStateException} if the provider is not registered.
+     *
+     * @hide
+     */
+    @NonNull
+    public INetworkStatsProviderCallback getProviderCallbackBinderOrThrow() {
+        if (mProviderCbBinder == null) {
+            throw new IllegalStateException("the provider is not registered");
+        }
+        return mProviderCbBinder;
+    }
+
+    /**
+     * Notify the system of new network statistics.
+     *
+     * Send the network statistics recorded since the last call to {@link #notifyStatsUpdated}. Must
+     * be called as soon as possible after {@link NetworkStatsProvider#onRequestStatsUpdate(int)}
+     * being called. Responding later increases the probability stats will be dropped. The
+     * provider can also call this whenever it wants to reports new stats for any reason.
+     * Note that the system will not necessarily immediately propagate the statistics to
+     * reflect the update.
+     *
+     * @param token the token under which these stats were gathered. Providers can call this method
+     *              with the current token as often as they want, until the token changes.
+     *              {@see NetworkStatsProvider#onRequestStatsUpdate()}
+     * @param ifaceStats the {@link NetworkStats} per interface to be reported.
+     *                   The provider should not include any traffic that is already counted by
+     *                   kernel interface counters.
+     * @param uidStats the same stats as above, but counts {@link NetworkStats}
+     *                 per uid.
+     */
+    public void notifyStatsUpdated(int token, @NonNull NetworkStats ifaceStats,
+            @NonNull NetworkStats uidStats) {
+        try {
+            getProviderCallbackBinderOrThrow().notifyStatsUpdated(token, ifaceStats, uidStats);
+        } catch (RemoteException e) {
+            e.rethrowAsRuntimeException();
+        }
+    }
+
+    /**
+     * Notify system that the quota set by {@code onSetAlert} has been reached.
+     */
+    public void notifyAlertReached() {
+        try {
+            getProviderCallbackBinderOrThrow().notifyAlertReached();
+        } catch (RemoteException e) {
+            e.rethrowAsRuntimeException();
+        }
+    }
+
+    /**
+     * Notify system that the quota set by {@code onSetLimit} has been reached.
+     */
+    public void notifyLimitReached() {
+        try {
+            getProviderCallbackBinderOrThrow().notifyLimitReached();
+        } catch (RemoteException e) {
+            e.rethrowAsRuntimeException();
+        }
+    }
+
+    /**
+     * Called by {@code NetworkStatsService} when it requires to know updated stats.
+     * The provider MUST respond by calling {@link #notifyStatsUpdated} as soon as possible.
+     * Responding later increases the probability stats will be dropped. Memory allowing, the
+     * system will try to take stats into account up to one minute after calling
+     * {@link #onRequestStatsUpdate}.
+     *
+     * @param token a positive number identifying the new state of the system under which
+     *              {@link NetworkStats} have to be gathered from now on. When this is called,
+     *              custom implementations of providers MUST tally and report the latest stats with
+     *              the previous token, under which stats were being gathered so far.
+     */
+    public abstract void onRequestStatsUpdate(int token);
+
+    /**
+     * Called by {@code NetworkStatsService} when setting the interface quota for the specified
+     * upstream interface. When this is called, the custom implementation should block all egress
+     * packets on the {@code iface} associated with the provider when {@code quotaBytes} bytes have
+     * been reached, and MUST respond to it by calling
+     * {@link NetworkStatsProvider#notifyLimitReached()}.
+     *
+     * @param iface the interface requiring the operation.
+     * @param quotaBytes the quota defined as the number of bytes, starting from zero and counting
+     *                   from now. A value of {@link #QUOTA_UNLIMITED} indicates there is no limit.
+     */
+    public abstract void onSetLimit(@NonNull String iface, long quotaBytes);
+
+    /**
+     * Called by {@code NetworkStatsService} when setting the alert bytes. Custom implementations
+     * MUST call {@link NetworkStatsProvider#notifyAlertReached()} when {@code quotaBytes} bytes
+     * have been reached. Unlike {@link #onSetLimit(String, long)}, the custom implementation should
+     * not block all egress packets.
+     *
+     * @param quotaBytes the quota defined as the number of bytes, starting from zero and counting
+     *                   from now. A value of {@link #QUOTA_UNLIMITED} indicates there is no alert.
+     */
+    public abstract void onSetAlert(long quotaBytes);
+}

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

@@ -103,7 +103,7 @@ import android.net.NetworkTemplate;
 import android.net.TrafficStats;
 import android.net.netstats.provider.INetworkStatsProvider;
 import android.net.netstats.provider.INetworkStatsProviderCallback;
-import android.net.netstats.provider.NetworkStatsProviderCallback;
+import android.net.netstats.provider.NetworkStatsProvider;
 import android.os.BestClock;
 import android.os.Binder;
 import android.os.DropBoxManager;
@@ -558,7 +558,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         } catch (RemoteException e) {
             // ignored; service lives in system_server
         }
-        invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.setAlert(mGlobalAlertBytes));
+        invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.onSetAlert(mGlobalAlertBytes));
     }
 
     @Override
@@ -1376,7 +1376,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         Trace.traceBegin(TRACE_TAG_NETWORK, "provider.requestStatsUpdate");
         final int registeredCallbackCount = mStatsProviderCbList.getRegisteredCallbackCount();
         mStatsProviderSem.drainPermits();
-        invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.requestStatsUpdate(0 /* unused */));
+        invokeForAllStatsProviderCallbacks(
+                (cb) -> cb.mProvider.onRequestStatsUpdate(0 /* unused */));
         try {
             mStatsProviderSem.tryAcquire(registeredCallbackCount,
                     MAX_STATS_PROVIDER_POLL_WAIT_TIME_MS, TimeUnit.MILLISECONDS);
@@ -1551,7 +1552,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         @Override
         public void setStatsProviderLimitAsync(@NonNull String iface, long quota) {
             Slog.v(TAG, "setStatsProviderLimitAsync(" + iface + "," + quota + ")");
-            invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.setLimit(iface, quota));
+            invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.onSetLimit(iface, quota));
         }
     }
 
@@ -1820,12 +1821,10 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
      *
      * @param tag a human readable identifier of the custom network stats provider.
      * @param provider the {@link INetworkStatsProvider} binder corresponding to the
-     *                 {@link android.net.netstats.provider.AbstractNetworkStatsProvider} to be
+     *                 {@link NetworkStatsProvider} to be registered.
-     *                 registered.
      *
-     * @return a binder interface of
+     * @return a {@link INetworkStatsProviderCallback} binder
-     *         {@link android.net.netstats.provider.NetworkStatsProviderCallback}, which can be
+     *         interface, which can be used to report events to the system.
-     *         used to report events to the system.
      */
     public @NonNull INetworkStatsProviderCallback registerNetworkStatsProvider(
             @NonNull String tag, @NonNull INetworkStatsProvider provider) {
@@ -1931,7 +1930,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         }
 
         @Override
-        public void onStatsUpdated(int token, @Nullable NetworkStats ifaceStats,
+        public void notifyStatsUpdated(int token, @Nullable NetworkStats ifaceStats,
                 @Nullable NetworkStats uidStats) {
             // TODO: 1. Use token to map ifaces to correct NetworkIdentity.
             //       2. Store the difference and store it directly to the recorder.
@@ -1943,12 +1942,12 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         }
 
         @Override
-        public void onAlertReached() throws RemoteException {
+        public void notifyAlertReached() throws RemoteException {
             mAlertObserver.limitReached(LIMIT_GLOBAL_ALERT, null /* unused */);
         }
 
         @Override
-        public void onLimitReached() {
+        public void notifyLimitReached() {
             Log.d(TAG, mTag + ": onLimitReached");
             LocalServices.getService(NetworkPolicyManagerInternal.class)
                     .onStatsProviderLimitReached(mTag);