diff --git a/services/core/java/com/android/server/wm/DeviceStateController.java b/services/core/java/com/android/server/wm/DeviceStateController.java index 270b2f80ecf03..41c052d950539 100644 --- a/services/core/java/com/android/server/wm/DeviceStateController.java +++ b/services/core/java/com/android/server/wm/DeviceStateController.java @@ -16,11 +16,13 @@ package com.android.server.wm; +import android.annotation.CallbackExecutor; import android.annotation.NonNull; import android.content.Context; import android.hardware.devicestate.DeviceStateManager; import android.os.Handler; import android.os.HandlerExecutor; +import android.util.ArrayMap; import com.android.internal.R; import com.android.internal.annotations.GuardedBy; @@ -29,6 +31,8 @@ import com.android.internal.util.ArrayUtils; import java.util.ArrayList; import java.util.List; +import java.util.Map; +import java.util.concurrent.Executor; import java.util.function.Consumer; /** @@ -37,6 +41,9 @@ import java.util.function.Consumer; */ final class DeviceStateController implements DeviceStateManager.DeviceStateCallback { + // Used to synchronize WindowManager services call paths with DeviceStateManager's callbacks. + @NonNull + private final WindowManagerGlobalLock mWmLock; @NonNull private final DeviceStateManager mDeviceStateManager; @NonNull @@ -50,10 +57,10 @@ final class DeviceStateController implements DeviceStateManager.DeviceStateCallb private final int mConcurrentDisplayDeviceState; @NonNull private final int[] mReverseRotationAroundZAxisStates; - @GuardedBy("this") + @GuardedBy("mWmLock") @NonNull @VisibleForTesting - final List> mDeviceStateCallbacks = new ArrayList<>(); + final Map, Executor> mDeviceStateCallbacks = new ArrayMap<>(); private final boolean mMatchBuiltInDisplayOrientationToDefaultDisplay; @@ -70,7 +77,9 @@ final class DeviceStateController implements DeviceStateManager.DeviceStateCallb CONCURRENT, } - DeviceStateController(@NonNull Context context, @NonNull Handler handler) { + DeviceStateController(@NonNull Context context, @NonNull Handler handler, + @NonNull WindowManagerGlobalLock wmLock) { + mWmLock = wmLock; mDeviceStateManager = context.getSystemService(DeviceStateManager.class); mOpenDeviceStates = context.getResources() @@ -94,14 +103,20 @@ final class DeviceStateController implements DeviceStateManager.DeviceStateCallb } } - void registerDeviceStateCallback(@NonNull Consumer callback) { - synchronized (this) { - mDeviceStateCallbacks.add(callback); + /** + * Registers a callback to be notified when the device state changes. Callers should always + * post the work onto their own worker thread to avoid holding the WindowManagerGlobalLock for + * an extended period of time. + */ + void registerDeviceStateCallback(@NonNull Consumer callback, + @NonNull @CallbackExecutor Executor executor) { + synchronized (mWmLock) { + mDeviceStateCallbacks.put(callback, executor); } } void unregisterDeviceStateCallback(@NonNull Consumer callback) { - synchronized (this) { + synchronized (mWmLock) { mDeviceStateCallbacks.remove(callback); } } @@ -144,11 +159,21 @@ final class DeviceStateController implements DeviceStateManager.DeviceStateCallb if (mCurrentDeviceState == null || !mCurrentDeviceState.equals(deviceState)) { mCurrentDeviceState = deviceState; - synchronized (this) { - for (Consumer callback : mDeviceStateCallbacks) { - callback.accept(mCurrentDeviceState); + // Make a copy here because it's possible that the consumer tries to remove a callback + // while we're still iterating through the list, which would end up in a + // ConcurrentModificationException. + final List, Executor>> entries = new ArrayList<>(); + synchronized (mWmLock) { + for (Map.Entry, Executor> entry + : mDeviceStateCallbacks.entrySet()) { + entries.add(entry); } } + + for (int i = 0; i < entries.size(); i++) { + Map.Entry, Executor> entry = entries.get(i); + entry.getValue().execute(() -> entry.getKey().accept(mCurrentDeviceState)); + } } } } diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index fb6d036b9eb9d..e44d27911262d 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -188,6 +188,7 @@ import android.metrics.LogMaker; import android.os.Bundle; import android.os.Debug; import android.os.Handler; +import android.os.HandlerExecutor; import android.os.IBinder; import android.os.Message; import android.os.PowerManager; @@ -1182,7 +1183,8 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp mDisplaySwitchTransitionLauncher.foldStateChanged(newFoldState); mDisplayRotation.foldStateChanged(newFoldState); }; - mDeviceStateController.registerDeviceStateCallback(mDeviceStateConsumer); + mDeviceStateController.registerDeviceStateCallback(mDeviceStateConsumer, + new HandlerExecutor(mWmService.mH)); mCloseToSquareMaxAspectRatio = mWmService.mContext.getResources().getFloat( R.dimen.config_closeToSquareDisplayMaxAspectRatio); diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java index 0074ebd579cf6..536915929e88b 100644 --- a/services/core/java/com/android/server/wm/RootWindowContainer.java +++ b/services/core/java/com/android/server/wm/RootWindowContainer.java @@ -438,7 +438,8 @@ class RootWindowContainer extends WindowContainer mTaskSupervisor = mService.mTaskSupervisor; mTaskSupervisor.mRootWindowContainer = this; mDisplayOffTokenAcquirer = mService.new SleepTokenAcquirerImpl(DISPLAY_OFF_SLEEP_TOKEN_TAG); - mDeviceStateController = new DeviceStateController(service.mContext, service.mH); + mDeviceStateController = new DeviceStateController(service.mContext, service.mH, + service.mGlobalLock); mDisplayRotationCoordinator = new DisplayRotationCoordinator(); } diff --git a/services/tests/wmtests/src/com/android/server/wm/DeviceStateControllerTests.java b/services/tests/wmtests/src/com/android/server/wm/DeviceStateControllerTests.java index 9d1fde452cdf9..b515a9dc0d6bc 100644 --- a/services/tests/wmtests/src/com/android/server/wm/DeviceStateControllerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/DeviceStateControllerTests.java @@ -34,6 +34,8 @@ import androidx.test.filters.SmallTest; import com.android.internal.R; +import com.google.common.util.concurrent.MoreExecutors; + import org.junit.Before; import org.junit.Test; @@ -117,7 +119,7 @@ public class DeviceStateControllerTests { public void testUnregisterDeviceStateCallback() { initialize(true /* supportFold */, true /* supportHalfFolded */); assertEquals(1, mTarget.mDeviceStateCallbacks.size()); - assertEquals(mDelegate, mTarget.mDeviceStateCallbacks.get(0)); + assertTrue(mTarget.mDeviceStateCallbacks.containsKey(mDelegate)); mTarget.onStateChanged(mOpenDeviceStates[0]); assertEquals(DeviceStateController.DeviceState.OPEN, mCurrentState); @@ -194,8 +196,9 @@ public class DeviceStateControllerTests { when(mMockContext.getResources()).thenReturn((mockRes)); mockFold(mSupportFold, mSupportHalfFold); Handler mockHandler = mock(Handler.class); - mTarget = new DeviceStateController(mMockContext, mockHandler); - mTarget.registerDeviceStateCallback(mDelegate); + mTarget = new DeviceStateController(mMockContext, mockHandler, + new WindowManagerGlobalLock()); + mTarget.registerDeviceStateCallback(mDelegate, MoreExecutors.directExecutor()); } } }