Merge "Fix ConcurrentModificationException on WindowToken" into main
diff --git a/core/java/android/app/servertransaction/ClientTransactionListenerController.java b/core/java/android/app/servertransaction/ClientTransactionListenerController.java
index c9b4aa1..cda2867 100644
--- a/core/java/android/app/servertransaction/ClientTransactionListenerController.java
+++ b/core/java/android/app/servertransaction/ClientTransactionListenerController.java
@@ -17,14 +17,13 @@
package android.app.servertransaction;
import static android.app.WindowConfiguration.areConfigurationsEqualForDisplay;
+import static android.view.Display.INVALID_DISPLAY;
import static com.android.window.flags.Flags.activityWindowInfoFlag;
import static com.android.window.flags.Flags.bundleClientTransactionFlag;
import static java.util.Objects.requireNonNull;
-import android.annotation.AnyThread;
-import android.annotation.MainThread;
import android.annotation.NonNull;
import android.app.Activity;
import android.app.ActivityThread;
@@ -62,9 +61,11 @@
* Keeps track of the Context whose Configuration will get updated, mapping to the config before
* the change.
*/
+ @GuardedBy("mLock")
private final ArrayMap<Context, Configuration> mContextToPreChangedConfigMap = new ArrayMap<>();
/** Whether there is an {@link ClientTransaction} being executed. */
+ @GuardedBy("mLock")
private boolean mIsClientTransactionExecuting;
/** Gets the singleton controller. */
@@ -96,7 +97,6 @@
* The listener will be invoked with two parameters: {@link Activity#getActivityToken()} and
* {@link ActivityWindowInfo}.
*/
- @AnyThread
public void registerActivityWindowInfoChangedListener(
@NonNull BiConsumer<IBinder, ActivityWindowInfo> listener) {
if (!activityWindowInfoFlag()) {
@@ -111,7 +111,6 @@
* Unregisters the listener that was previously registered via
* {@link #registerActivityWindowInfoChangedListener(BiConsumer)}
*/
- @AnyThread
public void unregisterActivityWindowInfoChangedListener(
@NonNull BiConsumer<IBinder, ActivityWindowInfo> listener) {
if (!activityWindowInfoFlag()) {
@@ -126,7 +125,6 @@
* Called when receives a {@link ClientTransaction} that is updating an activity's
* {@link ActivityWindowInfo}.
*/
- @MainThread
public void onActivityWindowInfoChanged(@NonNull IBinder activityToken,
@NonNull ActivityWindowInfo activityWindowInfo) {
if (!activityWindowInfoFlag()) {
@@ -146,80 +144,85 @@
}
/** Called when starts executing a remote {@link ClientTransaction}. */
- @MainThread
public void onClientTransactionStarted() {
- mIsClientTransactionExecuting = true;
+ synchronized (mLock) {
+ mIsClientTransactionExecuting = true;
+ }
}
/** Called when finishes executing a remote {@link ClientTransaction}. */
- @MainThread
public void onClientTransactionFinished() {
- notifyDisplayManagerIfNeeded();
- mIsClientTransactionExecuting = false;
+ final ArraySet<Integer> configUpdatedDisplayIds;
+ synchronized (mLock) {
+ mIsClientTransactionExecuting = false;
+
+ // When {@link Configuration} is changed, we want to trigger display change callback as
+ // well, because Display reads some fields from {@link Configuration}.
+ if (mContextToPreChangedConfigMap.isEmpty()) {
+ return;
+ }
+
+ // Calculate display ids that have config changed.
+ configUpdatedDisplayIds = new ArraySet<>();
+ final int contextCount = mContextToPreChangedConfigMap.size();
+ try {
+ for (int i = 0; i < contextCount; i++) {
+ final Context context = mContextToPreChangedConfigMap.keyAt(i);
+ final Configuration preChangedConfig = mContextToPreChangedConfigMap.valueAt(i);
+ if (shouldReportDisplayChange(context, preChangedConfig)) {
+ configUpdatedDisplayIds.add(context.getDisplayId());
+ }
+ }
+ } finally {
+ mContextToPreChangedConfigMap.clear();
+ }
+ }
+
+ // Dispatch the display changed callbacks.
+ final int displayCount = configUpdatedDisplayIds.size();
+ for (int i = 0; i < displayCount; i++) {
+ final int displayId = configUpdatedDisplayIds.valueAt(i);
+ onDisplayChanged(displayId);
+ }
}
/** Called before updating the Configuration of the given {@code context}. */
- @MainThread
public void onContextConfigurationPreChanged(@NonNull Context context) {
if (!bundleClientTransactionFlag() || ActivityThread.isSystem()) {
// Not enable for system server.
return;
}
- if (mContextToPreChangedConfigMap.containsKey(context)) {
- // There is an earlier change that hasn't been reported yet.
- return;
+ synchronized (mLock) {
+ if (mContextToPreChangedConfigMap.containsKey(context)) {
+ // There is an earlier change that hasn't been reported yet.
+ return;
+ }
+ mContextToPreChangedConfigMap.put(context,
+ new Configuration(context.getResources().getConfiguration()));
}
- mContextToPreChangedConfigMap.put(context,
- new Configuration(context.getResources().getConfiguration()));
}
/** Called after updating the Configuration of the given {@code context}. */
- @MainThread
public void onContextConfigurationPostChanged(@NonNull Context context) {
if (!bundleClientTransactionFlag() || ActivityThread.isSystem()) {
// Not enable for system server.
return;
}
- if (mIsClientTransactionExecuting) {
- // Wait until #onClientTransactionFinished to prevent it from triggering the same
- // #onDisplayChanged multiple times within the same ClientTransaction.
- return;
- }
- final Configuration preChangedConfig = mContextToPreChangedConfigMap.remove(context);
- if (preChangedConfig != null && shouldReportDisplayChange(context, preChangedConfig)) {
- onDisplayChanged(context.getDisplayId());
- }
- }
-
- /**
- * When {@link Configuration} is changed, we want to trigger display change callback as well,
- * because Display reads some fields from {@link Configuration}.
- */
- private void notifyDisplayManagerIfNeeded() {
- if (mContextToPreChangedConfigMap.isEmpty()) {
- return;
- }
- // Whether the configuration change should trigger DisplayListener#onDisplayChanged.
- try {
- // Calculate display ids that have config changed.
- final ArraySet<Integer> configUpdatedDisplayIds = new ArraySet<>();
- final int contextCount = mContextToPreChangedConfigMap.size();
- for (int i = 0; i < contextCount; i++) {
- final Context context = mContextToPreChangedConfigMap.keyAt(i);
- final Configuration preChangedConfig = mContextToPreChangedConfigMap.valueAt(i);
- if (shouldReportDisplayChange(context, preChangedConfig)) {
- configUpdatedDisplayIds.add(context.getDisplayId());
- }
+ int changedDisplayId = INVALID_DISPLAY;
+ synchronized (mLock) {
+ if (mIsClientTransactionExecuting) {
+ // Wait until #onClientTransactionFinished to prevent it from triggering the same
+ // #onDisplayChanged multiple times within the same ClientTransaction.
+ return;
}
-
- // Dispatch the display changed callbacks.
- final int displayCount = configUpdatedDisplayIds.size();
- for (int i = 0; i < displayCount; i++) {
- final int displayId = configUpdatedDisplayIds.valueAt(i);
- onDisplayChanged(displayId);
+ final Configuration preChangedConfig = mContextToPreChangedConfigMap.remove(context);
+ if (preChangedConfig != null && shouldReportDisplayChange(context, preChangedConfig)) {
+ changedDisplayId = context.getDisplayId();
}
- } finally {
- mContextToPreChangedConfigMap.clear();
+ }
+
+ if (changedDisplayId != INVALID_DISPLAY) {
+ onDisplayChanged(changedDisplayId);
}
}
diff --git a/core/java/android/window/WindowTokenClient.java b/core/java/android/window/WindowTokenClient.java
index 1ffb4ff..cc54a93 100644
--- a/core/java/android/window/WindowTokenClient.java
+++ b/core/java/android/window/WindowTokenClient.java
@@ -19,6 +19,8 @@
import static android.window.ConfigurationHelper.isDifferentDisplay;
import static android.window.ConfigurationHelper.shouldUpdateResources;
+import static com.android.window.flags.Flags.windowTokenConfigThreadSafe;
+
import android.annotation.AnyThread;
import android.annotation.MainThread;
import android.annotation.NonNull;
@@ -144,9 +146,8 @@
if (context == null) {
return;
}
- if (shouldReportConfigChange) {
- // Only report to ClientTransactionListenerController when shouldReportConfigChange,
- // which is on the MainThread.
+ if (shouldReportConfigChange && windowTokenConfigThreadSafe()) {
+ // Only report to ClientTransactionListenerController when shouldReportConfigChange.
final ClientTransactionListenerController controller =
getClientTransactionListenerController();
controller.onContextConfigurationPreChanged(context);
diff --git a/core/java/android/window/flags/windowing_sdk.aconfig b/core/java/android/window/flags/windowing_sdk.aconfig
index 6c00c70..001ed79 100644
--- a/core/java/android/window/flags/windowing_sdk.aconfig
+++ b/core/java/android/window/flags/windowing_sdk.aconfig
@@ -93,6 +93,16 @@
flag {
namespace: "windowing_sdk"
+ name: "window_token_config_thread_safe"
+ description: "Ensure the Configuration pre/post changed is thread safe"
+ bug: "334285008"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+}
+
+flag {
+ namespace: "windowing_sdk"
name: "always_defer_transition_when_apply_wct"
description: "Report error when defer transition fails when it should not"
bug: "335562144"
diff --git a/core/tests/coretests/src/android/app/servertransaction/ClientTransactionListenerControllerTest.java b/core/tests/coretests/src/android/app/servertransaction/ClientTransactionListenerControllerTest.java
index f8c2d6a..b6f4429 100644
--- a/core/tests/coretests/src/android/app/servertransaction/ClientTransactionListenerControllerTest.java
+++ b/core/tests/coretests/src/android/app/servertransaction/ClientTransactionListenerControllerTest.java
@@ -21,6 +21,7 @@
import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
import static com.android.window.flags.Flags.FLAG_BUNDLE_CLIENT_TRANSACTION_FLAG;
+import static com.android.window.flags.Flags.FLAG_WINDOW_TOKEN_CONFIG_THREAD_SAFE;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
@@ -189,6 +190,8 @@
@Test
public void testWindowTokenClient_onConfigurationChanged() {
+ mSetFlagsRule.enableFlags(FLAG_WINDOW_TOKEN_CONFIG_THREAD_SAFE);
+
doNothing().when(mController).onContextConfigurationPreChanged(any());
doNothing().when(mController).onContextConfigurationPostChanged(any());