Fix runtime restart when using aggregated net observers.

When observing network traffic on a per-iface basis (as opposed to a
per-iface+uid basis), do not attempt to redistribute VPN traffic,
because it is unnecessary and will lead to a crash further down the
stack.

This is a one-line functional change, but some comments have been
added throughout to clarify behavior.

Bug: 29390363
Change-Id: Icca13f181d7698ca3ddc92aa9248f06f130d49a3

3 files changed, 21 insertions, 6 deletions

diff --git a/services/core/java/com/android/server/net/NetworkStatsObservers.java b/services/core/java/com/android/server/net/NetworkStatsObservers.java
index ea361700faf4..336058453479 100644
--- a/services/core/java/com/android/server/net/NetworkStatsObservers.java
+++ b/services/core/java/com/android/server/net/NetworkStatsObservers.java
@@ -355,9 +355,10 @@ class NetworkStatsObservers {
         @Override
         protected void recordSample(StatsContext statsContext) {
             // Recorder does not need to be locked in this context since only the handler
-            // thread will update it
+            // thread will update it. We pass a null VPN array because usage is aggregated by uid
+            // for this snapshot, so VPN traffic can't be reattributed to responsible apps.
             mRecorder.recordSnapshotLocked(statsContext.mXtSnapshot, statsContext.mActiveIfaces,
-                    statsContext.mVpnArray, statsContext.mCurrentTime);
+                    null /* vpnArray */, statsContext.mCurrentTime);
         }
 
         /**
@@ -396,7 +397,8 @@ class NetworkStatsObservers {
         @Override
         protected void recordSample(StatsContext statsContext) {
             // Recorder does not need to be locked in this context since only the handler
-            // thread will update it
+            // thread will update it. We pass the VPN info so VPN traffic is reattributed to
+            // responsible apps.
             mRecorder.recordSnapshotLocked(statsContext.mUidSnapshot, statsContext.mActiveUidIfaces,
                     statsContext.mVpnArray, statsContext.mCurrentTime);
         }
diff --git a/services/core/java/com/android/server/net/NetworkStatsRecorder.java b/services/core/java/com/android/server/net/NetworkStatsRecorder.java
index 04dc917ec3ab..090a0762a4b5 100644
--- a/services/core/java/com/android/server/net/NetworkStatsRecorder.java
+++ b/services/core/java/com/android/server/net/NetworkStatsRecorder.java
@@ -22,6 +22,7 @@ import static android.net.TrafficStats.MB_IN_BYTES;
 import static android.text.format.DateUtils.YEAR_IN_MILLIS;
 import static com.android.internal.util.Preconditions.checkNotNull;
 
+import android.annotation.Nullable;
 import android.net.NetworkStats;
 import android.net.NetworkStats.NonMonotonicObserver;
 import android.net.NetworkStatsHistory;
@@ -199,9 +200,14 @@ public class NetworkStatsRecorder {
      * Record any delta that occurred since last {@link NetworkStats} snapshot,
      * using the given {@link Map} to identify network interfaces. First
      * snapshot is considered bootstrap, and is not counted as delta.
+     *
+     * @param vpnArray Optional info about the currently active VPN, if any. This is used to
+     *                 redistribute traffic from the VPN app to the underlying responsible apps.
+     *                 This should always be set to null if the provided snapshot is aggregated
+     *                 across all UIDs (e.g. contains UID_ALL buckets), regardless of VPN state.
      */
     public void recordSnapshotLocked(NetworkStats snapshot,
-            Map<String, NetworkIdentitySet> ifaceIdent, VpnInfo[] vpnArray,
+            Map<String, NetworkIdentitySet> ifaceIdent, @Nullable VpnInfo[] vpnArray,
             long currentTimeMillis) {
         final HashSet<String> unknownIfaces = Sets.newHashSet();
 
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java
index 8610fa1b7b49..23c111e56cde 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -1009,9 +1009,16 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         final NetworkStats xtSnapshot = mNetworkManager.getNetworkStatsSummaryXt();
         final NetworkStats devSnapshot = mNetworkManager.getNetworkStatsSummaryDev();
 
+
+        // For xt/dev, we pass a null VPN array because usage is aggregated by UID, so VPN traffic
+        // can't be reattributed to responsible apps.
+        mDevRecorder.recordSnapshotLocked(
+                devSnapshot, mActiveIfaces, null /* vpnArray */, currentTime);
+        mXtRecorder.recordSnapshotLocked(
+                xtSnapshot, mActiveIfaces, null /* vpnArray */, currentTime);
+
+        // For per-UID stats, pass the VPN info so VPN traffic is reattributed to responsible apps.
         VpnInfo[] vpnArray = mConnManager.getAllVpnInfo();
-        mDevRecorder.recordSnapshotLocked(devSnapshot, mActiveIfaces, null, currentTime);
-        mXtRecorder.recordSnapshotLocked(xtSnapshot, mActiveIfaces, null, currentTime);
         mUidRecorder.recordSnapshotLocked(uidSnapshot, mActiveUidIfaces, vpnArray, currentTime);
         mUidTagRecorder.recordSnapshotLocked(uidSnapshot, mActiveUidIfaces, vpnArray, currentTime);