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

Commit 781b7abb authored by Kevin Chyn's avatar Kevin Chyn Committed by Automerger Merge Worker
Browse files

Merge "Do not use Map.Entry outside of lock" into udc-dev am: 4aa81f11

parents 9417fe65 4aa81f11
Loading
Loading
Loading
Loading
+20 −10
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.annotation.CallbackExecutor;
import android.annotation.NonNull;
import android.content.Context;
import android.util.ArrayMap;
import android.util.Pair;

import com.android.internal.R;
import com.android.internal.annotations.GuardedBy;
@@ -165,19 +166,28 @@ final class DeviceStateController {

            // 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<Map.Entry<Consumer<DeviceState>, Executor>> entries = new ArrayList<>();
            synchronized (mWmLock) {
                for (Map.Entry<Consumer<DeviceState>, Executor> entry
                        : mDeviceStateCallbacks.entrySet()) {
                    entries.add(entry);
                }
            }
            // ConcurrentModificationException. Note that cannot use a List<Map.Entry> because the
            // entries are tied to the backing map. So, if a client removes a callback while
            // we are notifying clients, we will get a NPE.
            final List<Pair<Consumer<DeviceState>, Executor>> entries = copyDeviceStateCallbacks();

            for (int i = 0; i < entries.size(); i++) {
                Map.Entry<Consumer<DeviceState>, Executor> entry = entries.get(i);
                entry.getValue().execute(() -> entry.getKey().accept(mCurrentDeviceState));
                final Pair<Consumer<DeviceState>, Executor> entry = entries.get(i);
                entry.second.execute(() -> entry.first.accept(deviceState));
            }
        }
    }

    @VisibleForTesting
    @NonNull
    List<Pair<Consumer<DeviceState>, Executor>> copyDeviceStateCallbacks() {
        final List<Pair<Consumer<DeviceState>, Executor>> entries = new ArrayList<>();

        synchronized (mWmLock) {
            mDeviceStateCallbacks.forEach((deviceStateConsumer, executor) -> {
                entries.add(new Pair<>(deviceStateConsumer, executor));
            });
        }
        return entries;
    }
}
+24 −4
Original line number Diff line number Diff line
@@ -17,17 +17,16 @@
package com.android.server.wm;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.mock;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.when;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;

import android.content.Context;
import android.content.res.Resources;
import android.hardware.devicestate.DeviceStateManager;
import android.platform.test.annotations.Presubmit;
import android.util.Pair;

import androidx.test.filters.SmallTest;

@@ -38,6 +37,8 @@ import com.google.common.util.concurrent.MoreExecutors;
import org.junit.Before;
import org.junit.Test;

import java.util.List;
import java.util.concurrent.Executor;
import java.util.function.Consumer;

/**
@@ -58,6 +59,7 @@ public class DeviceStateControllerTests {
    private DeviceStateController.DeviceState mCurrentState =
            DeviceStateController.DeviceState.UNKNOWN;
    private Consumer<DeviceStateController.DeviceState> mDelegate;
    private Executor mExecutor = MoreExecutors.directExecutor();

    @Before
    public void setUp() {
@@ -124,13 +126,31 @@ public class DeviceStateControllerTests {
        mTarget.onDeviceStateReceivedByDisplayManager(mFoldedStates[0]);
        assertEquals(DeviceStateController.DeviceState.FOLDED, mCurrentState);

        // The callback should not receive state change when the it is unregistered.
        // The callback should not receive state change when it is unregistered.
        mTarget.unregisterDeviceStateCallback(mDelegate);
        assertTrue(mTarget.mDeviceStateCallbacks.isEmpty());
        mTarget.onDeviceStateReceivedByDisplayManager(mOpenDeviceStates[0]);
        assertEquals(DeviceStateController.DeviceState.FOLDED /* unchanged */, mCurrentState);
    }

    @Test
    public void testCopyDeviceStateCallbacks() {
        initialize(true /* supportFold */, true /* supportHalfFolded */);
        assertEquals(1, mTarget.mDeviceStateCallbacks.size());
        assertTrue(mTarget.mDeviceStateCallbacks.containsKey(mDelegate));

        List<Pair<Consumer<DeviceStateController.DeviceState>, Executor>> entries =
                mTarget.copyDeviceStateCallbacks();
        mTarget.unregisterDeviceStateCallback(mDelegate);

        // In contrast to List<Map.Entry> where the entries are tied to changes in the backing map,
        // List<Pair> should still contain non-null callbacks and executors even though they were
        // removed from the backing map via the unregister method above.
        assertEquals(1, entries.size());
        assertEquals(mDelegate, entries.get(0).first);
        assertEquals(mExecutor, entries.get(0).second);
    }

    private final int[] mFoldedStates = {0};
    private final int[] mOpenDeviceStates = {1};
    private final int[] mHalfFoldedStates = {2};
@@ -194,7 +214,7 @@ public class DeviceStateControllerTests {
            when(mMockContext.getResources()).thenReturn((mockRes));
            mockFold(mSupportFold, mSupportHalfFold);
            mTarget = new DeviceStateController(mMockContext, new WindowManagerGlobalLock());
            mTarget.registerDeviceStateCallback(mDelegate, MoreExecutors.directExecutor());
            mTarget.registerDeviceStateCallback(mDelegate, mExecutor);
        }
    }
}