Merge "BatteryService: Clean up init logic." (223d49f5) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/BatteryService.java

+51 −28

Original line number	Diff line number	Diff line
		@@ -49,6 +49,7 @@ import android.os.BatteryProperty;
		import android.os.Binder;
		import android.os.FileUtils;
		import android.os.Handler;
		import android.os.HandlerThread;
		import android.os.IBatteryPropertiesListener;
		import android.os.IBatteryPropertiesRegistrar;
		import android.os.IBinder;
		@@ -75,6 +76,7 @@ import java.io.PrintWriter;
		import java.util.Arrays;
		import java.util.List;
		import java.util.NoSuchElementException;
		import java.util.Objects;
		import java.util.concurrent.atomic.AtomicReference;

		/**
		@@ -176,6 +178,7 @@ public final class BatteryService extends SystemService {
		private HealthServiceWrapper mHealthServiceWrapper;
		private HealthHalCallback mHealthHalCallback;
		private BatteryPropertiesRegistrar mBatteryPropertiesRegistrar;
		private HandlerThread mHandlerThread;

		public BatteryService(Context context) {
		super(context);
		@@ -1195,14 +1198,13 @@ public final class BatteryService extends SystemService {
		Arrays.asList(INSTANCE_VENDOR, INSTANCE_HEALTHD);

		private final IServiceNotification mNotification = new Notification();
		private final HandlerThread mHandlerThread = new HandlerThread("HealthServiceRefresh");
		// These variables are fixed after init.
		private Callback mCallback;
		private IHealthSupplier mHealthSupplier;
		private String mInstanceName;

		private final Object mLastServiceSetLock = new Object();
		// Last IHealth service received.
		// set must be also be guarded with mLastServiceSetLock to ensure ordering.
		private final AtomicReference<IHealth> mLastService = new AtomicReference<>();

		/**
		@@ -1220,6 +1222,10 @@ public final class BatteryService extends SystemService {
		* Start monitoring registration of new IHealth services. Only instances that are in
		* {@code sAllInstances} and in device / framework manifest are used. This function should
		* only be called once.
		*
		* mCallback.onRegistration() is called synchronously (aka in init thread) before
		* this method returns.
		*
		* @throws RemoteException transaction error when talking to IServiceManager
		* @throws NoSuchElementException if one of the following cases:
		* - No service manager;
		@@ -1240,40 +1246,48 @@ public final class BatteryService extends SystemService {
		mCallback = callback;
		mHealthSupplier = healthSupplier;

		traceBegin("HealthInitGetManager");
		// Initialize mLastService and call callback for the first time (in init thread)
		IHealth newService = null;
		for (String name : sAllInstances) {
		traceBegin("HealthInitGetService_" + name);
		try {
		manager = managerSupplier.get();
		newService = healthSupplier.get(name);
		} catch (NoSuchElementException ex) {
		/* ignored, handled below */
		} finally {
		traceEnd();
		}
		for (String name : sAllInstances) {
		traceBegin("HealthInitGetTransport_" + name);
		try {
		if (manager.getTransport(IHealth.kInterfaceName, name) !=
		IServiceManager.Transport.EMPTY) {
		if (newService != null) {
		mInstanceName = name;
		mLastService.set(newService);
		break;
		}
		} finally {
		traceEnd();
		}
		}

		if (mInstanceName == null) {
		if (mInstanceName == null \|\| newService == null) {
		throw new NoSuchElementException(String.format(
		"No IHealth service instance among %s is available. Perhaps no permission?",
		sAllInstances.toString()));
		}
		mCallback.onRegistration(null, newService, mInstanceName);

		// Register for future service registrations
		traceBegin("HealthInitRegisterNotification");
		mHandlerThread.start();
		try {
		manager.registerForNotifications(IHealth.kInterfaceName, mInstanceName, mNotification);
		managerSupplier.get().registerForNotifications(
		IHealth.kInterfaceName, mInstanceName, mNotification);
		} finally {
		traceEnd();
		}
		Slog.i(TAG, "health: HealthServiceWrapper listening to instance " + mInstanceName);
		}

		@VisibleForTesting
		HandlerThread getHandlerThread() {
		return mHandlerThread;
		}

		interface Callback {
		/**
		* This function is invoked asynchronously when a new and related IServiceNotification
		@@ -1302,7 +1316,7 @@ public final class BatteryService extends SystemService {
		*/
		interface IHealthSupplier {
		default IHealth get(String name) throws NoSuchElementException, RemoteException {
		return IHealth.getService(name);
		return IHealth.getService(name, true /* retry */);
		}
		}

		@@ -1312,19 +1326,28 @@ public final class BatteryService extends SystemService {
		boolean preexisting) {
		if (!IHealth.kInterfaceName.equals(interfaceName)) return;
		if (!mInstanceName.equals(instanceName)) return;

		// This runnable only runs on mHandlerThread and ordering is ensured, hence
		// no locking is needed inside the runnable.
		mHandlerThread.getThreadHandler().post(new Runnable() {
		@Override
		public void run() {
		try {
		// ensures the order of multiple onRegistration on different threads.
		synchronized (mLastServiceSetLock) {
		IHealth newService = mHealthSupplier.get(instanceName);
		IHealth newService = mHealthSupplier.get(mInstanceName);
		IHealth oldService = mLastService.getAndSet(newService);
		Slog.i(TAG, "health: new instance registered " + instanceName);
		mCallback.onRegistration(oldService, newService, instanceName);
		}

		// preexisting may be inaccurate (race). Check for equality here.
		if (Objects.equals(newService, oldService)) return;

		Slog.i(TAG, "health: new instance registered " + mInstanceName);
		mCallback.onRegistration(oldService, newService, mInstanceName);
		} catch (NoSuchElementException \| RemoteException ex) {
		Slog.e(TAG, "health: Cannot get instance '" + instanceName + "': " +
		ex.getMessage() + ". Perhaps no permission?");
		Slog.e(TAG, "health: Cannot get instance '" + mInstanceName
		+ "': " + ex.getMessage() + ". Perhaps no permission?");
		}
		}
		});
		}
		}
		}
		}

services/tests/servicestests/src/com/android/server/BatteryServiceTest.java

+45 −15

Original line number	Diff line number	Diff line
		@@ -36,12 +36,14 @@ import org.mockito.ArgumentMatcher;
		import org.mockito.Mock;
		import org.mockito.MockitoAnnotations;
		import org.mockito.Spy;
		import org.mockito.invocation.InvocationOnMock;


		public class BatteryServiceTest extends AndroidTestCase {

		@Mock IServiceManager mMockedManager;
		@Mock IHealth mMockedHal;
		@Mock IHealth mMockedHal2;

		@Mock BatteryService.HealthServiceWrapper.Callback mCallback;
		@Mock BatteryService.HealthServiceWrapper.IServiceManagerSupplier mManagerSupplier;
		@@ -56,6 +58,12 @@ public class BatteryServiceTest extends AndroidTestCase {
		MockitoAnnotations.initMocks(this);
		}

		@Override
		public void tearDown() {
		if (mWrapper != null)
		mWrapper.getHandlerThread().quitSafely();
		}

		public static <T> ArgumentMatcher<T> isOneOf(Collection<T> collection) {
		return new ArgumentMatcher<T>() {
		@Override public boolean matches(T e) {
		@@ -70,42 +78,64 @@ public class BatteryServiceTest extends AndroidTestCase {
		private void initForInstances(String... instanceNamesArr) throws Exception {
		final Collection<String> instanceNames = Arrays.asList(instanceNamesArr);
		doAnswer((invocation) -> {
		Slog.e("BatteryServiceTest", "health: onRegistration " + invocation.getArguments()[2]);
		((IServiceNotification)invocation.getArguments()[2]).onRegistration(
		IHealth.kInterfaceName,
		(String)invocation.getArguments()[1],
		true /* preexisting */);
		// technically, preexisting is ignored by
		// BatteryService.HealthServiceWrapper.Notification, but still call it correctly.
		sendNotification(invocation, true);
		sendNotification(invocation, true);
		sendNotification(invocation, false);
		return null;
		}).when(mMockedManager).registerForNotifications(
		eq(IHealth.kInterfaceName),
		argThat(isOneOf(instanceNames)),
		any(IServiceNotification.class));

		doReturn(mMockedHal).when(mMockedManager)
		.get(eq(IHealth.kInterfaceName), argThat(isOneOf(instanceNames)));

		doReturn(IServiceManager.Transport.HWBINDER).when(mMockedManager)
		.getTransport(eq(IHealth.kInterfaceName), argThat(isOneOf(instanceNames)));

		doReturn(mMockedManager).when(mManagerSupplier).get();
		doReturn(mMockedHal).when(mHealthServiceSupplier)
		.get(argThat(isOneOf(instanceNames)));
		doReturn(mMockedHal) // init calls this
		.doReturn(mMockedHal) // notification 1
		.doReturn(mMockedHal) // notification 2
		.doReturn(mMockedHal2) // notification 3
		.doThrow(new RuntimeException("Should not call getService for more than 4 times"))
		.when(mHealthServiceSupplier).get(argThat(isOneOf(instanceNames)));

		mWrapper = new BatteryService.HealthServiceWrapper();
		}

		private void waitHandlerThreadFinish() throws Exception {
		for (int i = 0; i < 5; i++) {
		if (!mWrapper.getHandlerThread().getThreadHandler().hasMessagesOrCallbacks()) {
		return;
		}
		Thread.sleep(300);
		}
		assertFalse(mWrapper.getHandlerThread().getThreadHandler().hasMessagesOrCallbacks());
		}

		private static void sendNotification(InvocationOnMock invocation, boolean preexisting)
		throws Exception {
		((IServiceNotification)invocation.getArguments()[2]).onRegistration(
		IHealth.kInterfaceName,
		(String)invocation.getArguments()[1],
		preexisting);
		}

		@SmallTest
		public void testWrapPreferVendor() throws Exception {
		initForInstances(VENDOR, HEALTHD);
		mWrapper.init(mCallback, mManagerSupplier, mHealthServiceSupplier);
		verify(mCallback).onRegistration(same(null), same(mMockedHal), eq(VENDOR));
		waitHandlerThreadFinish();
		verify(mCallback, times(1)).onRegistration(same(null), same(mMockedHal), eq(VENDOR));
		verify(mCallback, never()).onRegistration(same(mMockedHal), same(mMockedHal), anyString());
		verify(mCallback, times(1)).onRegistration(same(mMockedHal), same(mMockedHal2), eq(VENDOR));
		}

		@SmallTest
		public void testUseHealthd() throws Exception {
		initForInstances(HEALTHD);
		mWrapper.init(mCallback, mManagerSupplier, mHealthServiceSupplier);
		verify(mCallback).onRegistration(same(null), same(mMockedHal), eq(HEALTHD));
		waitHandlerThreadFinish();
		verify(mCallback, times(1)).onRegistration(same(null), same(mMockedHal), eq(HEALTHD));
		verify(mCallback, never()).onRegistration(same(mMockedHal), same(mMockedHal), anyString());
		verify(mCallback, times(1)).onRegistration(same(mMockedHal), same(mMockedHal2), eq(HEALTHD));
		}

		@SmallTest