From 32f8f25f38f1fb3d2f9e2e84318409c11d488a9a Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Fri, 24 Jun 2022 11:37:47 -0400 Subject: [PATCH] Move CameraManager interactions to background Move all interactions with the CameraManager to a background thread. This makes initialization async, but it's ok as we will be initialized to the correct state (and that will be propagated) once we register the TorchCallback. For now, we call init in the constructor, but a refactor will happen to move this to a better call point. Also, remove optimistically setting the state of mFlashlightEnabled when setting it. Instead, make sure that we always reflect the device state by only changing it from the callback. Test: manual Test: atest FlashlightControllerImpl Fixes: 233870137 Change-Id: I9100c9101429f2dc840c6708ad3115a4eab6ab4b --- .../dagger/FrameworkServicesModule.java | 7 + .../policy/FlashlightControllerImpl.java | 134 +++++++++------- .../policy/FlashlightControllerImplTest.kt | 144 ++++++++++++++++++ 3 files changed, 226 insertions(+), 59 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/FlashlightControllerImplTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/dagger/FrameworkServicesModule.java b/packages/SystemUI/src/com/android/systemui/dagger/FrameworkServicesModule.java index aeda20f165156..fbbfc081c789f 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/FrameworkServicesModule.java +++ b/packages/SystemUI/src/com/android/systemui/dagger/FrameworkServicesModule.java @@ -45,6 +45,7 @@ import android.content.pm.ShortcutManager; import android.content.res.Resources; import android.hardware.SensorManager; import android.hardware.SensorPrivacyManager; +import android.hardware.camera2.CameraManager; import android.hardware.devicestate.DeviceStateManager; import android.hardware.display.AmbientDisplayConfiguration; import android.hardware.display.ColorDisplayManager; @@ -545,4 +546,10 @@ public class FrameworkServicesModule { static SafetyCenterManager provideSafetyCenterManager(Context context) { return context.getSystemService(SafetyCenterManager.class); } + + @Provides + @Singleton + static CameraManager provideCameraManager(Context context) { + return context.getSystemService(CameraManager.class); + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/FlashlightControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/FlashlightControllerImpl.java index 4cf1d2b3f91dd..9946b4b9ecaaa 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/FlashlightControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/FlashlightControllerImpl.java @@ -17,15 +17,11 @@ package com.android.systemui.statusbar.policy; import android.annotation.WorkerThread; -import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; import android.hardware.camera2.CameraAccessException; import android.hardware.camera2.CameraCharacteristics; import android.hardware.camera2.CameraManager; -import android.os.Handler; -import android.os.HandlerThread; -import android.os.Process; import android.provider.Settings; import android.provider.Settings.Secure; import android.text.TextUtils; @@ -33,12 +29,19 @@ import android.util.Log; import androidx.annotation.NonNull; +import com.android.internal.annotations.GuardedBy; +import com.android.systemui.broadcast.BroadcastSender; import com.android.systemui.dagger.SysUISingleton; +import com.android.systemui.dagger.qualifiers.Background; import com.android.systemui.dump.DumpManager; +import com.android.systemui.util.settings.SecureSettings; import java.io.PrintWriter; import java.lang.ref.WeakReference; import java.util.ArrayList; +import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import javax.inject.Inject; @@ -59,65 +62,88 @@ public class FlashlightControllerImpl implements FlashlightController { "com.android.settings.flashlight.action.FLASHLIGHT_CHANGED"; private final CameraManager mCameraManager; - private final Context mContext; - /** Call {@link #ensureHandler()} before using */ - private Handler mHandler; + private final Executor mExecutor; + private final SecureSettings mSecureSettings; + private final DumpManager mDumpManager; + private final BroadcastSender mBroadcastSender; - /** Lock on mListeners when accessing */ + private final boolean mHasFlashlight; + + @GuardedBy("mListeners") private final ArrayList> mListeners = new ArrayList<>(1); - /** Lock on {@code this} when accessing */ + @GuardedBy("this") private boolean mFlashlightEnabled; - - private String mCameraId; + @GuardedBy("this") private boolean mTorchAvailable; - @Inject - public FlashlightControllerImpl(Context context, DumpManager dumpManager) { - mContext = context; - mCameraManager = (CameraManager) mContext.getSystemService(Context.CAMERA_SERVICE); + private final AtomicReference mCameraId; + private final AtomicBoolean mInitted = new AtomicBoolean(false); - dumpManager.registerDumpable(getClass().getSimpleName(), this); - tryInitCamera(); + @Inject + public FlashlightControllerImpl( + DumpManager dumpManager, + CameraManager cameraManager, + @Background Executor bgExecutor, + SecureSettings secureSettings, + BroadcastSender broadcastSender, + PackageManager packageManager + ) { + mCameraManager = cameraManager; + mExecutor = bgExecutor; + mCameraId = new AtomicReference<>(null); + mSecureSettings = secureSettings; + mDumpManager = dumpManager; + mBroadcastSender = broadcastSender; + + mHasFlashlight = packageManager.hasSystemFeature(PackageManager.FEATURE_CAMERA_FLASH); + init(); } + private void init() { + if (!mInitted.getAndSet(true)) { + mDumpManager.registerDumpable(getClass().getSimpleName(), this); + mExecutor.execute(this::tryInitCamera); + } + } + + @WorkerThread private void tryInitCamera() { + if (!mHasFlashlight || mCameraId.get() != null) return; try { - mCameraId = getCameraId(); + mCameraId.set(getCameraId()); } catch (Throwable e) { Log.e(TAG, "Couldn't initialize.", e); return; } - if (mCameraId != null) { - ensureHandler(); - mCameraManager.registerTorchCallback(mTorchCallback, mHandler); + if (mCameraId.get() != null) { + mCameraManager.registerTorchCallback(mExecutor, mTorchCallback); } } public void setFlashlight(boolean enabled) { - boolean pendingError = false; - synchronized (this) { - if (mCameraId == null) return; - if (mFlashlightEnabled != enabled) { - mFlashlightEnabled = enabled; - try { - mCameraManager.setTorchMode(mCameraId, enabled); - } catch (CameraAccessException e) { - Log.e(TAG, "Couldn't set torch mode", e); - mFlashlightEnabled = false; - pendingError = true; + if (!mHasFlashlight) return; + if (mCameraId.get() == null) { + mExecutor.execute(this::tryInitCamera); + } + mExecutor.execute(() -> { + if (mCameraId.get() == null) return; + synchronized (this) { + if (mFlashlightEnabled != enabled) { + try { + mCameraManager.setTorchMode(mCameraId.get(), enabled); + } catch (CameraAccessException e) { + Log.e(TAG, "Couldn't set torch mode", e); + dispatchError(); + } } } - } - dispatchModeChanged(mFlashlightEnabled); - if (pendingError) { - dispatchError(); - } + }); } public boolean hasFlashlight() { - return mContext.getPackageManager().hasSystemFeature(PackageManager.FEATURE_CAMERA_FLASH); + return mHasFlashlight; } public synchronized boolean isEnabled() { @@ -131,13 +157,13 @@ public class FlashlightControllerImpl implements FlashlightController { @Override public void addCallback(@NonNull FlashlightListener l) { synchronized (mListeners) { - if (mCameraId == null) { - tryInitCamera(); + if (mCameraId.get() == null) { + mExecutor.execute(this::tryInitCamera); } cleanUpListenersLocked(l); mListeners.add(new WeakReference<>(l)); - l.onFlashlightAvailabilityChanged(mTorchAvailable); - l.onFlashlightChanged(mFlashlightEnabled); + l.onFlashlightAvailabilityChanged(isAvailable()); + l.onFlashlightChanged(isEnabled()); } } @@ -148,14 +174,7 @@ public class FlashlightControllerImpl implements FlashlightController { } } - private synchronized void ensureHandler() { - if (mHandler == null) { - HandlerThread thread = new HandlerThread(TAG, Process.THREAD_PRIORITY_BACKGROUND); - thread.start(); - mHandler = new Handler(thread.getLooper()); - } - } - + @WorkerThread private String getCameraId() throws CameraAccessException { String[] ids = mCameraManager.getCameraIdList(); for (String id : ids) { @@ -221,10 +240,9 @@ public class FlashlightControllerImpl implements FlashlightController { @Override @WorkerThread public void onTorchModeUnavailable(String cameraId) { - if (TextUtils.equals(cameraId, mCameraId)) { + if (TextUtils.equals(cameraId, mCameraId.get())) { setCameraAvailable(false); - Settings.Secure.putInt( - mContext.getContentResolver(), Settings.Secure.FLASHLIGHT_AVAILABLE, 0); + mSecureSettings.putInt(Settings.Secure.FLASHLIGHT_AVAILABLE, 0); } } @@ -232,14 +250,12 @@ public class FlashlightControllerImpl implements FlashlightController { @Override @WorkerThread public void onTorchModeChanged(String cameraId, boolean enabled) { - if (TextUtils.equals(cameraId, mCameraId)) { + if (TextUtils.equals(cameraId, mCameraId.get())) { setCameraAvailable(true); setTorchMode(enabled); - Settings.Secure.putInt( - mContext.getContentResolver(), Settings.Secure.FLASHLIGHT_AVAILABLE, 1); - Settings.Secure.putInt( - mContext.getContentResolver(), Secure.FLASHLIGHT_ENABLED, enabled ? 1 : 0); - mContext.sendBroadcast(new Intent(ACTION_FLASHLIGHT_CHANGED)); + mSecureSettings.putInt(Settings.Secure.FLASHLIGHT_AVAILABLE, 1); + mSecureSettings.putInt(Secure.FLASHLIGHT_ENABLED, enabled ? 1 : 0); + mBroadcastSender.sendBroadcast(new Intent(ACTION_FLASHLIGHT_CHANGED)); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/FlashlightControllerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/FlashlightControllerImplTest.kt new file mode 100644 index 0000000000000..db0029af4ee27 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/FlashlightControllerImplTest.kt @@ -0,0 +1,144 @@ +/* + * Copyright (C) 2022 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.systemui.statusbar.policy + +import android.content.pm.PackageManager +import android.hardware.camera2.CameraCharacteristics +import android.hardware.camera2.CameraManager +import android.hardware.camera2.impl.CameraMetadataNative +import android.test.suitebuilder.annotation.SmallTest +import android.testing.AndroidTestingRunner +import com.android.systemui.SysuiTestCase +import com.android.systemui.broadcast.BroadcastSender +import com.android.systemui.dump.DumpManager +import com.android.systemui.util.concurrency.FakeExecutor +import com.android.systemui.util.settings.FakeSettings +import com.android.systemui.util.time.FakeSystemClock +import java.util.concurrent.Executor +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.Mockito.any +import org.mockito.Mockito.clearInvocations +import org.mockito.Mockito.eq +import org.mockito.Mockito.never +import org.mockito.Mockito.verify +import org.mockito.Mockito.verifyNoMoreInteractions +import org.mockito.Mockito.verifyZeroInteractions +import org.mockito.Mockito.`when` +import org.mockito.MockitoAnnotations + +@SmallTest +@RunWith(AndroidTestingRunner::class) +class FlashlightControllerImplTest : SysuiTestCase() { + + @Mock + private lateinit var dumpManager: DumpManager + + @Mock + private lateinit var cameraManager: CameraManager + + @Mock + private lateinit var broadcastSender: BroadcastSender + + @Mock + private lateinit var packageManager: PackageManager + + private lateinit var fakeSettings: FakeSettings + private lateinit var fakeSystemClock: FakeSystemClock + private lateinit var backgroundExecutor: FakeExecutor + private lateinit var controller: FlashlightControllerImpl + + @Before + fun setup() { + MockitoAnnotations.initMocks(this) + + fakeSystemClock = FakeSystemClock() + backgroundExecutor = FakeExecutor(fakeSystemClock) + fakeSettings = FakeSettings() + + `when`(packageManager.hasSystemFeature(PackageManager.FEATURE_CAMERA_FLASH)) + .thenReturn(true) + + controller = FlashlightControllerImpl( + dumpManager, + cameraManager, + backgroundExecutor, + fakeSettings, + broadcastSender, + packageManager + ) + } + + @Test + fun testNoCameraManagerInteractionDirectlyOnConstructor() { + verifyZeroInteractions(cameraManager) + } + + @Test + fun testCameraManagerInitAfterConstructionOnExecutor() { + injectCamera() + backgroundExecutor.runAllReady() + + verify(cameraManager).registerTorchCallback(eq(backgroundExecutor), any()) + } + + @Test + fun testNoCallbackIfNoFlashCamera() { + injectCamera(flash = false) + backgroundExecutor.runAllReady() + + verify(cameraManager, never()).registerTorchCallback(any(), any()) + } + + @Test + fun testNoCallbackIfNoBackCamera() { + injectCamera(facing = CameraCharacteristics.LENS_FACING_FRONT) + backgroundExecutor.runAllReady() + + verify(cameraManager, never()).registerTorchCallback(any(), any()) + } + + @Test + fun testSetFlashlightInBackgroundExecutor() { + val id = injectCamera() + backgroundExecutor.runAllReady() + + clearInvocations(cameraManager) + val enable = !controller.isEnabled + controller.setFlashlight(enable) + verifyNoMoreInteractions(cameraManager) + + backgroundExecutor.runAllReady() + verify(cameraManager).setTorchMode(id, enable) + } + + private fun injectCamera( + flash: Boolean = true, + facing: Int = CameraCharacteristics.LENS_FACING_BACK + ): String { + val cameraID = "ID" + val camera = CameraCharacteristics(CameraMetadataNative().apply { + set(CameraCharacteristics.FLASH_INFO_AVAILABLE, flash) + set(CameraCharacteristics.LENS_FACING, facing) + }) + `when`(cameraManager.cameraIdList).thenReturn(arrayOf(cameraID)) + `when`(cameraManager.getCameraCharacteristics(cameraID)).thenReturn(camera) + return cameraID + } +}