diff options
author | junyulai <junyulai@google.com> | 2020-03-03 15:53:56 +0800 |
---|---|---|
committer | junyulai <junyulai@google.com> | 2020-03-04 16:01:21 +0800 |
commit | 51905ad64a8bca44729c07f761527daf4e5dceab (patch) | |
tree | cc64767cc358207347c6dea94fadaad44504d3c1 | |
parent | 7322edad465c536a04e07c0c6fabcfc1f173d5ad (diff) | |
download | base-51905ad64a8bca44729c07f761527daf4e5dceab.tar.gz |
Remove setHandler in NetworkStatsService
Currently, internal handler is set by setHandler after
constructing NSS object. This was introduced in ag/866187 to
access the handler in the unit test.
However, the design put NSS in a bad situation where all classes
that need handler or executor could not be final and need to be
dynamically allocated in order to get a valid handler.
Thus, since the usage of handler is removed in previous patch,
this change eliminate setHandler by initializing the handler in
the constructor.
Test: atest FrameworksNetTests
Bug: 150664039
Change-Id: I794a24d00b0ca9fdc78091e7b9ab7307e0f034b7
-rw-r--r-- | services/core/java/com/android/server/net/NetworkStatsService.java | 92 | ||||
-rw-r--r-- | tests/net/java/com/android/server/net/NetworkStatsServiceTest.java | 27 |
2 files changed, 64 insertions, 55 deletions
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index 896bf676d45b..66e691a79f97 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -306,9 +306,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { /** Data layer operation counters for splicing into other structures. */ private NetworkStats mUidOperations = new NetworkStats(0L, 10); - /** Must be set in factory by calling #setHandler. */ - private Handler mHandler; - private Handler.Callback mHandlerCallback; + @NonNull + private final Handler mHandler; private volatile boolean mSystemReady; private long mPersistThreshold = 2 * MB_IN_BYTES; @@ -324,6 +323,9 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private final static int DUMP_STATS_SESSION_COUNT = 20; + @NonNull + private final Dependencies mDeps; + private static @NonNull File getDefaultSystemDir() { return new File(Environment.getDataDirectory(), "system"); } @@ -339,9 +341,24 @@ public class NetworkStatsService extends INetworkStatsService.Stub { Clock.systemUTC()); } - private static final class NetworkStatsHandler extends Handler { - NetworkStatsHandler(Looper looper, Handler.Callback callback) { - super(looper, callback); + private final class NetworkStatsHandler extends Handler { + NetworkStatsHandler(@NonNull Looper looper) { + super(looper); + } + + @Override + public void handleMessage(Message msg) { + switch (msg.what) { + case MSG_PERFORM_POLL: { + performPoll(FLAG_PERSIST_ALL); + break; + } + case MSG_PERFORM_POLL_REGISTER_ALERT: { + performPoll(FLAG_PERSIST_NETWORK); + registerGlobalAlert(); + break; + } + } } } @@ -355,14 +372,10 @@ public class NetworkStatsService extends INetworkStatsService.Stub { NetworkStatsService service = new NetworkStatsService(context, networkManager, alarmManager, wakeLock, getDefaultClock(), context.getSystemService(TelephonyManager.class), new DefaultNetworkStatsSettings(context), new NetworkStatsFactory(), - new NetworkStatsObservers(), getDefaultSystemDir(), getDefaultBaseDir()); + new NetworkStatsObservers(), getDefaultSystemDir(), getDefaultBaseDir(), + new Dependencies()); service.registerLocalService(); - HandlerThread handlerThread = new HandlerThread(TAG); - Handler.Callback callback = new HandlerCallback(service); - handlerThread.start(); - Handler handler = new NetworkStatsHandler(handlerThread.getLooper(), callback); - service.setHandler(handler, callback); return service; } @@ -373,7 +386,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { AlarmManager alarmManager, PowerManager.WakeLock wakeLock, Clock clock, TelephonyManager teleManager, NetworkStatsSettings settings, NetworkStatsFactory factory, NetworkStatsObservers statsObservers, File systemDir, - File baseDir) { + File baseDir, @NonNull Dependencies deps) { mContext = Objects.requireNonNull(context, "missing Context"); mNetworkManager = Objects.requireNonNull(networkManager, "missing INetworkManagementService"); @@ -387,6 +400,26 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mSystemDir = Objects.requireNonNull(systemDir, "missing systemDir"); mBaseDir = Objects.requireNonNull(baseDir, "missing baseDir"); mUseBpfTrafficStats = new File("/sys/fs/bpf/map_netd_app_uid_stats_map").exists(); + mDeps = Objects.requireNonNull(deps, "missing Dependencies"); + + final HandlerThread handlerThread = mDeps.makeHandlerThread(); + handlerThread.start(); + mHandler = new NetworkStatsHandler(handlerThread.getLooper()); + } + + /** + * Dependencies of NetworkStatsService, for injection in tests. + */ + // TODO: Move more stuff into dependencies object. + @VisibleForTesting + public static class Dependencies { + /** + * Create a HandlerThread to use in NetworkStatsService. + */ + @NonNull + public HandlerThread makeHandlerThread() { + return new HandlerThread(TAG); + } } private void registerLocalService() { @@ -394,12 +427,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { new NetworkStatsManagerInternalImpl()); } - @VisibleForTesting - void setHandler(Handler handler, Handler.Callback callback) { - mHandler = handler; - mHandlerCallback = callback; - } - public void systemReady() { synchronized (mStatsLock) { mSystemReady = true; @@ -1920,33 +1947,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } - @VisibleForTesting - static class HandlerCallback implements Handler.Callback { - private final NetworkStatsService mService; - - HandlerCallback(NetworkStatsService service) { - this.mService = service; - } - - @Override - public boolean handleMessage(Message msg) { - switch (msg.what) { - case MSG_PERFORM_POLL: { - mService.performPoll(FLAG_PERSIST_ALL); - return true; - } - case MSG_PERFORM_POLL_REGISTER_ALERT: { - mService.performPoll(FLAG_PERSIST_NETWORK); - mService.registerGlobalAlert(); - return true; - } - default: { - return false; - } - } - } - } - private void assertSystemReady() { if (!mSystemReady) { throw new IllegalStateException("System not ready"); diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index 123b9c299f76..36deca3e37b7 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -64,6 +64,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.annotation.NonNull; import android.app.AlarmManager; import android.app.usage.NetworkStatsManager; import android.content.Context; @@ -191,15 +192,11 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { PowerManager.WakeLock wakeLock = powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG); - mService = new NetworkStatsService( - mServiceContext, mNetManager, mAlarmManager, wakeLock, mClock, - mServiceContext.getSystemService(TelephonyManager.class), mSettings, - mStatsFactory, new NetworkStatsObservers(), mStatsDir, getBaseDir(mStatsDir)); mHandlerThread = new HandlerThread("HandlerThread"); - mHandlerThread.start(); - Handler.Callback callback = new NetworkStatsService.HandlerCallback(mService); - final Handler handler = new Handler(mHandlerThread.getLooper(), callback); - mService.setHandler(handler, callback); + final NetworkStatsService.Dependencies deps = makeDependencies(); + mService = new NetworkStatsService(mServiceContext, mNetManager, mAlarmManager, wakeLock, + mClock, mServiceContext.getSystemService(TelephonyManager.class), mSettings, + mStatsFactory, new NetworkStatsObservers(), mStatsDir, getBaseDir(mStatsDir), deps); mElapsedRealtime = 0L; @@ -216,11 +213,21 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { // catch INetworkManagementEventObserver during systemReady() ArgumentCaptor<INetworkManagementEventObserver> networkObserver = - ArgumentCaptor.forClass(INetworkManagementEventObserver.class); + ArgumentCaptor.forClass(INetworkManagementEventObserver.class); verify(mNetManager).registerObserver(networkObserver.capture()); mNetworkObserver = networkObserver.getValue(); } + @NonNull + private NetworkStatsService.Dependencies makeDependencies() { + return new NetworkStatsService.Dependencies() { + @Override + public HandlerThread makeHandlerThread() { + return mHandlerThread; + } + }; + } + @After public void tearDown() throws Exception { IoUtils.deleteContents(mStatsDir); @@ -233,6 +240,8 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { mSession.close(); mService = null; + + mHandlerThread.quitSafely(); } @Test |