Merge "Use Service handler thread for NetworkStatsObservers" into main
diff --git a/service-t/src/com/android/server/net/NetworkStatsObservers.java b/service-t/src/com/android/server/net/NetworkStatsObservers.java
index 21cf351..cab29e3 100644
--- a/service-t/src/com/android/server/net/NetworkStatsObservers.java
+++ b/service-t/src/com/android/server/net/NetworkStatsObservers.java
@@ -32,7 +32,6 @@
 import android.net.NetworkTemplate;
 import android.net.netstats.IUsageCallback;
 import android.os.Handler;
-import android.os.HandlerThread;
 import android.os.IBinder;
 import android.os.Looper;
 import android.os.Message;
@@ -46,6 +45,7 @@
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.net.module.util.PerUidCounter;
 
+import java.util.Objects;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /**
@@ -78,8 +78,11 @@
     // Sequence number of DataUsageRequests
     private final AtomicInteger mNextDataUsageRequestId = new AtomicInteger();
 
-    // Lazily instantiated when an observer is registered.
-    private volatile Handler mHandler;
+    private final Handler mHandler;
+
+    NetworkStatsObservers(@NonNull Looper looper) {
+        mHandler = new Handler(Objects.requireNonNull(looper), mHandlerCallback);
+    }
 
     /**
      * Creates a wrapper that contains the caller context and a normalized request.
@@ -100,7 +103,7 @@
         if (LOG) Log.d(TAG, "Registering observer for " + requestInfo);
         mDataUsageRequestsPerUid.incrementCountOrThrow(callingUid);
 
-        getHandler().sendMessage(mHandler.obtainMessage(MSG_REGISTER, requestInfo));
+        mHandler.sendMessage(mHandler.obtainMessage(MSG_REGISTER, requestInfo));
         return request;
     }
 
@@ -110,7 +113,7 @@
      * <p>It will unregister the observer asynchronously, so it is safe to call from any thread.
      */
     public void unregister(DataUsageRequest request, int callingUid) {
-        getHandler().sendMessage(mHandler.obtainMessage(MSG_UNREGISTER, callingUid, 0 /* ignore */,
+        mHandler.sendMessage(mHandler.obtainMessage(MSG_UNREGISTER, callingUid, 0 /* ignore */,
                 request));
     }
 
@@ -125,34 +128,10 @@
                 long currentTime) {
         StatsContext statsContext = new StatsContext(xtSnapshot, uidSnapshot, activeIfaces,
                 activeUidIfaces, currentTime);
-        getHandler().sendMessage(mHandler.obtainMessage(MSG_UPDATE_STATS, statsContext));
+        mHandler.sendMessage(mHandler.obtainMessage(MSG_UPDATE_STATS, statsContext));
     }
 
-    private Handler getHandler() {
-        if (mHandler == null) {
-            synchronized (this) {
-                if (mHandler == null) {
-                    if (LOGV) Log.v(TAG, "Creating handler");
-                    mHandler = new Handler(getHandlerLooperLocked(), mHandlerCallback);
-                }
-            }
-        }
-        return mHandler;
-    }
-
-    @VisibleForTesting
-    protected Looper getHandlerLooperLocked() {
-        // TODO: Currently, callbacks are dispatched on this thread if the caller register
-        //  callback without supplying a Handler. To ensure that the service handler thread
-        //  is not blocked by client code, the observers must create their own thread. Once
-        //  all callbacks are dispatched outside of the handler thread, the service handler
-        //  thread can be used here.
-        HandlerThread handlerThread = new HandlerThread(TAG);
-        handlerThread.start();
-        return handlerThread.getLooper();
-    }
-
-    private Handler.Callback mHandlerCallback = new Handler.Callback() {
+    private final Handler.Callback mHandlerCallback = new Handler.Callback() {
         @Override
         public boolean handleMessage(Message msg) {
             switch (msg.what) {
diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java
index 9684d18..64b17eb 100644
--- a/service-t/src/com/android/server/net/NetworkStatsService.java
+++ b/service-t/src/com/android/server/net/NetworkStatsService.java
@@ -593,7 +593,7 @@
                 INetd.Stub.asInterface((IBinder) context.getSystemService(Context.NETD_SERVICE)),
                 alarmManager, wakeLock, getDefaultClock(),
                 new DefaultNetworkStatsSettings(), new NetworkStatsFactory(context),
-                new NetworkStatsObservers(), new Dependencies());
+                new Dependencies());
 
         return service;
     }
@@ -603,8 +603,7 @@
     @VisibleForTesting
     NetworkStatsService(Context context, INetd netd, AlarmManager alarmManager,
             PowerManager.WakeLock wakeLock, Clock clock, NetworkStatsSettings settings,
-            NetworkStatsFactory factory, NetworkStatsObservers statsObservers,
-            @NonNull Dependencies deps) {
+            NetworkStatsFactory factory, @NonNull Dependencies deps) {
         mContext = Objects.requireNonNull(context, "missing Context");
         mNetd = Objects.requireNonNull(netd, "missing Netd");
         mAlarmManager = Objects.requireNonNull(alarmManager, "missing AlarmManager");
@@ -612,7 +611,6 @@
         mSettings = Objects.requireNonNull(settings, "missing NetworkStatsSettings");
         mWakeLock = Objects.requireNonNull(wakeLock, "missing WakeLock");
         mStatsFactory = Objects.requireNonNull(factory, "missing factory");
-        mStatsObservers = Objects.requireNonNull(statsObservers, "missing NetworkStatsObservers");
         mDeps = Objects.requireNonNull(deps, "missing Dependencies");
         mStatsDir = mDeps.getOrCreateStatsDir();
         if (!mStatsDir.exists()) {
@@ -622,6 +620,7 @@
         final HandlerThread handlerThread = mDeps.makeHandlerThread();
         handlerThread.start();
         mHandler = new NetworkStatsHandler(handlerThread.getLooper());
+        mStatsObservers = new NetworkStatsObservers(handlerThread.getLooper());
         mNetworkStatsSubscriptionsMonitor = deps.makeSubscriptionsMonitor(mContext,
                 (command) -> mHandler.post(command) , this);
         mContentResolver = mContext.getContentResolver();
diff --git a/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java b/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
index e62ac74..0bbc34c 100644
--- a/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
+++ b/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
@@ -47,7 +47,6 @@
 import android.net.NetworkTemplate;
 import android.os.HandlerThread;
 import android.os.IBinder;
-import android.os.Looper;
 import android.os.Process;
 import android.os.UserHandle;
 import android.telephony.TelephonyManager;
@@ -127,13 +126,7 @@
 
         mObserverHandlerThread = new HandlerThread("NetworkStatsObserversTest");
         mObserverHandlerThread.start();
-        final Looper observerLooper = mObserverHandlerThread.getLooper();
-        mStatsObservers = new NetworkStatsObservers() {
-            @Override
-            protected Looper getHandlerLooperLocked() {
-                return observerLooper;
-            }
-        };
+        mStatsObservers = new NetworkStatsObservers(mObserverHandlerThread.getLooper());
 
         mActiveIfaces = new ArrayMap<>();
         mActiveUidIfaces = new ArrayMap<>();
diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
index 3ed51bc..3d7ad66 100644
--- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
+++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
@@ -123,7 +123,6 @@
 import android.os.Handler;
 import android.os.HandlerThread;
 import android.os.IBinder;
-import android.os.Looper;
 import android.os.PowerManager;
 import android.os.SimpleClock;
 import android.provider.Settings;
@@ -293,7 +292,6 @@
     private String mCompareStatsResult = null;
     private @Mock Resources mResources;
     private Boolean mIsDebuggable;
-    private HandlerThread mObserverHandlerThread;
     final TestDependencies mDeps = new TestDependencies();
 
     private class MockContext extends BroadcastInterceptingContext {
@@ -377,21 +375,8 @@
                 powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG);
 
         mHandlerThread = new HandlerThread("NetworkStatsServiceTest-HandlerThread");
-        // Create a separate thread for observers to run on. This thread cannot be the same
-        // as the handler thread, because the observer callback is fired on this thread, and
-        // it should not be blocked by client code. Additionally, creating the observers
-        // object requires a looper, which can only be obtained after a thread has been started.
-        mObserverHandlerThread = new HandlerThread("NetworkStatsServiceTest-ObserversThread");
-        mObserverHandlerThread.start();
-        final Looper observerLooper = mObserverHandlerThread.getLooper();
-        final NetworkStatsObservers statsObservers = new NetworkStatsObservers() {
-            @Override
-            protected Looper getHandlerLooperLocked() {
-                return observerLooper;
-            }
-        };
         mService = new NetworkStatsService(mServiceContext, mNetd, mAlarmManager, wakeLock,
-                mClock, mSettings, mStatsFactory, statsObservers, mDeps);
+                mClock, mSettings, mStatsFactory, mDeps);
 
         mElapsedRealtime = 0L;
 
@@ -589,10 +574,6 @@
             mHandlerThread.quitSafely();
             mHandlerThread.join();
         }
-        if (mObserverHandlerThread != null) {
-            mObserverHandlerThread.quitSafely();
-            mObserverHandlerThread.join();
-        }
     }
 
     private void initWifiStats(NetworkStateSnapshot snapshot) throws Exception {