diff options
4 files changed, 75 insertions, 58 deletions
diff --git a/core/java/android/app/servertransaction/ClientTransactionListenerController.java b/core/java/android/app/servertransaction/ClientTransactionListenerController.java index c9b4aa1456fd..cda286742d28 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 @@ public class ClientTransactionListenerController { * 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 @@ public class ClientTransactionListenerController { * 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 @@ public class ClientTransactionListenerController { * 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 @@ public class ClientTransactionListenerController { * 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 @@ public class ClientTransactionListenerController { } /** 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 1ffb4ffd12c2..cc54a93a6018 100644 --- a/core/java/android/window/WindowTokenClient.java +++ b/core/java/android/window/WindowTokenClient.java @@ -19,6 +19,8 @@ import static android.window.ConfigurationHelper.freeTextLayoutCachesIfNeeded; 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 @@ public class WindowTokenClient extends Binder { 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 6c00c70a0f55..001ed791aebf 100644 --- a/core/java/android/window/flags/windowing_sdk.aconfig +++ b/core/java/android/window/flags/windowing_sdk.aconfig @@ -93,6 +93,16 @@ flag { 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 f8c2d6aee84a..b6f4429aec8a 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 android.platform.test.flag.junit.SetFlagsRule.DefaultInitValueType 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 @@ public class ClientTransactionListenerControllerTest { @Test public void testWindowTokenClient_onConfigurationChanged() { + mSetFlagsRule.enableFlags(FLAG_WINDOW_TOKEN_CONFIG_THREAD_SAFE); + doNothing().when(mController).onContextConfigurationPreChanged(any()); doNothing().when(mController).onContextConfigurationPostChanged(any()); |