From 8b577139ae4a51835cafd3cb8bc0cc658219a230 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 24 Jun 2019 13:28:04 +0900 Subject: [PATCH] Transfer ownership of NetworkStatsFactory to NetworkStatsService. NetworkStatsService is the only consumer of the data coming from NetworkStatsFactory, but the factory itself is instantiated and owned by NetworkManagementService. Move it into NetworkStatsService instead. Test: atest FrameworksNetTests Change-Id: I52df1e545c646927952579da22845a12d1d7563b --- .../android/os/INetworkManagementService.aidl | 21 -------- .../server/NetworkManagementService.java | 44 ---------------- .../server/net/NetworkStatsService.java | 51 +++++++++++++------ .../server/net/NetworkStatsServiceTest.java | 24 +++++---- 4 files changed, 49 insertions(+), 91 deletions(-) diff --git a/core/java/android/os/INetworkManagementService.aidl b/core/java/android/os/INetworkManagementService.aidl index 7f60b9cec2840..135138030e43f 100644 --- a/core/java/android/os/INetworkManagementService.aidl +++ b/core/java/android/os/INetworkManagementService.aidl @@ -245,27 +245,6 @@ interface INetworkManagementService ** DATA USAGE RELATED **/ - /** - * Return global network statistics summarized at an interface level, - * without any UID-level granularity. - */ - NetworkStats getNetworkStatsSummaryDev(); - NetworkStats getNetworkStatsSummaryXt(); - - /** - * Return detailed network statistics with UID-level granularity, - * including interface and tag details. - */ - NetworkStats getNetworkStatsDetail(); - - /** - * Return detailed network statistics for the requested UID and interfaces, - * including interface and tag details. - * @param uid UID to obtain statistics for, or {@link NetworkStats#UID_ALL}. - * @param ifaces Interfaces to obtain statistics for, or {@link NetworkStats#INTERFACES_ALL}. - */ - NetworkStats getNetworkStatsUidDetail(int uid, in String[] ifaces); - /** * Return summary of network statistics all tethering interfaces. */ diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java index 6d08b5ab9b87c..e29bf8d294dee 100644 --- a/services/core/java/com/android/server/NetworkManagementService.java +++ b/services/core/java/com/android/server/NetworkManagementService.java @@ -34,7 +34,6 @@ import static android.net.NetworkPolicyManager.FIREWALL_CHAIN_NAME_STANDBY; import static android.net.NetworkPolicyManager.FIREWALL_RULE_DEFAULT; import static android.net.NetworkStats.SET_DEFAULT; import static android.net.NetworkStats.STATS_PER_UID; -import static android.net.NetworkStats.TAG_ALL; import static android.net.NetworkStats.TAG_NONE; import static android.net.TrafficStats.UID_TETHERING; @@ -90,7 +89,6 @@ import com.android.internal.app.IBatteryStats; import com.android.internal.util.DumpUtils; import com.android.internal.util.HexDump; import com.android.internal.util.Preconditions; -import com.android.server.net.NetworkStatsFactory; import com.google.android.collect.Maps; @@ -164,8 +162,6 @@ public class NetworkManagementService extends INetworkManagementService.Stub { private final RemoteCallbackList mObservers = new RemoteCallbackList<>(); - private final NetworkStatsFactory mStatsFactory = new NetworkStatsFactory(); - @GuardedBy("mTetheringStatsProviders") private final HashMap mTetheringStatsProviders = Maps.newHashMap(); @@ -1206,36 +1202,6 @@ public class NetworkManagementService extends INetworkManagementService.Stub { } } - @Override - public NetworkStats getNetworkStatsSummaryDev() { - mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG); - try { - return mStatsFactory.readNetworkStatsSummaryDev(); - } catch (IOException e) { - throw new IllegalStateException(e); - } - } - - @Override - public NetworkStats getNetworkStatsSummaryXt() { - mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG); - try { - return mStatsFactory.readNetworkStatsSummaryXt(); - } catch (IOException e) { - throw new IllegalStateException(e); - } - } - - @Override - public NetworkStats getNetworkStatsDetail() { - mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG); - try { - return mStatsFactory.readNetworkStatsDetail(); - } catch (IOException e) { - throw new IllegalStateException(e); - } - } - @Override public void setInterfaceQuota(String iface, long quotaBytes) { mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG); @@ -1535,16 +1501,6 @@ public class NetworkManagementService extends INetworkManagementService.Stub { return true; } - @Override - public NetworkStats getNetworkStatsUidDetail(int uid, String[] ifaces) { - mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG); - try { - return mStatsFactory.readNetworkStatsDetail(uid, ifaces, TAG_ALL); - } catch (IOException e) { - throw new IllegalStateException(e); - } - } - private class NetdTetheringStatsProvider extends ITetheringStatsProvider.Stub { @Override public NetworkStats getTetherStats(int how) { diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index 5505828063ed8..e67b39137a4ea 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -180,6 +180,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private final Context mContext; private final INetworkManagementService mNetworkManager; + private final NetworkStatsFactory mStatsFactory; private final AlarmManager mAlarmManager; private final Clock mClock; private final TelephonyManager mTeleManager; @@ -329,8 +330,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { NetworkStatsService service = new NetworkStatsService(context, networkManager, alarmManager, wakeLock, getDefaultClock(), TelephonyManager.getDefault(), - new DefaultNetworkStatsSettings(context), new NetworkStatsObservers(), - getDefaultSystemDir(), getDefaultBaseDir()); + new DefaultNetworkStatsSettings(context), new NetworkStatsFactory(), + new NetworkStatsObservers(), getDefaultSystemDir(), getDefaultBaseDir()); service.registerLocalService(); HandlerThread handlerThread = new HandlerThread(TAG); @@ -347,7 +348,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { NetworkStatsService(Context context, INetworkManagementService networkManager, AlarmManager alarmManager, PowerManager.WakeLock wakeLock, Clock clock, TelephonyManager teleManager, NetworkStatsSettings settings, - NetworkStatsObservers statsObservers, File systemDir, File baseDir) { + NetworkStatsFactory factory, NetworkStatsObservers statsObservers, File systemDir, + File baseDir) { mContext = checkNotNull(context, "missing Context"); mNetworkManager = checkNotNull(networkManager, "missing INetworkManagementService"); mAlarmManager = checkNotNull(alarmManager, "missing AlarmManager"); @@ -355,6 +357,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mSettings = checkNotNull(settings, "missing NetworkStatsSettings"); mTeleManager = checkNotNull(teleManager, "missing TelephonyManager"); mWakeLock = checkNotNull(wakeLock, "missing WakeLock"); + mStatsFactory = checkNotNull(factory, "missing factory"); mStatsObservers = checkNotNull(statsObservers, "missing NetworkStatsObservers"); mSystemDir = checkNotNull(systemDir, "missing systemDir"); mBaseDir = checkNotNull(baseDir, "missing baseDir"); @@ -752,14 +755,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // TODO: switch to data layer stats once kernel exports // for now, read network layer stats and flatten across all ifaces - final long token = Binder.clearCallingIdentity(); - final NetworkStats networkLayer; - try { - networkLayer = mNetworkManager.getNetworkStatsUidDetail(uid, - NetworkStats.INTERFACES_ALL); - } finally { - Binder.restoreCallingIdentity(token); - } + final NetworkStats networkLayer = readNetworkStatsUidDetail(uid, INTERFACES_ALL, TAG_ALL); // splice in operation counts networkLayer.spliceOperationsFrom(mUidOperations); @@ -1190,7 +1186,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mobileIfaces.add(stackedIface); } - NetworkStatsFactory.noteStackedIface(stackedIface, baseIface); + mStatsFactory.noteStackedIface(stackedIface, baseIface); } } } @@ -1220,7 +1216,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { final NetworkStats xtSnapshot = getNetworkStatsXt(); Trace.traceEnd(TRACE_TAG_NETWORK); Trace.traceBegin(TRACE_TAG_NETWORK, "snapshotDev"); - final NetworkStats devSnapshot = mNetworkManager.getNetworkStatsSummaryDev(); + final NetworkStats devSnapshot = readNetworkStatsSummaryDev(); Trace.traceEnd(TRACE_TAG_NETWORK); // Tethering snapshot for dev and xt stats. Counts per-interface data from tethering stats @@ -1614,6 +1610,30 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } + private NetworkStats readNetworkStatsSummaryDev() { + try { + return mStatsFactory.readNetworkStatsSummaryDev(); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + + private NetworkStats readNetworkStatsSummaryXt() { + try { + return mStatsFactory.readNetworkStatsSummaryXt(); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + + private NetworkStats readNetworkStatsUidDetail(int uid, String[] ifaces, int tag) { + try { + return mStatsFactory.readNetworkStatsDetail(uid, ifaces, tag); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + /** * Return snapshot of current UID statistics, including any * {@link TrafficStats#UID_TETHERING}, video calling data usage, and {@link #mUidOperations} @@ -1624,8 +1644,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { */ private NetworkStats getNetworkStatsUidDetail(String[] ifaces) throws RemoteException { - final NetworkStats uidSnapshot = mNetworkManager.getNetworkStatsUidDetail(UID_ALL, - ifaces); + final NetworkStats uidSnapshot = readNetworkStatsUidDetail(UID_ALL, ifaces, TAG_ALL); // fold tethering stats and operations into uid snapshot final NetworkStats tetherSnapshot = getNetworkStatsTethering(STATS_PER_UID); @@ -1655,7 +1674,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { * Return snapshot of current XT statistics with video calling data usage statistics. */ private NetworkStats getNetworkStatsXt() throws RemoteException { - final NetworkStats xtSnapshot = mNetworkManager.getNetworkStatsSummaryXt(); + final NetworkStats xtSnapshot = readNetworkStatsSummaryXt(); final TelephonyManager telephonyManager = (TelephonyManager) mContext.getSystemService( Context.TELEPHONY_SERVICE); diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index adaef40b51e3d..51dcc3cae2190 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -37,6 +37,7 @@ import static android.net.NetworkStats.SET_DEFAULT; import static android.net.NetworkStats.SET_FOREGROUND; import static android.net.NetworkStats.STATS_PER_IFACE; import static android.net.NetworkStats.STATS_PER_UID; +import static android.net.NetworkStats.TAG_ALL; import static android.net.NetworkStats.TAG_NONE; import static android.net.NetworkStats.UID_ALL; import static android.net.NetworkStatsHistory.FIELD_ALL; @@ -154,6 +155,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { private File mStatsDir; private @Mock INetworkManagementService mNetManager; + private @Mock NetworkStatsFactory mStatsFactory; private @Mock NetworkStatsSettings mSettings; private @Mock IBinder mBinder; private @Mock AlarmManager mAlarmManager; @@ -189,8 +191,8 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { mService = new NetworkStatsService( mServiceContext, mNetManager, mAlarmManager, wakeLock, mClock, - TelephonyManager.getDefault(), mSettings, new NetworkStatsObservers(), - mStatsDir, getBaseDir(mStatsDir)); + TelephonyManager.getDefault(), mSettings, mStatsFactory, + new NetworkStatsObservers(), mStatsDir, getBaseDir(mStatsDir)); mHandlerThread = new HandlerThread("HandlerThread"); mHandlerThread.start(); Handler.Callback callback = new NetworkStatsService.HandlerCallback(mService); @@ -205,7 +207,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { mService.systemReady(); // Verify that system ready fetches realtime stats - verify(mNetManager).getNetworkStatsUidDetail(UID_ALL, INTERFACES_ALL); + verify(mStatsFactory).readNetworkStatsDetail(UID_ALL, INTERFACES_ALL, TAG_ALL); mSession = mService.openSession(); assertNotNull("openSession() failed", mSession); @@ -696,7 +698,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { incrementCurrentTime(HOUR_IN_MILLIS); expectDefaultSettings(); expectNetworkStatsSummary(buildEmptyStats()); - when(mNetManager.getNetworkStatsUidDetail(eq(UID_ALL), any())) + when(mStatsFactory.readNetworkStatsDetail(eq(UID_ALL), any(), eq(TAG_ALL))) .thenReturn(new NetworkStats(getElapsedRealtime(), 1) .addValues(uidStats)); when(mNetManager.getNetworkStatsTethering(STATS_PER_UID)) @@ -712,9 +714,10 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { // // Additionally, we should have one call from the above call to mService#getDetailedUidStats // with the augmented ifaceFilter - verify(mNetManager, times(2)).getNetworkStatsUidDetail(UID_ALL, INTERFACES_ALL); - verify(mNetManager, times(1)).getNetworkStatsUidDetail( - eq(UID_ALL), eq(NetworkStatsFactory.augmentWithStackedInterfaces(ifaceFilter))); + verify(mStatsFactory, times(2)).readNetworkStatsDetail(UID_ALL, INTERFACES_ALL, TAG_ALL); + verify(mStatsFactory, times(1)).readNetworkStatsDetail( + eq(UID_ALL), eq(NetworkStatsFactory.augmentWithStackedInterfaces(ifaceFilter)), + eq(TAG_ALL)); assertTrue(ArrayUtils.contains(stats.getUniqueIfaces(), TEST_IFACE)); assertTrue(ArrayUtils.contains(stats.getUniqueIfaces(), stackedIface)); assertEquals(2, stats.size()); @@ -1062,11 +1065,11 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { } private void expectNetworkStatsSummaryDev(NetworkStats summary) throws Exception { - when(mNetManager.getNetworkStatsSummaryDev()).thenReturn(summary); + when(mStatsFactory.readNetworkStatsSummaryDev()).thenReturn(summary); } private void expectNetworkStatsSummaryXt(NetworkStats summary) throws Exception { - when(mNetManager.getNetworkStatsSummaryXt()).thenReturn(summary); + when(mStatsFactory.readNetworkStatsSummaryXt()).thenReturn(summary); } private void expectNetworkStatsTethering(int how, NetworkStats stats) @@ -1080,7 +1083,8 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { private void expectNetworkStatsUidDetail(NetworkStats detail, NetworkStats tetherStats) throws Exception { - when(mNetManager.getNetworkStatsUidDetail(UID_ALL, INTERFACES_ALL)).thenReturn(detail); + when(mStatsFactory.readNetworkStatsDetail(UID_ALL, INTERFACES_ALL, TAG_ALL)) + .thenReturn(detail); // also include tethering details, since they are folded into UID when(mNetManager.getNetworkStatsTethering(STATS_PER_UID)).thenReturn(tetherStats);