From 0b142d9ffd2dab27db7507c34cbea51c5c0a3ff0 Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Thu, 25 May 2023 22:28:36 +0000 Subject: [PATCH] Add tests to ensure input settings are loaded on boot The input settings are loaded on systemReady(). To add these tests, we must mock UEventManager throughout input to prevent UEventObservers from being added from the test process, which doesn't have permissions. Bug: 283782679 Test: atest InputManagerServiceTests Change-Id: I023e5c1ef44367c2147978acae5f225c9f50f11c Merged-In: I023e5c1ef44367c2147978acae5f225c9f50f11c --- .../server/input/BatteryController.java | 57 +++++++------------ .../server/input/InputManagerService.java | 16 ++++-- .../input/KeyboardBacklightController.java | 16 +++--- .../android/server/input/UEventManager.java | 42 ++++++++++++++ .../server/input/BatteryControllerTests.kt | 3 +- .../server/input/InputManagerServiceTests.kt | 53 ++++++++++++++++- .../input/KeyboardBacklightControllerTests.kt | 4 +- 7 files changed, 138 insertions(+), 53 deletions(-) create mode 100644 services/core/java/com/android/server/input/UEventManager.java diff --git a/services/core/java/com/android/server/input/BatteryController.java b/services/core/java/com/android/server/input/BatteryController.java index ff9ce6f16075a..38a0d37c5679c 100644 --- a/services/core/java/com/android/server/input/BatteryController.java +++ b/services/core/java/com/android/server/input/BatteryController.java @@ -46,7 +46,6 @@ import android.view.InputDevice; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; -import com.android.server.input.BatteryController.UEventManager.UEventBatteryListener; import java.io.PrintWriter; import java.util.Arrays; @@ -102,8 +101,9 @@ final class BatteryController { @GuardedBy("mLock") private BluetoothBatteryManager.BluetoothBatteryListener mBluetoothBatteryListener; - BatteryController(Context context, NativeInputManagerService nativeService, Looper looper) { - this(context, nativeService, looper, new UEventManager() {}, + BatteryController(Context context, NativeInputManagerService nativeService, Looper looper, + UEventManager uEventManager) { + this(context, nativeService, looper, uEventManager, new LocalBluetoothBatteryManager(context, looper)); } @@ -567,7 +567,7 @@ final class BatteryController { private BluetoothAdapter.OnMetadataChangedListener mBluetoothMetadataListener; @Nullable - private UEventBatteryListener mUEventBatteryListener; + private BatteryController.UEventBatteryListener mUEventBatteryListener; DeviceMonitor(int deviceId) { mState = new State(deviceId); @@ -630,7 +630,7 @@ final class BatteryController { return; } final int deviceId = mState.deviceId; - mUEventBatteryListener = new UEventBatteryListener() { + mUEventBatteryListener = new BatteryController.UEventBatteryListener() { @Override public void onBatteryUEvent(long eventTime) { handleUEventNotification(deviceId, eventTime); @@ -898,40 +898,25 @@ final class BatteryController { } } - // An interface used to change the API of UEventObserver to a more test-friendly format. @VisibleForTesting - interface UEventManager { - - @VisibleForTesting - abstract class UEventBatteryListener { - private final UEventObserver mObserver = new UEventObserver() { - @Override - public void onUEvent(UEvent event) { - final long eventTime = SystemClock.uptimeMillis(); - if (DEBUG) { - Slog.d(TAG, - "UEventListener: Received UEvent: " - + event + " eventTime: " + eventTime); - } - if (!"CHANGE".equalsIgnoreCase(event.get("ACTION")) - || !"POWER_SUPPLY".equalsIgnoreCase(event.get("SUBSYSTEM"))) { - // Disregard any UEvents that do not correspond to battery changes. - return; - } - UEventBatteryListener.this.onBatteryUEvent(eventTime); - } - }; - - public abstract void onBatteryUEvent(long eventTime); + abstract static class UEventBatteryListener extends UEventManager.UEventListener { + @Override + public void onUEvent(UEventObserver.UEvent event) { + final long eventTime = SystemClock.uptimeMillis(); + if (DEBUG) { + Slog.d(TAG, + "UEventListener: Received UEvent: " + + event + " eventTime: " + eventTime); + } + if (!"CHANGE".equalsIgnoreCase(event.get("ACTION")) + || !"POWER_SUPPLY".equalsIgnoreCase(event.get("SUBSYSTEM"))) { + // Disregard any UEvents that do not correspond to battery changes. + return; + } + UEventBatteryListener.this.onBatteryUEvent(eventTime); } - default void addListener(UEventBatteryListener listener, String match) { - listener.mObserver.startObserving(match); - } - - default void removeListener(UEventBatteryListener listener) { - listener.mObserver.stopObserving(); - } + public abstract void onBatteryUEvent(long eventTime); } // An interface used to change the API of adding a bluetooth battery listener to a more diff --git a/services/core/java/com/android/server/input/InputManagerService.java b/services/core/java/com/android/server/input/InputManagerService.java index c7b80af27d794..6a177e03102fd 100644 --- a/services/core/java/com/android/server/input/InputManagerService.java +++ b/services/core/java/com/android/server/input/InputManagerService.java @@ -393,10 +393,12 @@ public class InputManagerService extends IInputManager.Stub static class Injector { private final Context mContext; private final Looper mLooper; + private final UEventManager mUEventManager; - Injector(Context context, Looper looper) { + Injector(Context context, Looper looper, UEventManager uEventManager) { mContext = context; mLooper = looper; + mUEventManager = uEventManager; } Context getContext() { @@ -407,6 +409,10 @@ public class InputManagerService extends IInputManager.Stub return mLooper; } + UEventManager getUEventManager() { + return mUEventManager; + } + NativeInputManagerService getNativeService(InputManagerService service) { return new NativeInputManagerService.NativeImpl(service, mLooper.getQueue()); } @@ -417,7 +423,7 @@ public class InputManagerService extends IInputManager.Stub } public InputManagerService(Context context) { - this(new Injector(context, DisplayThread.get().getLooper())); + this(new Injector(context, DisplayThread.get().getLooper(), new UEventManager() {})); } @VisibleForTesting @@ -432,10 +438,12 @@ public class InputManagerService extends IInputManager.Stub mSettingsObserver = new InputSettingsObserver(mContext, mHandler, this, mNative); mKeyboardLayoutManager = new KeyboardLayoutManager(mContext, mNative, mDataStore, injector.getLooper()); - mBatteryController = new BatteryController(mContext, mNative, injector.getLooper()); + mBatteryController = new BatteryController(mContext, mNative, injector.getLooper(), + injector.getUEventManager()); mKeyboardBacklightController = InputFeatureFlagProvider.isKeyboardBacklightControlEnabled() ? new KeyboardBacklightController(mContext, mNative, mDataStore, - injector.getLooper()) : new KeyboardBacklightControllerInterface() {}; + injector.getLooper(), injector.getUEventManager()) + : new KeyboardBacklightControllerInterface() {}; mKeyRemapper = new KeyRemapper(mContext, mNative, mDataStore, injector.getLooper()); mUseDevInputEventForAudioJack = diff --git a/services/core/java/com/android/server/input/KeyboardBacklightController.java b/services/core/java/com/android/server/input/KeyboardBacklightController.java index 1253b5b5b303c..c3205afe14f23 100644 --- a/services/core/java/com/android/server/input/KeyboardBacklightController.java +++ b/services/core/java/com/android/server/input/KeyboardBacklightController.java @@ -94,6 +94,7 @@ final class KeyboardBacklightController implements private final PersistentDataStore mDataStore; private final Handler mHandler; private final AnimatorFactory mAnimatorFactory; + private final UEventManager mUEventManager; // Always access on handler thread or need to lock this for synchronization. private final SparseArray mKeyboardBacklights = new SparseArray<>(1); // Maintains state if all backlights should be on or turned off @@ -123,19 +124,21 @@ final class KeyboardBacklightController implements } KeyboardBacklightController(Context context, NativeInputManagerService nativeService, - PersistentDataStore dataStore, Looper looper) { - this(context, nativeService, dataStore, looper, ValueAnimator::ofInt); + PersistentDataStore dataStore, Looper looper, UEventManager uEventManager) { + this(context, nativeService, dataStore, looper, ValueAnimator::ofInt, uEventManager); } @VisibleForTesting KeyboardBacklightController(Context context, NativeInputManagerService nativeService, - PersistentDataStore dataStore, Looper looper, AnimatorFactory animatorFactory) { + PersistentDataStore dataStore, Looper looper, AnimatorFactory animatorFactory, + UEventManager uEventManager) { mContext = context; mNative = nativeService; mDataStore = dataStore; mHandler = new Handler(looper, this::handleMessage); mAnimatorFactory = animatorFactory; mAmbientController = new AmbientKeyboardBacklightController(context, looper); + mUEventManager = uEventManager; } @Override @@ -152,13 +155,12 @@ final class KeyboardBacklightController implements // We want to observe creation of such LED nodes since they might be created after device // FD created and InputDevice creation logic doesn't initialize LED nodes which leads to // backlight not working. - UEventObserver observer = new UEventObserver() { + mUEventManager.addListener(new UEventManager.UEventListener() { @Override - public void onUEvent(UEvent event) { + public void onUEvent(UEventObserver.UEvent event) { onKeyboardBacklightUEvent(event); } - }; - observer.startObserving(UEVENT_KEYBOARD_BACKLIGHT_TAG); + }, UEVENT_KEYBOARD_BACKLIGHT_TAG); if (InputFeatureFlagProvider.isAmbientKeyboardBacklightControlEnabled()) { // Start ambient backlight controller diff --git a/services/core/java/com/android/server/input/UEventManager.java b/services/core/java/com/android/server/input/UEventManager.java new file mode 100644 index 0000000000000..17d87e4f05968 --- /dev/null +++ b/services/core/java/com/android/server/input/UEventManager.java @@ -0,0 +1,42 @@ +/* + * Copyright 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.input; + +import android.os.UEventObserver; + +/** An interface used to change the API of UEventObserver to a more test-friendly format. */ +interface UEventManager { + + abstract class UEventListener { + private final UEventObserver mObserver = new UEventObserver() { + @Override + public void onUEvent(UEvent event) { + UEventListener.this.onUEvent(event); + } + }; + + public abstract void onUEvent(UEventObserver.UEvent event); + } + + default void addListener(UEventListener listener, String match) { + listener.mObserver.startObserving(match); + } + + default void removeListener(UEventListener listener) { + listener.mObserver.stopObserving(); + } +} diff --git a/services/tests/servicestests/src/com/android/server/input/BatteryControllerTests.kt b/services/tests/servicestests/src/com/android/server/input/BatteryControllerTests.kt index 416b1f49f5d9a..c36122b7e7883 100644 --- a/services/tests/servicestests/src/com/android/server/input/BatteryControllerTests.kt +++ b/services/tests/servicestests/src/com/android/server/input/BatteryControllerTests.kt @@ -40,8 +40,7 @@ import androidx.test.core.app.ApplicationProvider import com.android.server.input.BatteryController.BluetoothBatteryManager import com.android.server.input.BatteryController.BluetoothBatteryManager.BluetoothBatteryListener import com.android.server.input.BatteryController.POLLING_PERIOD_MILLIS -import com.android.server.input.BatteryController.UEventManager -import com.android.server.input.BatteryController.UEventManager.UEventBatteryListener +import com.android.server.input.BatteryController.UEventBatteryListener import com.android.server.input.BatteryController.USI_BATTERY_VALIDITY_DURATION_MILLIS import org.hamcrest.Description import org.hamcrest.Matcher diff --git a/services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt b/services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt index 677144c144a76..72066ea951e3c 100644 --- a/services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt +++ b/services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt @@ -23,9 +23,12 @@ import android.hardware.display.DisplayViewport import android.os.IInputConstants import android.os.test.TestLooper import android.platform.test.annotations.Presubmit +import android.provider.Settings +import android.test.mock.MockContentResolver import android.view.Display import android.view.PointerIcon import androidx.test.InstrumentationRegistry +import com.android.internal.util.test.FakeSettingsProvider import com.google.common.truth.Truth.assertThat import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue @@ -33,6 +36,8 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyBoolean +import org.mockito.ArgumentMatchers.anyFloat import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.eq import org.mockito.Mock @@ -44,7 +49,9 @@ import org.mockito.Mockito.spy import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.Mockito.verifyNoMoreInteractions +import org.mockito.Mockito.verifyZeroInteractions import org.mockito.junit.MockitoJUnit +import org.mockito.stubbing.OngoingStubbing import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit @@ -58,7 +65,10 @@ import java.util.concurrent.TimeUnit class InputManagerServiceTests { @get:Rule - val rule = MockitoJUnit.rule()!! + val mockitoRule = MockitoJUnit.rule()!! + + @get:Rule + val fakeSettingsProviderRule = FakeSettingsProvider.rule()!! @Mock private lateinit var native: NativeInputManagerService @@ -66,17 +76,25 @@ class InputManagerServiceTests { @Mock private lateinit var wmCallbacks: InputManagerService.WindowManagerCallbacks + @Mock + private lateinit var uEventManager: UEventManager + private lateinit var service: InputManagerService private lateinit var localService: InputManagerInternal private lateinit var context: Context private lateinit var testLooper: TestLooper + private lateinit var contentResolver: MockContentResolver @Before fun setup() { context = spy(ContextWrapper(InstrumentationRegistry.getContext())) + contentResolver = MockContentResolver(context) + contentResolver.addProvider(Settings.AUTHORITY, FakeSettingsProvider()) + whenever(context.contentResolver).thenReturn(contentResolver) testLooper = TestLooper() service = - InputManagerService(object : InputManagerService.Injector(context, testLooper.looper) { + InputManagerService(object : InputManagerService.Injector( + context, testLooper.looper, uEventManager) { override fun getNativeService( service: InputManagerService? ): NativeInputManagerService { @@ -91,10 +109,37 @@ class InputManagerServiceTests { service.setWindowManagerCallbacks(wmCallbacks) } + @Test + fun testStart() { + verifyZeroInteractions(native) + + service.start() + verify(native).start() + } + + @Test + fun testInputSettingsUpdatedOnSystemRunning() { + verifyZeroInteractions(native) + + service.systemRunning() + + verify(native).setPointerSpeed(anyInt()) + verify(native).setTouchpadPointerSpeed(anyInt()) + verify(native).setTouchpadNaturalScrollingEnabled(anyBoolean()) + verify(native).setTouchpadTapToClickEnabled(anyBoolean()) + verify(native).setTouchpadRightClickZoneEnabled(anyBoolean()) + verify(native).setShowTouches(anyBoolean()) + verify(native).reloadPointerIcons() + verify(native).notifyKeyGestureTimeoutsChanged() + verify(native).setMotionClassifierEnabled(anyBoolean()) + verify(native).setMaximumObscuringOpacityForTouch(anyFloat()) + verify(native).setStylusPointerIconEnabled(anyBoolean()) + } + @Test fun testPointerDisplayUpdatesWhenDisplayViewportsChanged() { val displayId = 123 - `when`(wmCallbacks.pointerDisplayId).thenReturn(displayId) + whenever(wmCallbacks.pointerDisplayId).thenReturn(displayId) val viewports = listOf() localService.setDisplayViewports(viewports) verify(native).setDisplayViewports(any(Array::class.java)) @@ -337,3 +382,5 @@ class InputManagerServiceTests { thread.join(100 /*millis*/) } } + +private fun whenever(methodCall: T): OngoingStubbing = `when`(methodCall) diff --git a/services/tests/servicestests/src/com/android/server/input/KeyboardBacklightControllerTests.kt b/services/tests/servicestests/src/com/android/server/input/KeyboardBacklightControllerTests.kt index 67158f24839dc..3f4a4fba841e6 100644 --- a/services/tests/servicestests/src/com/android/server/input/KeyboardBacklightControllerTests.kt +++ b/services/tests/servicestests/src/com/android/server/input/KeyboardBacklightControllerTests.kt @@ -105,6 +105,8 @@ class KeyboardBacklightControllerTests { private lateinit var iInputManager: IInputManager @Mock private lateinit var native: NativeInputManagerService + @Mock + private lateinit var uEventManager: UEventManager private lateinit var keyboardBacklightController: KeyboardBacklightController private lateinit var context: Context private lateinit var dataStore: PersistentDataStore @@ -130,7 +132,7 @@ class KeyboardBacklightControllerTests { }) testLooper = TestLooper() keyboardBacklightController = KeyboardBacklightController(context, native, dataStore, - testLooper.looper, FakeAnimatorFactory()) + testLooper.looper, FakeAnimatorFactory(), uEventManager) InputManagerGlobal.resetInstance(iInputManager) val inputManager = InputManager(context) `when`(context.getSystemService(eq(Context.INPUT_SERVICE))).thenReturn(inputManager)