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
2022-06-24 11:37:47 -04:00
parent cb8c65fef9
commit 32f8f25f38
3 changed files with 226 additions and 59 deletions
--- 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);
+    }
 }
--- 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<WeakReference<FlashlightListener>> 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<String> 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));
            }
        }

--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/FlashlightControllerImplTest.kt
+++ 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<Executor>(), any())
+    }
+
+    @Test
+    fun testNoCallbackIfNoBackCamera() {
+        injectCamera(facing = CameraCharacteristics.LENS_FACING_FRONT)
+        backgroundExecutor.runAllReady()
+
+        verify(cameraManager, never()).registerTorchCallback(any<Executor>(), 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
+    }
+}