Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6a2ba93d authored by Chris Li's avatar Chris Li Committed by Android (Google) Code Review
Browse files

Merge "Fix ConcurrentModificationException on WindowToken" into main

parents b4506f65 f2ae0e31
Loading
Loading
Loading
Loading
+58 −55
Original line number Diff line number Diff line
@@ -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,25 +144,55 @@ public class ClientTransactionListenerController {
    }

    /** Called when starts executing a remote {@link ClientTransaction}. */
    @MainThread
    public void onClientTransactionStarted() {
        synchronized (mLock) {
            mIsClientTransactionExecuting = true;
        }
    }

    /** Called when finishes executing a remote {@link ClientTransaction}. */
    @MainThread
    public void onClientTransactionFinished() {
        notifyDisplayManagerIfNeeded();
        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;
        }
        synchronized (mLock) {
            if (mContextToPreChangedConfigMap.containsKey(context)) {
                // There is an earlier change that hasn't been reported yet.
                return;
@@ -172,14 +200,16 @@ public class ClientTransactionListenerController {
            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;
        }
        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.
@@ -187,39 +217,12 @@ public class ClientTransactionListenerController {
            }
            final Configuration preChangedConfig = mContextToPreChangedConfigMap.remove(context);
            if (preChangedConfig != null && shouldReportDisplayChange(context, preChangedConfig)) {
            onDisplayChanged(context.getDisplayId());
                changedDisplayId = 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());
                }
            }

            // 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);
            }
        } finally {
            mContextToPreChangedConfigMap.clear();
        if (changedDisplayId != INVALID_DISPLAY) {
            onDisplayChanged(changedDisplayId);
        }
    }

+4 −3
Original line number Diff line number Diff line
@@ -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);
+10 −0
Original line number Diff line number Diff line
@@ -91,6 +91,16 @@ flag {
    is_fixed_read_only: true
}

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"
+3 −0
Original line number Diff line number Diff line
@@ -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());