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

Commit 2fa48adb authored by Eric Lin's avatar Eric Lin
Browse files

Revert DeviceState callback to main thread in FFP.

This partially reverts commit I8a4af1c5ac98f5653e4293a049291ba5ff7cd464.

Revert device state callback to the main thread in FFP to fix crashes in
Chrome and Photos. The original change allowed callbacks off the main
thread but this caused crashes because:
1. Chrome loads window extension directly; it crashs when receiving
   callbacks on binder thread.
2. Photos uses Java API with Runnable::run, breaking the flowOn main
   operator; it crashs when receiving callbacks on the binder thread.

Window layout info remains available via getter because the device state
is available synchronously in the DeviceStateManagerGlobal constructor,
so this rollback does not impact functionality.

Bug: 337820752
Bug: 379501835
Test: atest WMJetpackUnitTests:DeviceStateManagerFoldingFeatureProducerTest
Test: Run with latest Chrome (131.0.6778.39)
Flag: com.android.window.flags.wlinfo_oncreate
Change-Id: Ifc5af9955a27b6cfb5405e63c0763f3c55a5a75e
parent 99e033fd
Loading
Loading
Loading
Loading
+13 −52
Original line number Diff line number Diff line
@@ -30,8 +30,6 @@ import android.text.TextUtils;
import android.util.Log;
import android.util.SparseIntArray;

import androidx.annotation.BinderThread;
import androidx.annotation.GuardedBy;
import androidx.annotation.MainThread;
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
@@ -39,14 +37,12 @@ import androidx.window.common.layout.CommonFoldingFeature;
import androidx.window.common.layout.DisplayFoldFeatureCommon;

import com.android.internal.R;
import com.android.window.flags.Flags;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.Executor;
import java.util.function.Consumer;

/**
@@ -70,20 +66,8 @@ public final class DeviceStateManagerFoldingFeatureProducer
     * "rear display". Concurrent mode for example is activated via public API and can be active in
     * both the "open" and "half folded" device states.
     */
    // TODO: b/337820752 - Add @GuardedBy("mCurrentDeviceStateLock") after flag cleanup.
    private DeviceState mCurrentDeviceState = INVALID_DEVICE_STATE;

    /**
     * Lock to synchronize access to {@link #mCurrentDeviceState}.
     *
     * <p>This lock is used to ensure thread-safety when accessing and modifying the
     * {@link #mCurrentDeviceState} field. It is acquired by both the binder thread (if
     * {@link Flags#wlinfoOncreate()} is enabled) and the main thread (if
     * {@link Flags#wlinfoOncreate()} is disabled) to prevent race conditions and
     * ensure data consistency.
     */
    private final Object mCurrentDeviceStateLock = new Object();

    @NonNull
    private final RawFoldingFeatureProducer mRawFoldSupplier;

@@ -95,16 +79,13 @@ public final class DeviceStateManagerFoldingFeatureProducer
        // The GuardedBy analysis is intra-procedural, meaning it doesn’t consider the getData()
        // implementation. See https://errorprone.info/bugpattern/GuardedBy for limitations.
        @SuppressWarnings("GuardedBy")
        @BinderThread // When Flags.wlinfoOncreate() is enabled.
        @MainThread // When Flags.wlinfoOncreate() is disabled.
        @MainThread
        @Override
        public void onDeviceStateChanged(@NonNull DeviceState state) {
            synchronized (mCurrentDeviceStateLock) {
            mCurrentDeviceState = state;
            mRawFoldSupplier.getData(DeviceStateManagerFoldingFeatureProducer.this
                    ::notifyFoldingFeatureChangeLocked);
        }
        }
    };

    public DeviceStateManagerFoldingFeatureProducer(@NonNull Context context,
@@ -115,10 +96,8 @@ public final class DeviceStateManagerFoldingFeatureProducer
                new DeviceStateMapper(context, deviceStateManager.getSupportedDeviceStates());

        if (!mDeviceStateMapper.isDeviceStateToPostureMapEmpty()) {
            final Executor executor =
                    Flags.wlinfoOncreate() ? Runnable::run : context.getMainExecutor();
            Objects.requireNonNull(deviceStateManager)
                    .registerCallback(executor, mDeviceStateCallback);
                    .registerCallback(context.getMainExecutor(), mDeviceStateCallback);
        }
    }

@@ -145,7 +124,6 @@ public final class DeviceStateManagerFoldingFeatureProducer
    @Override
    protected void onListenersChanged() {
        super.onListenersChanged();
        synchronized (mCurrentDeviceStateLock) {
        if (hasListeners()) {
            mRawFoldSupplier.addDataChangedCallback(this::notifyFoldingFeatureChangeLocked);
        } else {
@@ -153,14 +131,11 @@ public final class DeviceStateManagerFoldingFeatureProducer
            mRawFoldSupplier.removeDataChangedCallback(this::notifyFoldingFeatureChangeLocked);
        }
    }
    }

    @NonNull
    private DeviceState getCurrentDeviceState() {
        synchronized (mCurrentDeviceStateLock) {
        return mCurrentDeviceState;
    }
    }

    @NonNull
    @Override
@@ -231,7 +206,6 @@ public final class DeviceStateManagerFoldingFeatureProducer
        });
    }

    @GuardedBy("mCurrentDeviceStateLock")
    private void notifyFoldingFeatureChangeLocked(String displayFeaturesString) {
        final DeviceState state = mCurrentDeviceState;
        if (!mDeviceStateMapper.isDeviceStateValid(state)) {
@@ -252,29 +226,16 @@ public final class DeviceStateManagerFoldingFeatureProducer
        return parseListFromString(displayFeaturesString, hingeState);
    }

    /**
     * Internal class to map device states to corresponding postures.
     *
     * <p>This class encapsulates the logic for mapping device states to postures. The mapping is
     * immutable after initialization to ensure thread safety.
     */
    /** Internal class to map device states to corresponding postures. */
    private static class DeviceStateMapper {
        /**
         * Emulated device state
         * {@link DeviceStateManager.DeviceStateCallback#onDeviceStateChanged(DeviceState)} to
         * {@link CommonFoldingFeature.State} map.
         *
         * <p>This map must be immutable after initialization to ensure thread safety, as it may be
         * accessed from multiple threads. Modifications should only occur during object
         * construction.
         */
        private final SparseIntArray mDeviceStateToPostureMap = new SparseIntArray();

        /**
         * The list of device states that are supported.
         *
         * <p>This list must be immutable after initialization to ensure thread safety.
         */
        /** The list of device states that are supported. */
        @NonNull
        private final List<DeviceState> mSupportedStates;

+1 −29
Original line number Diff line number Diff line
@@ -20,9 +20,6 @@ import android.content.Context
import android.content.res.Resources
import android.hardware.devicestate.DeviceState
import android.hardware.devicestate.DeviceStateManager
import android.platform.test.annotations.DisableFlags
import android.platform.test.annotations.EnableFlags
import android.platform.test.flag.junit.SetFlagsRule
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.window.common.layout.CommonFoldingFeature
import androidx.window.common.layout.CommonFoldingFeature.COMMON_STATE_FLAT
@@ -34,15 +31,11 @@ import androidx.window.common.layout.DisplayFoldFeatureCommon
import androidx.window.common.layout.DisplayFoldFeatureCommon.DISPLAY_FOLD_FEATURE_PROPERTY_SUPPORTS_HALF_OPENED
import androidx.window.common.layout.DisplayFoldFeatureCommon.DISPLAY_FOLD_FEATURE_TYPE_SCREEN_FOLD_IN
import com.android.internal.R
import com.android.window.flags.Flags
import com.google.common.truth.Truth.assertThat
import java.util.Optional
import java.util.concurrent.Executor
import java.util.function.Consumer
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.kotlin.any
import org.mockito.kotlin.doAnswer
import org.mockito.kotlin.doReturn
@@ -60,9 +53,6 @@ import org.mockito.kotlin.verify
 */
@RunWith(AndroidJUnit4::class)
class DeviceStateManagerFoldingFeatureProducerTest {
    @get:Rule
    val setFlagsRule: SetFlagsRule = SetFlagsRule()

    private val mMockDeviceStateManager = mock<DeviceStateManager>()
    private val mMockResources = mock<Resources> {
        on { getStringArray(R.array.config_device_state_postures) } doReturn DEVICE_STATE_POSTURES
@@ -79,8 +69,7 @@ class DeviceStateManagerFoldingFeatureProducerTest {
    }

    @Test
    @DisableFlags(Flags.FLAG_WLINFO_ONCREATE)
    fun testRegisterCallback_whenWlinfoOncreateIsDisabled_usesMainExecutor() {
    fun testRegisterCallback_usesMainExecutor() {
        DeviceStateManagerFoldingFeatureProducer(
            mMockContext,
            mRawFoldSupplier,
@@ -90,23 +79,6 @@ class DeviceStateManagerFoldingFeatureProducerTest {
        verify(mMockDeviceStateManager).registerCallback(eq(mMockContext.mainExecutor), any())
    }

    @Test
    @EnableFlags(Flags.FLAG_WLINFO_ONCREATE)
    fun testRegisterCallback_whenWlinfoOncreateIsEnabled_usesRunnableRun() {
        val executorCaptor = ArgumentCaptor.forClass(Executor::class.java)
        val runnable = mock<Runnable>()

        DeviceStateManagerFoldingFeatureProducer(
            mMockContext,
            mRawFoldSupplier,
            mMockDeviceStateManager,
        )

        verify(mMockDeviceStateManager).registerCallback(executorCaptor.capture(), any())
        executorCaptor.value.execute(runnable)
        verify(runnable).run()
    }

    @Test
    fun testGetCurrentData_validCurrentState_returnsFoldingFeatureWithState() {
        val ffp = DeviceStateManagerFoldingFeatureProducer(