From bb905fd00949f6b7747b5f5261b704a6962e1862 Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Wed, 8 Dec 2021 15:35:53 -0800 Subject: [PATCH] CommunalSource.Connection Introduction This changelist introduces the Connection interface, abstracting details around the result reporting mechanism. It also sets the expectation that the client holds onto the connection, allowing connectors to build around this assumption. The Connection interface also allows connection state to be contained in a single entity, tied to the primer. As a result, the CommunalSources no longer need to track state and maintain their own callbacks. Test: atest CommunalSourcePrimerTest Bug: 209607168 Change-Id: I599d1ba421d76ec9e9c20c82718104358929c5e4 --- .../systemui/communal/CommunalSource.java | 43 +++--- .../communal/CommunalSourceMonitor.java | 80 +++++------ .../communal/CommunalSourcePrimer.java | 53 +++---- .../NotificationPanelViewController.java | 7 - .../service/ObservableServiceConnection.java | 12 +- .../communal/CommunalManagerUpdaterTest.java | 8 +- .../communal/CommunalSourceMonitorTest.java | 24 +++- .../communal/CommunalSourcePrimerTest.java | 131 +++++++++--------- .../systemui/util/ref/GcWeakReference.java | 36 +++++ .../ObservableServiceConnectionTest.java | 4 +- 10 files changed, 217 insertions(+), 181 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/util/ref/GcWeakReference.java diff --git a/packages/SystemUI/src/com/android/systemui/communal/CommunalSource.java b/packages/SystemUI/src/com/android/systemui/communal/CommunalSource.java index 53586f58c5d2c..42ecd5c835bbd 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/CommunalSource.java +++ b/packages/SystemUI/src/com/android/systemui/communal/CommunalSource.java @@ -35,7 +35,23 @@ public interface CommunalSource { * {@link Connector} defines an interface for {@link CommunalSource} instances to be generated. */ interface Connector { - ListenableFuture> connect(); + Connection connect(Connection.Callback callback); + } + + /** + * {@link Connection} defines an interface for an entity which holds the necessary components + * for establishing and maintaining a connection to the communal source. + */ + interface Connection { + /** + * {@link Callback} defines an interface for clients to be notified when a source is ready + */ + interface Callback { + void onSourceEstablished(Optional source); + void onDisconnected(); + } + + void disconnect(); } /** @@ -86,29 +102,4 @@ public interface CommunalSource { * value will be {@code null} in case of a failure. */ ListenableFuture requestCommunalView(Context context); - - /** - * Adds a {@link Callback} to receive future status updates regarding this - * {@link CommunalSource}. - * - * @param callback The {@link Callback} to be added. - */ - void addCallback(Callback callback); - - /** - * Removes a {@link Callback} from receiving future updates. - * - * @param callback The {@link Callback} to be removed. - */ - void removeCallback(Callback callback); - - /** - * An interface for receiving updates on the state of the {@link CommunalSource}. - */ - interface Callback { - /** - * Invoked when the {@link CommunalSource} is no longer available for use. - */ - void onDisconnected(); - } } diff --git a/packages/SystemUI/src/com/android/systemui/communal/CommunalSourceMonitor.java b/packages/SystemUI/src/com/android/systemui/communal/CommunalSourceMonitor.java index d3018e310c8fe..4aa46f1813dca 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/CommunalSourceMonitor.java +++ b/packages/SystemUI/src/com/android/systemui/communal/CommunalSourceMonitor.java @@ -21,12 +21,14 @@ import android.util.Log; import com.android.internal.annotations.VisibleForTesting; import com.android.systemui.communal.conditions.CommunalConditionsMonitor; import com.android.systemui.dagger.SysUISingleton; +import com.android.systemui.dagger.qualifiers.Main; import com.google.android.collect.Lists; import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Iterator; +import java.util.concurrent.Executor; import javax.inject.Inject; @@ -41,6 +43,7 @@ public class CommunalSourceMonitor { // A list of {@link Callback} that have registered to receive updates. private final ArrayList> mCallbacks = Lists.newArrayList(); private final CommunalConditionsMonitor mConditionsMonitor; + private final Executor mExecutor; private CommunalSource mCurrentSource; @@ -50,14 +53,6 @@ public class CommunalSourceMonitor { // Whether the class is currently listening for condition changes. private boolean mListeningForConditions = false; - private CommunalSource.Callback mSourceCallback = new CommunalSource.Callback() { - @Override - public void onDisconnected() { - // Clear source reference. - setSource(null /* source */); - } - }; - private final CommunalConditionsMonitor.Callback mConditionsCallback = allConditionsMet -> { if (mAllCommunalConditionsMet != allConditionsMet) { @@ -70,7 +65,9 @@ public class CommunalSourceMonitor { @VisibleForTesting @Inject - public CommunalSourceMonitor(CommunalConditionsMonitor communalConditionsMonitor) { + public CommunalSourceMonitor(@Main Executor executor, + CommunalConditionsMonitor communalConditionsMonitor) { + mExecutor = executor; mConditionsMonitor = communalConditionsMonitor; } @@ -81,35 +78,28 @@ public class CommunalSourceMonitor { * @param source The new {@link CommunalSource}. */ public void setSource(CommunalSource source) { - if (mCurrentSource != null) { - mCurrentSource.removeCallback(mSourceCallback); - } - mCurrentSource = source; if (mAllCommunalConditionsMet) { executeOnSourceAvailableCallbacks(); } - - // Add callback to be informed when the source disconnects. - if (mCurrentSource != null) { - mCurrentSource.addCallback(mSourceCallback); - } } private void executeOnSourceAvailableCallbacks() { - // If the new source is valid, inform registered Callbacks of its presence. - Iterator> itr = mCallbacks.iterator(); - while (itr.hasNext()) { - Callback cb = itr.next().get(); - if (cb == null) { - itr.remove(); - } else { - cb.onSourceAvailable( - (mAllCommunalConditionsMet && mCurrentSource != null) ? new WeakReference<>( - mCurrentSource) : null); + mExecutor.execute(() -> { + // If the new source is valid, inform registered Callbacks of its presence. + Iterator> itr = mCallbacks.iterator(); + while (itr.hasNext()) { + Callback cb = itr.next().get(); + if (cb == null) { + itr.remove(); + } else { + cb.onSourceAvailable( + (mAllCommunalConditionsMet && mCurrentSource != null) + ? new WeakReference<>(mCurrentSource) : null); + } } - } + }); } /** @@ -118,17 +108,19 @@ public class CommunalSourceMonitor { * @param callback The {@link Callback} to add. */ public void addCallback(Callback callback) { - mCallbacks.add(new WeakReference<>(callback)); + mExecutor.execute(() -> { + mCallbacks.add(new WeakReference<>(callback)); - // Inform the callback of any already present CommunalSource. - if (mAllCommunalConditionsMet && mCurrentSource != null) { - callback.onSourceAvailable(new WeakReference<>(mCurrentSource)); - } + // Inform the callback of any already present CommunalSource. + if (mAllCommunalConditionsMet && mCurrentSource != null) { + callback.onSourceAvailable(new WeakReference<>(mCurrentSource)); + } - if (!mListeningForConditions) { - mConditionsMonitor.addCallback(mConditionsCallback); - mListeningForConditions = true; - } + if (!mListeningForConditions) { + mConditionsMonitor.addCallback(mConditionsCallback); + mListeningForConditions = true; + } + }); } /** @@ -137,12 +129,14 @@ public class CommunalSourceMonitor { * @param callback The {@link Callback} to add. */ public void removeCallback(Callback callback) { - mCallbacks.removeIf(el -> el.get() == callback); + mExecutor.execute(() -> { + mCallbacks.removeIf(el -> el.get() == callback); - if (mCallbacks.isEmpty() && mListeningForConditions) { - mConditionsMonitor.removeCallback(mConditionsCallback); - mListeningForConditions = false; - } + if (mCallbacks.isEmpty() && mListeningForConditions) { + mConditionsMonitor.removeCallback(mConditionsCallback); + mListeningForConditions = false; + } + }); } /** diff --git a/packages/SystemUI/src/com/android/systemui/communal/CommunalSourcePrimer.java b/packages/SystemUI/src/com/android/systemui/communal/CommunalSourcePrimer.java index 10442396c835d..13b1dc034a197 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/CommunalSourcePrimer.java +++ b/packages/SystemUI/src/com/android/systemui/communal/CommunalSourcePrimer.java @@ -27,8 +27,6 @@ import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.util.concurrency.DelayableExecutor; import com.android.systemui.util.time.SystemClock; -import com.google.common.util.concurrent.ListenableFuture; - import java.util.Optional; import javax.inject.Inject; @@ -53,10 +51,11 @@ public class CommunalSourcePrimer extends CoreStartable { private int mReconnectAttempts = 0; private Runnable mCurrentReconnectCancelable; - private ListenableFuture> mGetSourceFuture; - private final Optional mConnector; private final Optional mObserver; + private final Optional mConnector; + + private CommunalSource.Connection mCurrentConnection; private final Runnable mConnectRunnable = new Runnable() { @Override @@ -66,6 +65,10 @@ public class CommunalSourcePrimer extends CoreStartable { } }; + private final CommunalSource.Observer.Callback mObserverCallback = () -> { + initiateConnectionAttempt(); + }; + @Inject public CommunalSourcePrimer(Context context, @Main Resources resources, SystemClock clock, @@ -132,7 +135,7 @@ public class CommunalSourcePrimer extends CoreStartable { @Override protected void onBootCompleted() { if (mObserver.isPresent()) { - mObserver.get().addCallback(() -> initiateConnectionAttempt()); + mObserver.get().addCallback(mObserverCallback); } initiateConnectionAttempt(); } @@ -142,34 +145,36 @@ public class CommunalSourcePrimer extends CoreStartable { Log.d(TAG, "attempting to communal to communal source"); } - if (mGetSourceFuture != null) { + if (mCurrentConnection != null) { if (DEBUG) { Log.d(TAG, "canceling in-flight connection"); } - mGetSourceFuture.cancel(true); + mCurrentConnection.disconnect(); } - mGetSourceFuture = mConnector.get().connect(); - mGetSourceFuture.addListener(() -> { - try { - final long startTime = mSystemClock.currentTimeMillis(); - Optional result = mGetSourceFuture.get(); - if (result.isPresent()) { - final CommunalSource source = result.get(); - source.addCallback(() -> { - if (mSystemClock.currentTimeMillis() - startTime > mMinConnectionDuration) { - initiateConnectionAttempt(); - } else { - scheduleConnectionAttempt(); - } - }); + mCurrentConnection = mConnector.get().connect(new CommunalSource.Connection.Callback() { + private long mStartTime; + + @Override + public void onSourceEstablished(Optional optionalSource) { + mStartTime = mSystemClock.currentTimeMillis(); + + if (optionalSource.isPresent()) { + final CommunalSource source = optionalSource.get(); mMonitor.setSource(source); } else { scheduleConnectionAttempt(); } - } catch (Exception e) { - e.printStackTrace(); } - }, mMainExecutor); + + @Override + public void onDisconnected() { + if (mSystemClock.currentTimeMillis() - mStartTime > mMinConnectionDuration) { + initiateConnectionAttempt(); + } else { + scheduleConnectionAttempt(); + } + } + }); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java index a64e579ce72e8..d0cd8cc6054f6 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java @@ -519,8 +519,6 @@ public class NotificationPanelViewController extends PanelViewController { private WeakReference mCommunalSource; - private final CommunalSource.Callback mCommunalSourceCallback; - private final CommandQueue mCommandQueue; private final NotificationLockscreenUserManager mLockscreenUserManager; private final UserManager mUserManager; @@ -906,9 +904,6 @@ public class NotificationPanelViewController extends PanelViewController { mMaxKeyguardNotifications = resources.getInteger(R.integer.keyguard_max_notification_count); mKeyguardUnfoldTransition = unfoldComponent.map(c -> c.getKeyguardUnfoldTransition()); - mCommunalSourceCallback = () -> { - mUiExecutor.execute(() -> setCommunalSource(null /*source*/)); - }; mCommunalSourceMonitorCallback = (source) -> { mUiExecutor.execute(() -> setCommunalSource(source)); @@ -4719,7 +4714,6 @@ public class NotificationPanelViewController extends PanelViewController { CommunalSource existingSource = mCommunalSource != null ? mCommunalSource.get() : null; if (existingSource != null) { - existingSource.removeCallback(mCommunalSourceCallback); mCommunalViewController.show(null /*source*/); } @@ -4728,7 +4722,6 @@ public class NotificationPanelViewController extends PanelViewController { CommunalSource currentSource = mCommunalSource != null ? mCommunalSource.get() : null; // Set source and register callback if (currentSource != null && mCommunalViewController != null) { - currentSource.addCallback(mCommunalSourceCallback); mCommunalViewController.show(source); } diff --git a/packages/SystemUI/src/com/android/systemui/util/service/ObservableServiceConnection.java b/packages/SystemUI/src/com/android/systemui/util/service/ObservableServiceConnection.java index bad0c37b81648..c57dbe37ca5f3 100644 --- a/packages/SystemUI/src/com/android/systemui/util/service/ObservableServiceConnection.java +++ b/packages/SystemUI/src/com/android/systemui/util/service/ObservableServiceConnection.java @@ -16,9 +16,7 @@ package com.android.systemui.util.service; -import android.annotation.CallbackExecutor; import android.annotation.IntDef; -import android.annotation.NonNull; import android.content.ComponentName; import android.content.Context; import android.content.Intent; @@ -26,6 +24,8 @@ import android.content.ServiceConnection; import android.os.IBinder; import android.util.Log; +import com.android.systemui.dagger.qualifiers.Main; + import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.ref.WeakReference; @@ -35,6 +35,8 @@ import java.util.Optional; import java.util.concurrent.Executor; import java.util.function.Consumer; +import javax.inject.Inject; + /** * {@link ObservableServiceConnection} is a concrete implementation of {@link ServiceConnection} * that enables monitoring the status of a binder connection. It also aides in automatically @@ -119,17 +121,17 @@ public class ObservableServiceConnection implements ServiceConnection { * Default constructor for {@link ObservableServiceConnection}. * @param context The context from which the service will be bound with. * @param serviceIntent The intent to bind service with. - * @param flags The flags to use during the binding * @param executor The executor for connection callbacks to be delivered on * @param transformer A {@link ServiceTransformer} for transforming the resulting service * into a desired type. */ + @Inject public ObservableServiceConnection(Context context, Intent serviceIntent, - @Context.BindServiceFlags int flags, @NonNull @CallbackExecutor Executor executor, + @Main Executor executor, ServiceTransformer transformer) { mContext = context; mServiceIntent = serviceIntent; - mFlags = flags; + mFlags = Context.BIND_AUTO_CREATE; mExecutor = executor; mTransformer = transformer; mCallbacks = new ArrayList<>(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalManagerUpdaterTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalManagerUpdaterTest.java index abc20994749ac..9a9b7c4f62eb7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalManagerUpdaterTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalManagerUpdaterTest.java @@ -29,6 +29,8 @@ import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; import com.android.systemui.communal.conditions.CommunalConditionsMonitor; +import com.android.systemui.util.concurrency.FakeExecutor; +import com.android.systemui.util.time.FakeSystemClock; import org.junit.Before; import org.junit.Test; @@ -45,6 +47,8 @@ public class CommunalManagerUpdaterTest extends SysuiTestCase { @Mock private CommunalConditionsMonitor mCommunalConditionsMonitor; + private FakeExecutor mExecutor = new FakeExecutor(new FakeSystemClock()); + @Before public void setup() { MockitoAnnotations.initMocks(this); @@ -56,7 +60,7 @@ public class CommunalManagerUpdaterTest extends SysuiTestCase { return null; }).when(mCommunalConditionsMonitor).addCallback(any()); - mMonitor = new CommunalSourceMonitor(mCommunalConditionsMonitor); + mMonitor = new CommunalSourceMonitor(mExecutor, mCommunalConditionsMonitor); final CommunalManagerUpdater updater = new CommunalManagerUpdater(mContext, mMonitor); updater.start(); clearInvocations(mCommunalManager); @@ -65,6 +69,7 @@ public class CommunalManagerUpdaterTest extends SysuiTestCase { @Test public void testUpdateSystemService_false() { mMonitor.setSource(null); + mExecutor.runAllReady(); verify(mCommunalManager).setCommunalViewShowing(false); } @@ -72,6 +77,7 @@ public class CommunalManagerUpdaterTest extends SysuiTestCase { public void testUpdateSystemService_true() { final CommunalSource source = mock(CommunalSource.class); mMonitor.setSource(source); + mExecutor.runAllReady(); verify(mCommunalManager).setCommunalViewShowing(true); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSourceMonitorTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSourceMonitorTest.java index df9a2cb1d268a..409dd940ceb6d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSourceMonitorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSourceMonitorTest.java @@ -32,6 +32,8 @@ import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; import com.android.systemui.communal.conditions.CommunalConditionsMonitor; +import com.android.systemui.util.concurrency.FakeExecutor; +import com.android.systemui.util.time.FakeSystemClock; import org.junit.Before; import org.junit.Test; @@ -52,11 +54,12 @@ public class CommunalSourceMonitorTest extends SysuiTestCase { @Captor private ArgumentCaptor mConditionsCallbackCaptor; private CommunalSourceMonitor mCommunalSourceMonitor; + private FakeExecutor mExecutor = new FakeExecutor(new FakeSystemClock()); @Before public void setup() { MockitoAnnotations.initMocks(this); - mCommunalSourceMonitor = new CommunalSourceMonitor(mCommunalConditionsMonitor); + mCommunalSourceMonitor = new CommunalSourceMonitor(mExecutor, mCommunalConditionsMonitor); } @Test @@ -64,7 +67,7 @@ public class CommunalSourceMonitorTest extends SysuiTestCase { final CommunalSourceMonitor.Callback callback = mock(CommunalSourceMonitor.Callback.class); final CommunalSource source = mock(CommunalSource.class); - mCommunalSourceMonitor.setSource(source); + setSource(source); mCommunalSourceMonitor.addCallback(callback); setConditionsMet(true); @@ -78,7 +81,7 @@ public class CommunalSourceMonitorTest extends SysuiTestCase { mCommunalSourceMonitor.addCallback(callback); mCommunalSourceMonitor.removeCallback(callback); - mCommunalSourceMonitor.setSource(source); + setSource(source); verify(callback, never()).onSourceAvailable(any()); } @@ -91,7 +94,7 @@ public class CommunalSourceMonitorTest extends SysuiTestCase { mCommunalSourceMonitor.addCallback(callback); setConditionsMet(true); clearInvocations(callback); - mCommunalSourceMonitor.setSource(source); + setSource(source); verifyOnSourceAvailableCalledWith(callback, source); } @@ -103,7 +106,7 @@ public class CommunalSourceMonitorTest extends SysuiTestCase { mCommunalSourceMonitor.addCallback(callback); setConditionsMet(false); - mCommunalSourceMonitor.setSource(source); + setSource(source); verify(callback, never()).onSourceAvailable(any()); } @@ -114,7 +117,7 @@ public class CommunalSourceMonitorTest extends SysuiTestCase { final CommunalSource source = mock(CommunalSource.class); mCommunalSourceMonitor.addCallback(callback); - mCommunalSourceMonitor.setSource(source); + setSource(source); // The callback should not have executed since communal is disabled. verify(callback, never()).onSourceAvailable(any()); @@ -130,7 +133,7 @@ public class CommunalSourceMonitorTest extends SysuiTestCase { final CommunalSource source = mock(CommunalSource.class); mCommunalSourceMonitor.addCallback(callback); - mCommunalSourceMonitor.setSource(source); + setSource(source); setConditionsMet(true); verifyOnSourceAvailableCalledWith(callback, source); @@ -151,9 +154,16 @@ public class CommunalSourceMonitorTest extends SysuiTestCase { // Pushes an update on whether the communal conditions are met, assuming that a callback has // been registered with the communal conditions monitor. private void setConditionsMet(boolean value) { + mExecutor.runAllReady(); verify(mCommunalConditionsMonitor).addCallback(mConditionsCallbackCaptor.capture()); final CommunalConditionsMonitor.Callback conditionsCallback = mConditionsCallbackCaptor.getValue(); conditionsCallback.onConditionsChanged(value); + mExecutor.runAllReady(); + } + + private void setSource(CommunalSource source) { + mCommunalSourceMonitor.setSource(source); + mExecutor.runAllReady(); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSourcePrimerTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSourcePrimerTest.java index 2ed38a4f36024..1d9a0594d7e21 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSourcePrimerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalSourcePrimerTest.java @@ -16,6 +16,7 @@ package com.android.systemui.communal; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -26,14 +27,16 @@ import android.content.Context; import android.content.res.Resources; import android.testing.AndroidTestingRunner; -import androidx.concurrent.futures.CallbackToFutureAdapter; import androidx.test.filters.SmallTest; import com.android.systemui.R; import com.android.systemui.SysuiTestCase; -import com.android.systemui.util.concurrency.FakeExecutor; +import com.android.systemui.util.concurrency.FakeExecutor;; +import com.android.systemui.util.ref.GcWeakReference; import com.android.systemui.util.time.FakeSystemClock; +import com.google.common.util.concurrent.ListenableFuture; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -52,6 +55,35 @@ public class CommunalSourcePrimerTest extends SysuiTestCase { private static final int RETRY_DELAY_MS = 1000; private static final int CONNECTION_MIN_DURATION_MS = 5000; + // A simple implementation of {@link CommunalSource.Observer} to capture a callback value. + // Used to ensure the references to a {@link CommunalSource.Observer.Callback} can be fully + // removed. + private static class FakeObserver implements CommunalSource.Observer { + public GcWeakReference mLastCallback; + + @Override + public void addCallback(Callback callback) { + mLastCallback = new GcWeakReference<>(callback); + } + + @Override + public void removeCallback(Callback callback) { + if (mLastCallback.get() == callback) { + mLastCallback = null; + } + } + } + + // A simple implementation of {@link CommunalSource} to capture callback values. This + // implementation better emulates the {@link WeakReference} wrapping behavior of + // {@link CommunalSource} implementations than a mock. + private static class FakeSource implements CommunalSource { + @Override + public ListenableFuture requestCommunalView(Context context) { + return null; + } + } + @Mock private Context mContext; @@ -61,8 +93,7 @@ public class CommunalSourcePrimerTest extends SysuiTestCase { private FakeSystemClock mFakeClock = new FakeSystemClock(); private FakeExecutor mFakeExecutor = new FakeExecutor(mFakeClock); - @Mock - private CommunalSource mSource; + private FakeSource mSource = new FakeSource(); @Mock private CommunalSourceMonitor mCommunalSourceMonitor; @@ -71,7 +102,9 @@ public class CommunalSourcePrimerTest extends SysuiTestCase { private CommunalSource.Connector mConnector; @Mock - private CommunalSource.Observer mObserver; + private CommunalSource.Connection mConnection; + + private FakeObserver mObserver = new FakeObserver(); private CommunalSourcePrimer mPrimer; @@ -93,35 +126,35 @@ public class CommunalSourcePrimerTest extends SysuiTestCase { mCommunalSourceMonitor, Optional.of(mConnector), Optional.of(mObserver)); } + private CommunalSource.Connection.Callback captureCallbackAndSend( + CommunalSource.Connector connector, Optional source) { + ArgumentCaptor connectionCallback = + ArgumentCaptor.forClass(CommunalSource.Connection.Callback.class); + + verify(connector).connect(connectionCallback.capture()); + Mockito.clearInvocations(connector); + + final CommunalSource.Connection.Callback callback = connectionCallback.getValue(); + callback.onSourceEstablished(source); + + return callback; + } + @Test public void testConnect() { - when(mConnector.connect()).thenReturn( - CallbackToFutureAdapter.getFuture(completer -> { - completer.set(Optional.of(mSource)); - return "test"; - })); - mPrimer.onBootCompleted(); - mFakeExecutor.runAllReady(); + captureCallbackAndSend(mConnector, Optional.of(mSource)); verify(mCommunalSourceMonitor).setSource(mSource); } @Test public void testRetryOnBindFailure() throws Exception { - when(mConnector.connect()).thenReturn( - CallbackToFutureAdapter.getFuture(completer -> { - completer.set(Optional.empty()); - return "test"; - })); - mPrimer.onBootCompleted(); - mFakeExecutor.runAllReady(); // Verify attempts happen. Note that we account for the retries plus initial attempt, which // is not scheduled. for (int attemptCount = 0; attemptCount < MAX_RETRIES + 1; attemptCount++) { - verify(mConnector, times(1)).connect(); - clearInvocations(mConnector); + captureCallbackAndSend(mConnector, Optional.empty()); mFakeExecutor.advanceClockToNext(); mFakeExecutor.runAllReady(); } @@ -131,76 +164,42 @@ public class CommunalSourcePrimerTest extends SysuiTestCase { @Test public void testRetryOnDisconnectFailure() throws Exception { - when(mConnector.connect()).thenReturn( - CallbackToFutureAdapter.getFuture(completer -> { - completer.set(Optional.of(mSource)); - return "test"; - })); - mPrimer.onBootCompleted(); - mFakeExecutor.runAllReady(); - // Verify attempts happen. Note that we account for the retries plus initial attempt, which // is not scheduled. for (int attemptCount = 0; attemptCount < MAX_RETRIES + 1; attemptCount++) { - verify(mConnector, times(1)).connect(); - clearInvocations(mConnector); - ArgumentCaptor callbackCaptor = - ArgumentCaptor.forClass(CommunalSource.Callback.class); - verify(mSource).addCallback(callbackCaptor.capture()); - clearInvocations(mSource); + final CommunalSource.Connection.Callback callback = + captureCallbackAndSend(mConnector, Optional.of(mSource)); verify(mCommunalSourceMonitor).setSource(Mockito.notNull()); clearInvocations(mCommunalSourceMonitor); - callbackCaptor.getValue().onDisconnected(); + callback.onDisconnected(); mFakeExecutor.advanceClockToNext(); mFakeExecutor.runAllReady(); } - verify(mConnector, never()).connect(); + verify(mConnector, never()).connect(any()); } @Test public void testAttemptOnPackageChange() { - when(mConnector.connect()).thenReturn( - CallbackToFutureAdapter.getFuture(completer -> { - completer.set(Optional.empty()); - return "test"; - })); - mPrimer.onBootCompleted(); - mFakeExecutor.runAllReady(); + captureCallbackAndSend(mConnector, Optional.empty()); - final ArgumentCaptor callbackCaptor = - ArgumentCaptor.forClass(CommunalSource.Observer.Callback.class); - verify(mObserver).addCallback(callbackCaptor.capture()); + mObserver.mLastCallback.get().onSourceChanged(); - clearInvocations(mConnector); - callbackCaptor.getValue().onSourceChanged(); - - verify(mConnector, times(1)).connect(); + verify(mConnector, times(1)).connect(any()); } @Test public void testDisconnect() { - final ArgumentCaptor callbackCaptor = - ArgumentCaptor.forClass(CommunalSource.Callback.class); - - when(mConnector.connect()).thenReturn( - CallbackToFutureAdapter.getFuture(completer -> { - completer.set(Optional.of(mSource)); - return "test"; - })); - mPrimer.onBootCompleted(); - mFakeExecutor.runAllReady(); + final CommunalSource.Connection.Callback callback = + captureCallbackAndSend(mConnector, Optional.of(mSource)); verify(mCommunalSourceMonitor).setSource(mSource); - verify(mSource).addCallback(callbackCaptor.capture()); - clearInvocations(mConnector); mFakeClock.advanceTime(CONNECTION_MIN_DURATION_MS + 1); - callbackCaptor.getValue().onDisconnected(); - mFakeExecutor.runAllReady(); + callback.onDisconnected(); - verify(mConnector).connect(); + verify(mConnector).connect(any()); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/ref/GcWeakReference.java b/packages/SystemUI/tests/src/com/android/systemui/util/ref/GcWeakReference.java new file mode 100644 index 0000000000000..ff6fd6f7194cc --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/util/ref/GcWeakReference.java @@ -0,0 +1,36 @@ +/* + * 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.ref; + +import com.android.internal.util.GcUtils; + +import java.lang.ref.WeakReference; + +/** + * A WeakReference subclass that forces gc/finalizing on access. + */ +public class GcWeakReference extends WeakReference { + public GcWeakReference(T referent) { + super(referent); + } + + @Override + public T get() throws RuntimeException { + GcUtils.runGcAndFinalizersSync(); + return super.get(); + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/service/ObservableServiceConnectionTest.java b/packages/SystemUI/tests/src/com/android/systemui/util/service/ObservableServiceConnectionTest.java index 7c6d80b8fde34..22d7273dcd20b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/util/service/ObservableServiceConnectionTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/util/service/ObservableServiceConnectionTest.java @@ -99,7 +99,7 @@ public class ObservableServiceConnectionTest extends SysuiTestCase { @Test public void testConnect() { ObservableServiceConnection connection = new ObservableServiceConnection<>(mContext, - mIntent, 0, mExecutor, mTransformer); + mIntent, mExecutor, mTransformer); // Register twice to ensure only one callback occurs. connection.addCallback(mCallback); connection.addCallback(mCallback); @@ -119,7 +119,7 @@ public class ObservableServiceConnectionTest extends SysuiTestCase { @Test public void testDisconnect() { ObservableServiceConnection connection = new ObservableServiceConnection<>(mContext, - mIntent, 0, mExecutor, mTransformer); + mIntent, mExecutor, mTransformer); connection.addCallback(mCallback); connection.onServiceDisconnected(mComponentName);