Delay binding to vms publisher service clients

Also, refactor VmsPublisherService to reduce memory footprint, flatten
class hierarchy, get rid of lock

Test: bat_land

Bug: 79697265
Change-Id: Ia53c43a97f960ffa8e78617612ff9fac378e7fb4

2 files changed, 205 insertions, 205 deletions

diff --git a/service/src/com/android/car/VmsPublisherService.java b/service/src/com/android/car/VmsPublisherService.java
index 299ca1783d..a240e171de 100644
--- a/service/src/com/android/car/VmsPublisherService.java
+++ b/service/src/com/android/car/VmsPublisherService.java
@@ -16,37 +16,36 @@
 
 package com.android.car;
 
-import android.annotation.SystemApi;
 import android.car.vms.IVmsPublisherClient;
 import android.car.vms.IVmsPublisherService;
 import android.car.vms.IVmsSubscriberClient;
 import android.car.vms.VmsLayer;
 import android.car.vms.VmsLayersOffering;
 import android.car.vms.VmsSubscriptionState;
+import android.content.BroadcastReceiver;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
+import android.content.IntentFilter;
 import android.content.ServiceConnection;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.os.Binder;
 import android.os.Handler;
 import android.os.IBinder;
+import android.os.Message;
 import android.os.RemoteException;
 import android.os.UserHandle;
 import android.text.TextUtils;
+import android.util.ArrayMap;
+import android.util.ArraySet;
 import android.util.Log;
 
 import com.android.car.hal.VmsHalService;
-import com.android.internal.annotations.GuardedBy;
+import com.android.car.hal.VmsHalService.VmsHalPublisherListener;
 
 import java.io.PrintWriter;
-import java.lang.ref.WeakReference;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -54,36 +53,65 @@ import java.util.Set;
  * Receives HAL updates by implementing VmsHalService.VmsHalListener.
  * Binds to publishers and configures them to use this service.
  * Notifies publishers of subscription changes.
- *
- * @hide
  */
-@SystemApi
-public class VmsPublisherService extends IVmsPublisherService.Stub
-        implements CarServiceBase, VmsHalService.VmsHalPublisherListener {
+public class VmsPublisherService extends IVmsPublisherService.Stub implements CarServiceBase {
     private static final boolean DBG = true;
     private static final String TAG = "VmsPublisherService";
 
+    private static final int MSG_HAL_SUBSCRIPTION_CHANGED = 1;
+
     private final Context mContext;
     private final VmsHalService mHal;
-    private final VmsPublisherManager mPublisherManager;
-    private Set<String> mSafePermissions;
+    private final Map<String, PublisherConnection> mPublisherConnectionMap = new ArrayMap<>();
+    private final Map<String, IVmsPublisherClient> mPublisherMap = new ArrayMap<>();
+    private final Set<String> mSafePermissions;
+    private final Handler mHandler = new EventHandler();
+    private final VmsHalPublisherListener mHalPublisherListener;
+
+    private BroadcastReceiver mBootCompleteReceiver;
 
     public VmsPublisherService(Context context, VmsHalService hal) {
         mContext = context;
         mHal = hal;
-        mPublisherManager = new VmsPublisherManager(this, context, new Handler());
+
+        mHalPublisherListener = subscriptionState -> mHandler.sendMessage(
+                mHandler.obtainMessage(MSG_HAL_SUBSCRIPTION_CHANGED, subscriptionState));
+
+        // Load permissions that can be granted to publishers.
+        mSafePermissions = new ArraySet<>(
+                Arrays.asList(mContext.getResources().getStringArray(R.array.vmsSafePermissions)));
     }
 
     // Implements CarServiceBase interface.
     @Override
     public void init() {
-        mHal.addPublisherListener(this);
-        // Load permissions that can be granted to publishers.
-        mSafePermissions = new HashSet<>(
-                Arrays.asList(mContext.getResources().getStringArray(R.array.vmsSafePermissions)));
-        // Launch publishers.
+        mHal.addPublisherListener(mHalPublisherListener);
+
+        if (isTestEnvironment()) {
+            Log.d(TAG, "Running under test environment");
+            bindToAllPublishers();
+        } else {
+            mBootCompleteReceiver = new BroadcastReceiver() {
+                @Override
+                public void onReceive(Context context, Intent intent) {
+                    if (Intent.ACTION_LOCKED_BOOT_COMPLETED.equals(intent.getAction())) {
+                        onLockedBootCompleted();
+                    } else {
+                        Log.e(TAG, "Unexpected action received: " + intent);
+                    }
+                }
+            };
+
+            mContext.registerReceiver(mBootCompleteReceiver,
+                    new IntentFilter(Intent.ACTION_LOCKED_BOOT_COMPLETED));
+        }
+    }
+
+    private void bindToAllPublishers() {
         String[] publisherNames = mContext.getResources().getStringArray(
                 R.array.vmsPublisherClients);
+        if (DBG) Log.d(TAG, "Publishers found: " + publisherNames.length);
+
         for (String publisherName : publisherNames) {
             if (TextUtils.isEmpty(publisherName)) {
                 Log.e(TAG, "empty publisher name");
@@ -92,32 +120,48 @@ public class VmsPublisherService extends IVmsPublisherService.Stub
             ComponentName name = ComponentName.unflattenFromString(publisherName);
             if (name == null) {
                 Log.e(TAG, "invalid publisher name: " + publisherName);
+                continue;
             }
 
-            if (mContext.getPackageManager().isPackageAvailable(name.getPackageName())) {
-                mPublisherManager.bind(name);
-            } else {
+            if (!mContext.getPackageManager().isPackageAvailable(name.getPackageName())) {
                 Log.w(TAG, "VMS publisher not installed: " + publisherName);
+                continue;
             }
+
+            bind(name);
         }
     }
 
     @Override
     public void release() {
-        mPublisherManager.release();
-        mHal.removePublisherListener(this);
+        if (mBootCompleteReceiver != null) {
+            mContext.unregisterReceiver(mBootCompleteReceiver);
+            mBootCompleteReceiver = null;
+        }
+        mHal.removePublisherListener(mHalPublisherListener);
+
+        for (PublisherConnection connection : mPublisherConnectionMap.values()) {
+            mContext.unbindService(connection);
+        }
+        mPublisherConnectionMap.clear();
+        mPublisherMap.clear();
     }
 
     @Override
     public void dump(PrintWriter writer) {
+        writer.println("*" + getClass().getSimpleName() + "*");
+        writer.println("mSafePermissions: " + mSafePermissions);
+        writer.println("mPublisherMap:" + mPublisherMap);
+        writer.println("mPublisherConnectionMap:" + mPublisherConnectionMap);
     }
 
+    /* Called in arbitrary binder thread */
     @Override
     public void setLayersOffering(IBinder token, VmsLayersOffering offering) {
         mHal.setPublisherLayersOffering(token, offering);
     }
 
-    // Implements IVmsPublisherService interface.
+    /* Called in arbitrary binder thread */
     @Override
     public void publish(IBinder token, VmsLayer layer, int publisherId, byte[] payload) {
         if (DBG) {
@@ -149,27 +193,33 @@ public class VmsPublisherService extends IVmsPublisherService.Stub
         }
     }
 
+    /* Called in arbitrary binder thread */
     @Override
     public VmsSubscriptionState getSubscriptions() {
         ICarImpl.assertVmsPublisherPermission(mContext);
         return mHal.getSubscriptionState();
     }
 
+    /* Called in arbitrary binder thread */
     @Override
     public int getPublisherId(byte[] publisherInfo) {
         ICarImpl.assertVmsPublisherPermission(mContext);
         return mHal.getPublisherId(publisherInfo);
     }
 
-    // Implements VmsHalListener interface
+    private void onLockedBootCompleted() {
+        if (DBG) Log.i(TAG, "onLockedBootCompleted");
+
+        bindToAllPublishers();
+    }
+
     /**
      * This method is only invoked by VmsHalService.notifyPublishers which is synchronized.
      * Therefore this method only sees a non-decreasing sequence.
      */
-    @Override
-    public void onChange(VmsSubscriptionState subscriptionState) {
+    private void handleHalSubscriptionChanged(VmsSubscriptionState subscriptionState) {
         // Send the message to application listeners.
-        for (IVmsPublisherClient client : mPublisherManager.getClients()) {
+        for (IVmsPublisherClient client : mPublisherMap.values()) {
             try {
                 client.onVmsSubscriptionChange(subscriptionState);
             } catch (RemoteException ex) {
@@ -179,202 +229,154 @@ public class VmsPublisherService extends IVmsPublisherService.Stub
     }
 
     /**
-     * Keeps track of publishers that are using this service.
+     * Tries to bind to a publisher.
+     *
+     * @param name publisher component name (e.g. android.car.vms.logger/.LoggingService).
      */
-    private static class VmsPublisherManager {
-        /**
-         * Allows to modify mPublisherMap and mPublisherConnectionMap as a single unit.
-         */
-        private final Object mLock = new Object();
-        @GuardedBy("mLock")
-        private final Map<String, PublisherConnection> mPublisherConnectionMap = new HashMap<>();
-        @GuardedBy("mLock")
-        private final Map<String, IVmsPublisherClient> mPublisherMap = new HashMap<>();
-        private final WeakReference<VmsPublisherService> mPublisherService;
-        private Context mContext;
-        private Handler mHandler;
-
-
-        VmsPublisherManager(
-                VmsPublisherService publisherService, Context context, Handler handler) {
-            if (context == null) {
-                throw new IllegalArgumentException("Context is null");
-            }
-            mPublisherService = new WeakReference<>(publisherService);
-            mContext = context;
-            mHandler = handler;
+    private void bind(ComponentName name) {
+        String publisherName = name.flattenToString();
+        if (DBG) {
+            Log.d(TAG, "binding to: " + publisherName);
         }
 
-        /**
-         * Tries to bind to a publisher.
-         *
-         * @param name publisher component name (e.g. android.car.vms.logger/.LoggingService).
-         */
-        public void bind(ComponentName name) {
-            VmsPublisherService publisherService = mPublisherService.get();
-            if (publisherService == null) return;
-            String publisherName = name.flattenToString();
-            if (DBG) {
-                Log.d(TAG, "binding to: " + publisherName);
-            }
-            synchronized (mLock) {
-                if (mPublisherConnectionMap.containsKey(publisherName)) {
-                    // Already registered, nothing to do.
-                    return;
-                }
-                grantPermissions(name);
-                Intent intent = new Intent();
-                intent.setComponent(name);
-                PublisherConnection connection = new PublisherConnection(mContext, mHandler, name);
-                if (publisherService.mContext.bindServiceAsUser(intent, connection,
-                        Context.BIND_AUTO_CREATE, UserHandle.SYSTEM)) {
-                    mPublisherConnectionMap.put(publisherName, connection);
-                } else {
-                    Log.e(TAG, "unable to bind to: " + publisherName);
-                }
-            }
+        if (mPublisherConnectionMap.containsKey(publisherName)) {
+            // Already registered, nothing to do.
+            return;
         }
-
-        /**
-         * Removes the publisher and associated connection.
-         *
-         * @param name publisher component name (e.g. android.car.vms.Logger).
-         */
-        public void unbind(ComponentName name) {
-            VmsPublisherService publisherService = mPublisherService.get();
-            if (publisherService == null) return;
-            String publisherName = name.flattenToString();
-            if (DBG) {
-                Log.d(TAG, "unbinding from: " + publisherName);
-            }
-            synchronized (mLock) {
-                boolean found = mPublisherMap.remove(publisherName) != null;
-                if (found) {
-                    PublisherConnection connection = mPublisherConnectionMap.get(publisherName);
-                    publisherService.mContext.unbindService(connection);
-                    mPublisherConnectionMap.remove(publisherName);
-                } else {
-                    Log.e(TAG, "unbind: unknown publisher." + publisherName);
-                }
-            }
+        grantPermissions(name);
+        Intent intent = new Intent();
+        intent.setComponent(name);
+        PublisherConnection connection = new PublisherConnection(name);
+        if (mContext.bindServiceAsUser(intent, connection,
+                Context.BIND_AUTO_CREATE, UserHandle.SYSTEM)) {
+            mPublisherConnectionMap.put(publisherName, connection);
+        } else {
+            Log.e(TAG, "unable to bind to: " + publisherName);
         }
+    }
 
-        /**
-         * Returns the list of publishers currently registered.
-         *
-         * @return list of publishers.
-         */
-        public List<IVmsPublisherClient> getClients() {
-            synchronized (mLock) {
-                return new ArrayList<>(mPublisherMap.values());
-            }
+    /**
+     * Removes the publisher and associated connection.
+     *
+     * @param name publisher component name (e.g. android.car.vms.Logger).
+     */
+    private void unbind(ComponentName name) {
+        String publisherName = name.flattenToString();
+        if (DBG) {
+            Log.d(TAG, "unbinding from: " + publisherName);
         }
 
-        public void release() {
-            VmsPublisherService publisherService = mPublisherService.get();
-            if (publisherService == null) return;
-            for (PublisherConnection connection : mPublisherConnectionMap.values()) {
-                publisherService.mContext.unbindService(connection);
-            }
-            mPublisherConnectionMap.clear();
-            mPublisherMap.clear();
+        boolean found = mPublisherMap.remove(publisherName) != null;
+        if (found) {
+            PublisherConnection connection = mPublisherConnectionMap.get(publisherName);
+            mContext.unbindService(connection);
+            mPublisherConnectionMap.remove(publisherName);
+        } else {
+            Log.e(TAG, "unbind: unknown publisher." + publisherName);
         }
+    }
 
-        private void grantPermissions(ComponentName component) {
-            VmsPublisherService publisherService = mPublisherService.get();
-            if (publisherService == null) return;
-            final PackageManager packageManager = publisherService.mContext.getPackageManager();
-            final String packageName = component.getPackageName();
-            PackageInfo packageInfo;
-            try {
-                packageInfo = packageManager.getPackageInfo(packageName,
-                        PackageManager.GET_PERMISSIONS);
-            } catch (PackageManager.NameNotFoundException e) {
-                Log.e(TAG, "Error getting package info for " + packageName, e);
-                return;
+    private void grantPermissions(ComponentName component) {
+        final PackageManager packageManager = mContext.getPackageManager();
+        final String packageName = component.getPackageName();
+        PackageInfo packageInfo;
+        try {
+            packageInfo = packageManager.getPackageInfo(packageName,
+                    PackageManager.GET_PERMISSIONS);
+        } catch (PackageManager.NameNotFoundException e) {
+            Log.e(TAG, "Error getting package info for " + packageName, e);
+            return;
+        }
+        if (packageInfo.requestedPermissions == null) return;
+        for (String permission : packageInfo.requestedPermissions) {
+            if (!mSafePermissions.contains(permission)) {
+                continue;
             }
-            if (packageInfo.requestedPermissions == null) return;
-            for (String permission : packageInfo.requestedPermissions) {
-                if (!publisherService.mSafePermissions.contains(permission)) {
-                    continue;
-                }
-                if (packageManager.checkPermission(permission, packageName)
-                        == PackageManager.PERMISSION_GRANTED) {
-                    continue;
-                }
-                try {
-                    packageManager.grantRuntimePermission(packageName, permission,
-                            UserHandle.SYSTEM);
-                    Log.d(TAG, "Permission " + permission + " granted to " + packageName);
-                } catch (SecurityException | IllegalArgumentException e) {
-                    Log.e(TAG, "Error while trying to grant " + permission + " to " + packageName,
-                            e);
-                }
+            if (packageManager.checkPermission(permission, packageName)
+                    == PackageManager.PERMISSION_GRANTED) {
+                continue;
+            }
+            try {
+                packageManager.grantRuntimePermission(packageName, permission,
+                        UserHandle.SYSTEM);
+                Log.d(TAG, "Permission " + permission + " granted to " + packageName);
+            } catch (SecurityException | IllegalArgumentException e) {
+                Log.e(TAG, "Error while trying to grant " + permission + " to " + packageName,
+                        e);
             }
         }
+    }
 
-        class PublisherConnection implements ServiceConnection {
+    private boolean isTestEnvironment() {
+        // If the context is derived from other package it means we're running under
+        // environment.
+        return !TextUtils.equals(mContext.getBasePackageName(), mContext.getPackageName());
+    }
+
+    class PublisherConnection implements ServiceConnection {
+        private final IBinder mToken = new Binder();
+        private final ComponentName mName;
 
-            private final Context mContext;
-            private final IBinder mToken = new Binder();
-            private final ComponentName mName;
-            private final Handler mHandler;
+        PublisherConnection(ComponentName name) {
+            mName = name;
+        }
 
-            PublisherConnection(Context context, Handler handler, ComponentName name) {
-                mContext = context;
-                mHandler = handler;
-                mName = name;
+        private final Runnable mBindRunnable = new Runnable() {
+            @Override
+            public void run() {
+                Log.d(TAG, "delayed binding for: " + mName);
+                bind(mName);
             }
+        };
 
-            private final Runnable mBindRunnable = new Runnable() {
-                @Override
-                public void run() {
-                    Log.d(TAG, "delayed binding for: " + mName);
-                    VmsPublisherManager.this.bind(mName);
-                }
-            };
+        /**
+         * Once the service binds to a publisher service, the publisher binder is added to
+         * mPublisherMap
+         * and the publisher is configured to use this service.
+         */
+        @Override
+        public void onServiceConnected(ComponentName name, IBinder binder) {
+            if (DBG) {
+                Log.d(TAG, "onServiceConnected, name: " + name + ", binder: " + binder);
+            }
+            IVmsPublisherClient service = IVmsPublisherClient.Stub.asInterface(binder);
+            mPublisherMap.put(name.flattenToString(), service);
+            try {
+                service.setVmsPublisherService(mToken, VmsPublisherService.this);
+            } catch (RemoteException e) {
+                Log.e(TAG, "unable to configure publisher: " + name, e);
+            }
+        }
 
-            /**
-             * Once the service binds to a publisher service, the publisher binder is added to
-             * mPublisherMap
-             * and the publisher is configured to use this service.
-             */
-            @Override
-            public void onServiceConnected(ComponentName name, IBinder binder) {
-                VmsPublisherService publisherService = mPublisherService.get();
-                if (publisherService == null) return;
-                if (DBG) {
-                    Log.d(TAG, "onServiceConnected, name: " + name + ", binder: " + binder);
-                }
-                IVmsPublisherClient service = IVmsPublisherClient.Stub.asInterface(binder);
-                synchronized (mLock) {
-                    mPublisherMap.put(name.flattenToString(), service);
-                }
-                try {
-                    service.setVmsPublisherService(mToken, publisherService);
-                } catch (RemoteException e) {
-                    Log.e(TAG, "unable to configure publisher: " + name, e);
-                }
+        /**
+         * Tries to rebind to the publisher service.
+         */
+        @Override
+        public void onServiceDisconnected(ComponentName name) {
+            String publisherName = name.flattenToString();
+            Log.d(TAG, "onServiceDisconnected, name: " + publisherName);
+
+            int millisecondsToWait = mContext.getResources().getInteger(
+                    com.android.car.R.integer.millisecondsBeforeRebindToVmsPublisher);
+            if (!mName.flattenToString().equals(name.flattenToString())) {
+                throw new IllegalArgumentException(
+                    "Mismatch on publisherConnection. Expected: " + mName + " Got: " + name);
             }
+            mHandler.postDelayed(mBindRunnable, millisecondsToWait);
 
-            /**
-             * Tries to rebind to the publisher service.
-             */
-            @Override
-            public void onServiceDisconnected(ComponentName name) {
-                String publisherName = name.flattenToString();
-                Log.d(TAG, "onServiceDisconnected, name: " + publisherName);
-
-                int millisecondsToWait = mContext.getResources().getInteger(
-                        com.android.car.R.integer.millisecondsBeforeRebindToVmsPublisher);
-                if (!mName.flattenToString().equals(name.flattenToString())) {
-                    throw new IllegalArgumentException(
-                        "Mismatch on publisherConnection. Expected: " + mName + " Got: " + name);
-                }
-                mHandler.postDelayed(mBindRunnable, millisecondsToWait);
+            unbind(name);
+        }
+    }
 
-                VmsPublisherManager.this.unbind(name);
+    private class EventHandler extends Handler {
+        @Override
+        public void handleMessage(Message msg) {
+            switch (msg.what) {
+                case MSG_HAL_SUBSCRIPTION_CHANGED:
+                    handleHalSubscriptionChanged((VmsSubscriptionState) msg.obj);
+                    return;
             }
+            super.handleMessage(msg);
         }
     }
 }
diff --git a/service/src/com/android/car/VmsSubscriberService.java b/service/src/com/android/car/VmsSubscriberService.java
index 371b418215..3f28b0c8cb 100644
--- a/service/src/com/android/car/VmsSubscriberService.java
+++ b/service/src/com/android/car/VmsSubscriberService.java
@@ -16,7 +16,6 @@
 
 package com.android.car;
 
-import android.annotation.SystemApi;
 import android.car.Car;
 import android.car.vms.IVmsSubscriberClient;
 import android.car.vms.IVmsSubscriberService;
@@ -42,7 +41,6 @@ import java.util.Set;
  * + Receives HAL updates by implementing VmsHalService.VmsHalListener.
  * + Offers subscriber/publisher services by implementing IVmsService.Stub.
  */
-@SystemApi
 public class VmsSubscriberService extends IVmsSubscriberService.Stub
         implements CarServiceBase, VmsHalService.VmsHalSubscriberListener {
     private static final boolean DBG = true;