From 4dda2323e9803de7c7f5d080389457d1128106ec Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Tue, 15 Jun 2021 10:11:35 -0400 Subject: [PATCH] New MessageRouter interface to replace Handler. Proposal introduced in http://go/sysui-message-router This introduces a testable, controllerable MessageRouter interface intended to replace Handler#sendMessage. Listeners subscribe to specific messages rather than subclassing Handler. Also, metadata can be passed in any form, rather than the generic android.os.Message. To use, developers simply inject either an @Main or a @Background MessageRouter. This provides them with a new instance of a MessageRouter, tied to the corresponding Executor. As an example, the internal Handler subclass in GarbageMonitor has been replaced. There is an overall reduction in the number of lines of code, and the corresponding test has been refactored to be more deterministic and useful. Bug: 191077376 Test: atest SystemUITests Change-Id: Ibd1e05b1b5b7421c2085d8811b8f5cd61cf0ffde --- .../util/concurrency/MessageRouter.java | 198 ++++++++++ .../util/concurrency/MessageRouterImpl.java | 186 +++++++++ .../concurrency/SysUIConcurrencyModule.java | 16 + .../systemui/util/leak/GarbageMonitor.java | 59 ++- .../concurrency/MessageRouterImplTest.java | 371 ++++++++++++++++++ .../util/leak/GarbageMonitorTest.java | 102 ++--- 6 files changed, 831 insertions(+), 101 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/util/concurrency/MessageRouter.java create mode 100644 packages/SystemUI/src/com/android/systemui/util/concurrency/MessageRouterImpl.java create mode 100644 packages/SystemUI/tests/src/com/android/systemui/util/concurrency/MessageRouterImplTest.java diff --git a/packages/SystemUI/src/com/android/systemui/util/concurrency/MessageRouter.java b/packages/SystemUI/src/com/android/systemui/util/concurrency/MessageRouter.java new file mode 100644 index 0000000000000..542cf6559c64d --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/util/concurrency/MessageRouter.java @@ -0,0 +1,198 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.util.concurrency; + +/** + * Allows triggering methods based on a passed in id or message, generally on another thread. + * + * Messages sent on to this router must be processed in order. That is to say, if three + * messages are sent with no delay, they must be processed in the order they were sent. Moreover, + * if messages are sent with various delays, they must be processed in order of their delay. + * + * Messages can be passed by either a simple integer or an instance of a class. Unique integers are + * considered unique messages. Unique message classes (not instances) are considered unique + * messages. You can use message classes to pass extra data for processing to subscribers. + * + * 
+ *      // Three messages with three unique integer messages.
+ *      // They can be subscribed to independently.
+ *      router.sendMessage(0);
+ *      router.sendMessage(1);
+ *      router.sendMessage(2);
+ *
+ *      // Three messages with two unique message classes.
+ *      // The first and third messages will be delivered to the same subscribers.
+ *      router.sendMessage(new Foo(0));
+ *      router.sendMessage(new Bar(1));
+ *      router.sendMessage(new Foo(2));
+ *
+ * + * The number of unique ids and message types used should be relatively constrained. Construct + * a custom message-class and put unique, per-message data inside of it. + */ +public interface MessageRouter { + /** + * Alerts any listeners subscribed to the passed in id. + * + * The number of unique ids used should be relatively constrained - used to identify the type + * of message being sent. If unique information needs to be passed with each call, use + * {@link #sendMessage(Object)}. + * + * @param id An identifier for the message + */ + default void sendMessage(int id) { + sendMessageDelayed(id, 0); + } + + /** + * Alerts any listeners subscribed to the passed in message. + * + * The number of message types used should be relatively constrained. If no unique information + * needs to be passed in, you can simply use {@link #sendMessage(int)}} which takes an integer + * instead of a unique class type. + * + * The class of the passed in object will be used to router the message. + * + * @param data A message containing extra data for processing. + */ + default void sendMessage(Object data) { + sendMessageDelayed(data, 0); + } + + /** + * Alerts any listeners subscribed to the passed in id in the future. + * + * The number of unique ids used should be relatively constrained - used to identify the type + * of message being sent. If unique information needs to be passed with each call, use + * {@link #sendMessageDelayed(Object, long)}. + * + * @param id An identifier for the message + * @param delayMs Number of milliseconds to wait before alerting. + */ + void sendMessageDelayed(int id, long delayMs); + + + /** + * Alerts any listeners subscribed to the passed in message in the future. + * + * The number of message types used should be relatively constrained. If no unique information + * needs to be passed in, you can simply use {@link #sendMessageDelayed(int, long)} which takes + * an integer instead of a unique class type. + * + * @param data A message containing extra data for processing. + * @param delayMs Number of milliseconds to wait before alerting. + */ + void sendMessageDelayed(Object data, long delayMs); + + /** + * Cancel all unprocessed messages for a given id. + * + * If a message has multiple listeners and one of those listeners has been alerted, the other + * listeners that follow it may also be alerted. This is only guaranteed to cancel messages + * that are still queued. + * + * @param id The message id to cancel. + */ + void cancelMessages(int id); + + /** + * Cancel all unprocessed messages for a given message type. + * + * If a message has multiple listeners and one of those listeners has been alerted, the other + * listeners that follow it may also be alerted. This is only guaranteed to cancel messages + * that are still queued. + * + * @param messageType The class of the message to cancel + */ + void cancelMessages(Class messageType); + + /** + * Add a listener for a message that does not handle any extra data. + * + * See also {@link #subscribeTo(Class, DataMessageListener)}. + * + * @param id The message id to listener for. + * @param listener + */ + void subscribeTo(int id, SimpleMessageListener listener); + + /** + * Add a listener for a message of a specific type. + * + * See also {@link #subscribeTo(Class, DataMessageListener)}. + * + * @param messageType The class of message to listen for. + * @param listener + */ + void subscribeTo(Class messageType, DataMessageListener listener); + + /** + * Remove a listener for a specific message. + * + * See also {@link #unsubscribeFrom(Class, DataMessageListener)} + * + * @param id The message id to stop listening for. + * @param listener The listener to remove. + */ + void unsubscribeFrom(int id, SimpleMessageListener listener); + + /** + * Remove a listener for a specific message. + * + * See also {@link #unsubscribeFrom(int, SimpleMessageListener)}. + * + * @param messageType The class of message to stop listening for. + * @param listener The listener to remove. + */ + void unsubscribeFrom(Class messageType, DataMessageListener listener); + + /** + * Remove a listener for all messages that it is subscribed to. + * + * See also {@link #unsubscribeFrom(DataMessageListener)}. + * + * @param listener The listener to remove. + */ + void unsubscribeFrom(SimpleMessageListener listener); + + /** + * Remove a listener for all messages that it is subscribed to. + * + * See also {@link #unsubscribeFrom(SimpleMessageListener)}. + * + * @param listener The listener to remove. + */ + void unsubscribeFrom(DataMessageListener listener); + + /** + * A Listener interface for when no extra data is expected or desired. + */ + interface SimpleMessageListener { + /** */ + void onMessage(int id); + } + + /** + * A Listener interface for when extra data is expected or desired. + * + * @param + */ + interface DataMessageListener { + /** */ + void onMessage(T data); + } +} diff --git a/packages/SystemUI/src/com/android/systemui/util/concurrency/MessageRouterImpl.java b/packages/SystemUI/src/com/android/systemui/util/concurrency/MessageRouterImpl.java new file mode 100644 index 0000000000000..7145c775fe399 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/util/concurrency/MessageRouterImpl.java @@ -0,0 +1,186 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.util.concurrency; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * Implementation of {@link MessageRouter}. + */ +public class MessageRouterImpl implements MessageRouter { + + private final DelayableExecutor mDelayableExecutor; + + private final Map> mIdMessageCancelers = new HashMap<>(); + private final Map, List> mDataMessageCancelers = new HashMap<>(); + private final Map> mSimpleMessageListenerMap = + new HashMap<>(); + private final Map, List>> mDataMessageListenerMap = + new HashMap<>(); + + public MessageRouterImpl(DelayableExecutor delayableExecutor) { + mDelayableExecutor = delayableExecutor; + } + + @Override + public void sendMessageDelayed(int id, long delayMs) { + addCanceler(id, mDelayableExecutor.executeDelayed(() -> onMessage(id), delayMs)); + } + + @Override + public void sendMessageDelayed(Object data, long delayMs) { + addCanceler((Class) data.getClass(), mDelayableExecutor.executeDelayed( + () -> onMessage(data), delayMs)); + } + + @Override + public void cancelMessages(int id) { + synchronized (mIdMessageCancelers) { + if (mIdMessageCancelers.containsKey(id)) { + for (Runnable canceler : mIdMessageCancelers.get(id)) { + canceler.run(); + } + // Remove, don't clear, otherwise this could look like a memory leak as + // more and more unique message ids are passed in. + mIdMessageCancelers.remove(id); + } + } + } + + @Override + public void cancelMessages(Class messageType) { + synchronized (mDataMessageCancelers) { + if (mDataMessageCancelers.containsKey(messageType)) { + for (Runnable canceler : mDataMessageCancelers.get(messageType)) { + canceler.run(); + } + // Remove, don't clear, otherwise this could look like a memory leak as + // more and more unique message types are passed in. + mDataMessageCancelers.remove(messageType); + } + } + } + + @Override + public void subscribeTo(int id, SimpleMessageListener listener) { + synchronized (mSimpleMessageListenerMap) { + mSimpleMessageListenerMap.putIfAbsent(id, new ArrayList<>()); + mSimpleMessageListenerMap.get(id).add(listener); + } + } + + @Override + public void subscribeTo(Class messageType, DataMessageListener listener) { + synchronized (mDataMessageListenerMap) { + mDataMessageListenerMap.putIfAbsent(messageType, new ArrayList<>()); + mDataMessageListenerMap.get(messageType).add((DataMessageListener) listener); + } + } + + @Override + public void unsubscribeFrom(int id, SimpleMessageListener listener) { + synchronized (mSimpleMessageListenerMap) { + if (mSimpleMessageListenerMap.containsKey(id)) { + mSimpleMessageListenerMap.get(id).remove(listener); + } + } + } + + @Override + public void unsubscribeFrom(Class messageType, DataMessageListener listener) { + synchronized (mDataMessageListenerMap) { + if (mDataMessageListenerMap.containsKey(messageType)) { + mDataMessageListenerMap.get(messageType).remove(listener); + } + } + } + + @Override + public void unsubscribeFrom(SimpleMessageListener listener) { + synchronized (mSimpleMessageListenerMap) { + for (Integer id : mSimpleMessageListenerMap.keySet()) { + mSimpleMessageListenerMap.get(id).remove(listener); + } + } + } + + @Override + public void unsubscribeFrom(DataMessageListener listener) { + synchronized (mDataMessageListenerMap) { + for (Class messageType : mDataMessageListenerMap.keySet()) { + mDataMessageListenerMap.get(messageType).remove(listener); + } + } + } + + private void addCanceler(int id, Runnable canceler) { + synchronized (mIdMessageCancelers) { + mIdMessageCancelers.putIfAbsent(id, new ArrayList<>()); + mIdMessageCancelers.get(id).add(canceler); + } + } + + private void addCanceler(Class data, Runnable canceler) { + synchronized (mDataMessageCancelers) { + mDataMessageCancelers.putIfAbsent(data, new ArrayList<>()); + mDataMessageCancelers.get(data).add(canceler); + } + } + + private void onMessage(int id) { + synchronized (mSimpleMessageListenerMap) { + if (mSimpleMessageListenerMap.containsKey(id)) { + for (SimpleMessageListener listener : mSimpleMessageListenerMap.get(id)) { + listener.onMessage(id); + } + } + } + + synchronized (mIdMessageCancelers) { + if (mIdMessageCancelers.containsKey(id) && !mIdMessageCancelers.get(id).isEmpty()) { + mIdMessageCancelers.get(id).remove(0); + if (mIdMessageCancelers.get(id).isEmpty()) { + mIdMessageCancelers.remove(id); + } + } + } + } + + private void onMessage(Object data) { + synchronized (mDataMessageListenerMap) { + if (mDataMessageListenerMap.containsKey(data.getClass())) { + for (DataMessageListener listener : mDataMessageListenerMap.get( + data.getClass())) { + listener.onMessage(data); + } + } + } + + synchronized (mDataMessageCancelers) { + if (mDataMessageCancelers.containsKey(data.getClass()) + && !mDataMessageCancelers.get(data.getClass()).isEmpty()) { + mDataMessageCancelers.get(data.getClass()).remove(0); + if (mDataMessageCancelers.get(data.getClass()).isEmpty()) { + mDataMessageCancelers.remove(data.getClass()); + } + } + } + } +} diff --git a/packages/SystemUI/src/com/android/systemui/util/concurrency/SysUIConcurrencyModule.java b/packages/SystemUI/src/com/android/systemui/util/concurrency/SysUIConcurrencyModule.java index b9b20c73c5d57..e9e794ea884b6 100644 --- a/packages/SystemUI/src/com/android/systemui/util/concurrency/SysUIConcurrencyModule.java +++ b/packages/SystemUI/src/com/android/systemui/util/concurrency/SysUIConcurrencyModule.java @@ -170,4 +170,20 @@ public abstract class SysUIConcurrencyModule { public static Executor provideUiBackgroundExecutor() { return Executors.newSingleThreadExecutor(); } + + /** */ + @Provides + @Main + public static MessageRouter providesMainMessageRouter( + @Main DelayableExecutor executor) { + return new MessageRouterImpl(executor); + } + + /** */ + @Provides + @Background + public static MessageRouter providesBackgroundMessageRouter( + @Background DelayableExecutor executor) { + return new MessageRouterImpl(executor); + } } diff --git a/packages/SystemUI/src/com/android/systemui/util/leak/GarbageMonitor.java b/packages/SystemUI/src/com/android/systemui/util/leak/GarbageMonitor.java index edea3055a783a..c7998882876ac 100644 --- a/packages/SystemUI/src/com/android/systemui/util/leak/GarbageMonitor.java +++ b/packages/SystemUI/src/com/android/systemui/util/leak/GarbageMonitor.java @@ -36,7 +36,6 @@ import android.graphics.drawable.Drawable; import android.os.Build; import android.os.Handler; import android.os.Looper; -import android.os.Message; import android.os.Process; import android.os.SystemProperties; import android.provider.Settings; @@ -60,6 +59,8 @@ import com.android.systemui.qs.QSHost; import com.android.systemui.qs.logging.QSLogger; import com.android.systemui.qs.tileimpl.QSIconViewImpl; import com.android.systemui.qs.tileimpl.QSTileImpl; +import com.android.systemui.util.concurrency.DelayableExecutor; +import com.android.systemui.util.concurrency.MessageRouter; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -110,18 +111,18 @@ public class GarbageMonitor implements Dumpable { private static final int DO_GARBAGE_INSPECTION = 1000; private static final int DO_HEAP_TRACK = 3000; - private static final int GARBAGE_ALLOWANCE = 5; + static final int GARBAGE_ALLOWANCE = 5; private static final String TAG = "GarbageMonitor"; private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); - private final Handler mHandler; + private final MessageRouter mMessageRouter; private final TrackedGarbage mTrackedGarbage; private final LeakReporter mLeakReporter; private final Context mContext; - private final ActivityManager mAm; + private final DelayableExecutor mDelayableExecutor; private MemoryTile mQSTile; - private DumpTruck mDumpTruck; + private final DumpTruck mDumpTruck; private final LongSparseArray mData = new LongSparseArray<>(); private final ArrayList mPids = new ArrayList<>(); @@ -133,13 +134,16 @@ public class GarbageMonitor implements Dumpable { @Inject public GarbageMonitor( Context context, - @Background Looper bgLooper, + @Background DelayableExecutor delayableExecutor, + @Background MessageRouter messageRouter, LeakDetector leakDetector, LeakReporter leakReporter) { mContext = context.getApplicationContext(); - mAm = (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE); - mHandler = new BackgroundHeapCheckHandler(bgLooper); + mDelayableExecutor = delayableExecutor; + mMessageRouter = messageRouter; + mMessageRouter.subscribeTo(DO_GARBAGE_INSPECTION, this::doGarbageInspection); + mMessageRouter.subscribeTo(DO_HEAP_TRACK, this::doHeapTrack); mTrackedGarbage = leakDetector.getTrackedGarbage(); mLeakReporter = leakReporter; @@ -158,13 +162,13 @@ public class GarbageMonitor implements Dumpable { return; } - mHandler.sendEmptyMessage(DO_GARBAGE_INSPECTION); + mMessageRouter.sendMessage(DO_GARBAGE_INSPECTION); } public void startHeapTracking() { startTrackingProcess( android.os.Process.myPid(), mContext.getPackageName(), System.currentTimeMillis()); - mHandler.sendEmptyMessage(DO_HEAP_TRACK); + mMessageRouter.sendMessage(DO_HEAP_TRACK); } private boolean gcAndCheckGarbage() { @@ -586,33 +590,18 @@ public class GarbageMonitor implements Dumpable { } } - private class BackgroundHeapCheckHandler extends Handler { - BackgroundHeapCheckHandler(Looper onLooper) { - super(onLooper); - if (Looper.getMainLooper().equals(onLooper)) { - throw new RuntimeException( - "BackgroundHeapCheckHandler may not run on the ui thread"); - } + private void doGarbageInspection(int id) { + if (gcAndCheckGarbage()) { + mDelayableExecutor.executeDelayed(this::reinspectGarbageAfterGc, 100); } - @Override - public void handleMessage(Message m) { - switch (m.what) { - case DO_GARBAGE_INSPECTION: - if (gcAndCheckGarbage()) { - postDelayed(GarbageMonitor.this::reinspectGarbageAfterGc, 100); - } + mMessageRouter.cancelMessages(DO_GARBAGE_INSPECTION); + mMessageRouter.sendMessageDelayed(DO_GARBAGE_INSPECTION, GARBAGE_INSPECTION_INTERVAL); + } - removeMessages(DO_GARBAGE_INSPECTION); - sendEmptyMessageDelayed(DO_GARBAGE_INSPECTION, GARBAGE_INSPECTION_INTERVAL); - break; - - case DO_HEAP_TRACK: - update(); - removeMessages(DO_HEAP_TRACK); - sendEmptyMessageDelayed(DO_HEAP_TRACK, HEAP_TRACK_INTERVAL); - break; - } - } + private void doHeapTrack(int id) { + update(); + mMessageRouter.cancelMessages(DO_HEAP_TRACK); + mMessageRouter.sendMessageDelayed(DO_HEAP_TRACK, HEAP_TRACK_INTERVAL); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/concurrency/MessageRouterImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/util/concurrency/MessageRouterImplTest.java new file mode 100644 index 0000000000000..78fc6803ea7ec --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/util/concurrency/MessageRouterImplTest.java @@ -0,0 +1,371 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.util.concurrency; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; + +import android.testing.AndroidTestingRunner; + +import androidx.test.filters.SmallTest; + +import com.android.systemui.SysuiTestCase; +import com.android.systemui.util.time.FakeSystemClock; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InOrder; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +@SmallTest +@RunWith(AndroidTestingRunner.class) +public class MessageRouterImplTest extends SysuiTestCase { + private static final int MESSAGE_A = 0; + private static final int MESSAGE_B = 1; + private static final int MESSAGE_C = 2; + + private static final String METADATA_A = "A"; + private static final String METADATA_B = "B"; + private static final String METADATA_C = "C"; + private static final Foobar METADATA_FOO = new Foobar(); + + FakeExecutor mFakeExecutor = new FakeExecutor(new FakeSystemClock()); + @Mock + MessageRouter.SimpleMessageListener mNoMdListener; + @Mock + MessageRouter.DataMessageListener mStringListener; + @Mock + MessageRouter.DataMessageListener mFoobarListener; + private MessageRouterImpl mMR; + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + mMR = new MessageRouterImpl(mFakeExecutor); + } + + @Test + public void testSingleMessage_NoMetaData() { + mMR.subscribeTo(MESSAGE_A, mNoMdListener); + + mMR.sendMessage(MESSAGE_A); + verify(mNoMdListener, never()).onMessage(anyInt()); + + mFakeExecutor.runAllReady(); + verify(mNoMdListener).onMessage(MESSAGE_A); + } + + @Test + public void testSingleMessage_WithMetaData() { + mMR.subscribeTo(String.class, mStringListener); + + mMR.sendMessage(METADATA_A); + verify(mStringListener, never()).onMessage(anyString()); + + mFakeExecutor.runAllReady(); + verify(mStringListener).onMessage(METADATA_A); + } + + @Test + public void testMessages_WithMixedMetaData() { + mMR.subscribeTo(String.class, mStringListener); + mMR.subscribeTo(Foobar.class, mFoobarListener); + + mMR.sendMessage(METADATA_A); + verify(mStringListener, never()).onMessage(anyString()); + verify(mFoobarListener, never()).onMessage(any(Foobar.class)); + + mFakeExecutor.runAllReady(); + verify(mStringListener).onMessage(METADATA_A); + verify(mFoobarListener, never()).onMessage(any(Foobar.class)); + + reset(mStringListener); + reset(mFoobarListener); + + mMR.sendMessage(METADATA_FOO); + verify(mStringListener, never()).onMessage(anyString()); + verify(mFoobarListener, never()).onMessage(any(Foobar.class)); + + mFakeExecutor.runAllReady(); + verify(mStringListener, never()).onMessage(anyString()); + verify(mFoobarListener).onMessage(METADATA_FOO); + } + + @Test + public void testMessages_WithAndWithoutMetaData() { + mMR.subscribeTo(MESSAGE_A, mNoMdListener); + mMR.subscribeTo(String.class, mStringListener); + + mMR.sendMessage(MESSAGE_A); + verify(mNoMdListener, never()).onMessage(anyInt()); + verify(mStringListener, never()).onMessage(anyString()); + + mFakeExecutor.runAllReady(); + verify(mNoMdListener).onMessage(MESSAGE_A); + verify(mStringListener, never()).onMessage(anyString()); + + reset(mNoMdListener); + reset(mStringListener); + + mMR.sendMessage(METADATA_A); + verify(mNoMdListener, never()).onMessage(anyInt()); + verify(mStringListener, never()).onMessage(anyString()); + + mFakeExecutor.runAllReady(); + verify(mNoMdListener, never()).onMessage(anyInt()); + verify(mStringListener).onMessage(METADATA_A); + } + + @Test + public void testRepeatedMessage() { + mMR.subscribeTo(String.class, mStringListener); + + mMR.sendMessage(METADATA_A); + mMR.sendMessage(METADATA_A); + mMR.sendMessage(METADATA_B); + verify(mStringListener, never()).onMessage(anyString()); + + InOrder ordered = inOrder(mStringListener); + mFakeExecutor.runNextReady(); + ordered.verify(mStringListener).onMessage(METADATA_A); + mFakeExecutor.runNextReady(); + ordered.verify(mStringListener).onMessage(METADATA_A); + mFakeExecutor.runNextReady(); + ordered.verify(mStringListener).onMessage(METADATA_B); + } + + @Test + public void testCancelMessage() { + mMR.subscribeTo(MESSAGE_A, mNoMdListener); + mMR.subscribeTo(MESSAGE_B, mNoMdListener); + mMR.subscribeTo(MESSAGE_C, mNoMdListener); + + mMR.sendMessage(MESSAGE_A); + mMR.sendMessage(MESSAGE_B); + mMR.sendMessage(MESSAGE_B); + mMR.sendMessage(MESSAGE_B); + mMR.sendMessage(MESSAGE_B); + mMR.sendMessage(MESSAGE_C); + verify(mNoMdListener, never()).onMessage(anyInt()); + + mMR.cancelMessages(MESSAGE_B); + + mFakeExecutor.runAllReady(); + + InOrder ordered = inOrder(mNoMdListener); + ordered.verify(mNoMdListener).onMessage(MESSAGE_A); + ordered.verify(mNoMdListener).onMessage(MESSAGE_C); + } + + @Test + public void testSendMessage_NoSubscriber() { + mMR.sendMessage(MESSAGE_A); + verify(mNoMdListener, never()).onMessage(anyInt()); + + mFakeExecutor.runAllReady(); + verify(mNoMdListener, never()).onMessage(anyInt()); + + mMR.subscribeTo(MESSAGE_A, mNoMdListener); + + mFakeExecutor.runAllReady(); + verify(mNoMdListener, never()).onMessage(anyInt()); + + mMR.sendMessage(MESSAGE_A); + verify(mNoMdListener, never()).onMessage(anyInt()); + + mFakeExecutor.runAllReady(); + verify(mNoMdListener).onMessage(MESSAGE_A); + } + + @Test + public void testUnsubscribe_SpecificMessage_NoMetadata() { + mMR.subscribeTo(MESSAGE_A, mNoMdListener); + mMR.subscribeTo(MESSAGE_B, mNoMdListener); + mMR.sendMessage(MESSAGE_A); + mMR.sendMessage(MESSAGE_B); + + mFakeExecutor.runAllReady(); + InOrder ordered = inOrder(mNoMdListener); + ordered.verify(mNoMdListener).onMessage(MESSAGE_A); + ordered.verify(mNoMdListener).onMessage(MESSAGE_B); + + reset(mNoMdListener); + mMR.unsubscribeFrom(MESSAGE_A, mNoMdListener); + mMR.sendMessage(MESSAGE_A); + mMR.sendMessage(MESSAGE_B); + + mFakeExecutor.runAllReady(); + verify(mNoMdListener, never()).onMessage(MESSAGE_A); + verify(mNoMdListener).onMessage(MESSAGE_B); + } + + @Test + public void testUnsubscribe_SpecificMessage_WithMetadata() { + mMR.subscribeTo(String.class, mStringListener); + mMR.subscribeTo(Foobar.class, mFoobarListener); + mMR.sendMessage(METADATA_A); + mMR.sendMessage(METADATA_FOO); + + mFakeExecutor.runNextReady(); + verify(mStringListener).onMessage(METADATA_A); + mFakeExecutor.runNextReady(); + verify(mFoobarListener).onMessage(METADATA_FOO); + + reset(mStringListener); + reset(mFoobarListener); + mMR.unsubscribeFrom(String.class, mStringListener); + mMR.sendMessage(METADATA_A); + mMR.sendMessage(METADATA_FOO); + + mFakeExecutor.runAllReady(); + verify(mStringListener, never()).onMessage(METADATA_A); + verify(mFoobarListener).onMessage(METADATA_FOO); + } + + @Test + public void testUnsubscribe_AllMessages_NoMetadata() { + mMR.subscribeTo(MESSAGE_A, mNoMdListener); + mMR.subscribeTo(MESSAGE_B, mNoMdListener); + mMR.subscribeTo(MESSAGE_C, mNoMdListener); + + mMR.sendMessage(MESSAGE_A); + mMR.sendMessage(MESSAGE_B); + mMR.sendMessage(MESSAGE_C); + + mFakeExecutor.runAllReady(); + verify(mNoMdListener).onMessage(MESSAGE_A); + verify(mNoMdListener).onMessage(MESSAGE_B); + verify(mNoMdListener).onMessage(MESSAGE_C); + + reset(mNoMdListener); + + mMR.unsubscribeFrom(mNoMdListener); + mMR.sendMessage(MESSAGE_A); + mMR.sendMessage(MESSAGE_B); + mMR.sendMessage(MESSAGE_C); + + mFakeExecutor.runAllReady(); + verify(mNoMdListener, never()).onMessage(MESSAGE_A); + verify(mNoMdListener, never()).onMessage(MESSAGE_B); + verify(mNoMdListener, never()).onMessage(MESSAGE_C); + } + + @Test + public void testUnsubscribe_AllMessages_WithMetadata() { + mMR.subscribeTo(String.class, mStringListener); + + mMR.sendMessage(METADATA_A); + mMR.sendMessage(METADATA_B); + mMR.sendMessage(METADATA_C); + + mFakeExecutor.runAllReady(); + verify(mStringListener).onMessage(METADATA_A); + verify(mStringListener).onMessage(METADATA_B); + verify(mStringListener).onMessage(METADATA_C); + + reset(mStringListener); + + mMR.unsubscribeFrom(mStringListener); + mMR.sendMessage(METADATA_A); + mMR.sendMessage(METADATA_B); + mMR.sendMessage(METADATA_C); + + mFakeExecutor.runAllReady(); + verify(mStringListener, never()).onMessage(METADATA_A); + verify(mStringListener, never()).onMessage(METADATA_B); + verify(mStringListener, never()).onMessage(METADATA_C); + } + + @Test + public void testSingleDelayedMessage_NoMetaData() { + mMR.subscribeTo(MESSAGE_A, mNoMdListener); + + mMR.sendMessageDelayed(MESSAGE_A, 100); + verify(mNoMdListener, never()).onMessage(anyInt()); + + mFakeExecutor.runAllReady(); + verify(mNoMdListener, never()).onMessage(anyInt()); + + mFakeExecutor.advanceClockToNext(); + mFakeExecutor.runAllReady(); + verify(mNoMdListener).onMessage(MESSAGE_A); + } + + @Test + public void testSingleDelayedMessage_WithMetaData() { + mMR.subscribeTo(String.class, mStringListener); + + mMR.sendMessageDelayed(METADATA_C, 1000); + verify(mStringListener, never()).onMessage(anyString()); + + mFakeExecutor.runAllReady(); + verify(mStringListener, never()).onMessage(anyString()); + + mFakeExecutor.advanceClockToNext(); + mFakeExecutor.runAllReady(); + verify(mStringListener).onMessage(METADATA_C); + } + + @Test + public void testMultipleDelayedMessages() { + mMR.subscribeTo(String.class, mStringListener); + + mMR.sendMessageDelayed(METADATA_A, 100); + mMR.sendMessageDelayed(METADATA_B, 1000); + mMR.sendMessageDelayed(METADATA_C, 500); + verify(mStringListener, never()).onMessage(anyString()); + + mFakeExecutor.runAllReady(); + verify(mStringListener, never()).onMessage(anyString()); + + mFakeExecutor.advanceClockToLast(); + mFakeExecutor.runAllReady(); + + InOrder ordered = inOrder(mStringListener); + ordered.verify(mStringListener).onMessage(METADATA_A); + ordered.verify(mStringListener).onMessage(METADATA_C); + ordered.verify(mStringListener).onMessage(METADATA_B); + } + + @Test + public void testCancelDelayedMessages() { + mMR.subscribeTo(String.class, mStringListener); + + mMR.sendMessageDelayed(METADATA_A, 100); + mMR.sendMessageDelayed(METADATA_B, 1000); + mMR.sendMessageDelayed(METADATA_C, 500); + verify(mStringListener, never()).onMessage(anyString()); + + mFakeExecutor.runAllReady(); + verify(mStringListener, never()).onMessage(anyString()); + + mMR.cancelMessages(String.class); + mFakeExecutor.advanceClockToLast(); + mFakeExecutor.runAllReady(); + + verify(mStringListener, never()).onMessage(anyString()); + } + + private static class Foobar {} +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/leak/GarbageMonitorTest.java b/packages/SystemUI/tests/src/com/android/systemui/util/leak/GarbageMonitorTest.java index bcc20c2d1b378..724d14e203745 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/util/leak/GarbageMonitorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/util/leak/GarbageMonitorTest.java @@ -17,124 +17,94 @@ package com.android.systemui.util.leak; import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import android.content.Context; -import android.os.Looper; import android.testing.AndroidTestingRunner; -import android.testing.TestableLooper; -import android.testing.TestableLooper.RunWithLooper; import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; +import com.android.systemui.util.concurrency.FakeExecutor; +import com.android.systemui.util.concurrency.MessageRouterImpl; +import com.android.systemui.util.time.FakeSystemClock; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; @SmallTest @RunWith(AndroidTestingRunner.class) -@RunWithLooper public class GarbageMonitorTest extends SysuiTestCase { - private LeakReporter mLeakReporter; - private TrackedGarbage mTrackedGarbage; - private TestableGarbageMonitor mGarbageMonitor; + @Mock private LeakReporter mLeakReporter; + @Mock private TrackedGarbage mTrackedGarbage; + private GarbageMonitor mGarbageMonitor; + private final FakeExecutor mFakeExecutor = new FakeExecutor(new FakeSystemClock()); @Before public void setup() { - mTrackedGarbage = mock(TrackedGarbage.class); - mLeakReporter = mock(LeakReporter.class); + MockitoAnnotations.initMocks(this); mGarbageMonitor = - new TestableGarbageMonitor( + new GarbageMonitor( mContext, - TestableLooper.get(this).getLooper(), + mFakeExecutor, + new MessageRouterImpl(mFakeExecutor), new LeakDetector(null, mTrackedGarbage, null), mLeakReporter); } - @Test - public void testCallbacks_getScheduled() { - mGarbageMonitor.startLeakMonitor(); - mGarbageMonitor.runCallbacksOnce(); - mGarbageMonitor.runCallbacksOnce(); - mGarbageMonitor.runCallbacksOnce(); - } - - @Test - public void testNoGarbage_doesntDump() { - when(mTrackedGarbage.countOldGarbage()).thenReturn(0); - - mGarbageMonitor.startLeakMonitor(); - mGarbageMonitor.runCallbacksOnce(); - mGarbageMonitor.runCallbacksOnce(); - mGarbageMonitor.runCallbacksOnce(); - - verify(mLeakReporter, never()).dumpLeak(anyInt()); - } - @Test public void testALittleGarbage_doesntDump() { - when(mTrackedGarbage.countOldGarbage()).thenReturn(4); + when(mTrackedGarbage.countOldGarbage()).thenReturn(GarbageMonitor.GARBAGE_ALLOWANCE); - mGarbageMonitor.startLeakMonitor(); - mGarbageMonitor.runCallbacksOnce(); - mGarbageMonitor.runCallbacksOnce(); - mGarbageMonitor.runCallbacksOnce(); + mGarbageMonitor.reinspectGarbageAfterGc(); verify(mLeakReporter, never()).dumpLeak(anyInt()); } @Test public void testTransientGarbage_doesntDump() { - when(mTrackedGarbage.countOldGarbage()).thenReturn(100); + when(mTrackedGarbage.countOldGarbage()).thenReturn(GarbageMonitor.GARBAGE_ALLOWANCE + 1); + // Start the leak monitor. Nothing gets reported immediately. mGarbageMonitor.startLeakMonitor(); - mGarbageMonitor.runInspectCallback(); + mFakeExecutor.runAllReady(); + verify(mLeakReporter, never()).dumpLeak(anyInt()); + // Garbage gets reset to 0 before the leak reporte actually gets called. when(mTrackedGarbage.countOldGarbage()).thenReturn(0); + mFakeExecutor.advanceClockToLast(); + mFakeExecutor.runAllReady(); - mGarbageMonitor.runReinspectCallback(); - + // Therefore nothing gets dumped. verify(mLeakReporter, never()).dumpLeak(anyInt()); } @Test public void testLotsOfPersistentGarbage_dumps() { - when(mTrackedGarbage.countOldGarbage()).thenReturn(100); + when(mTrackedGarbage.countOldGarbage()).thenReturn(GarbageMonitor.GARBAGE_ALLOWANCE + 1); - mGarbageMonitor.startLeakMonitor(); - mGarbageMonitor.runCallbacksOnce(); + mGarbageMonitor.reinspectGarbageAfterGc(); - verify(mLeakReporter).dumpLeak(anyInt()); + verify(mLeakReporter).dumpLeak(GarbageMonitor.GARBAGE_ALLOWANCE + 1); } - private static class TestableGarbageMonitor extends GarbageMonitor { - public TestableGarbageMonitor( - Context context, - Looper looper, - LeakDetector leakDetector, - LeakReporter leakReporter) { - super(context, looper, leakDetector, leakReporter); - } + @Test + public void testLotsOfPersistentGarbage_dumpsAfterAtime() { + when(mTrackedGarbage.countOldGarbage()).thenReturn(GarbageMonitor.GARBAGE_ALLOWANCE + 1); - void runInspectCallback() { - startLeakMonitor(); - } + // Start the leak monitor. Nothing gets reported immediately. + mGarbageMonitor.startLeakMonitor(); + mFakeExecutor.runAllReady(); + verify(mLeakReporter, never()).dumpLeak(anyInt()); - void runReinspectCallback() { - reinspectGarbageAfterGc(); - } + mFakeExecutor.advanceClockToLast(); + mFakeExecutor.runAllReady(); - void runCallbacksOnce() { - // Note that TestableLooper doesn't currently support delayed messages so we need to run - // callbacks explicitly. - runInspectCallback(); - runReinspectCallback(); - } + verify(mLeakReporter).dumpLeak(GarbageMonitor.GARBAGE_ALLOWANCE + 1); } } \ No newline at end of file