From 2a2a8e1b2c90cc804a9afa8913536281124cf377 Mon Sep 17 00:00:00 2001 From: Xiaowen Lei Date: Tue, 21 Jun 2022 22:55:10 +0000 Subject: Remove SmartSpaceComplication when the SmartspaceTargetListener is removed. The listener removes the SmartSpaceComplication when the targets becomes empty. If the listener isn't connected, we don't know whether/when the targets becomes empty. Therefore we should just remove the complication when the listener is removed. This fixes the extra DATE card impression on the Dream surface. The CardPagerAdapter adds a default DATE card when its setTargets method is called with an empty list. This CL ensures that for Dream Smartspace, the setTargets method is never called with an empty list. Reorganized the existing testAvailability() test into multiple tests. The new logic is tested in testOverlayInActive_removesTargetListener_removesComplication(). Bug: 231251252 Test: on device via `adb logcat ...` Test: atest SmartSpaceComplicationTest Change-Id: Iac0d8a9697f42e2c70c83c5da050f17190fcc3c1 --- .../dreams/DreamOverlayStateController.java | 6 ++ .../systemui/dreams/SmartSpaceComplication.java | 1 + .../ComplicationHostViewController.java | 9 ++- .../complication/ComplicationLayoutEngine.java | 4 +- .../dreams/complication/ComplicationViewModel.java | 5 ++ .../dreams/SmartSpaceComplicationTest.java | 89 ++++++++++++++++++++-- 6 files changed, 106 insertions(+), 8 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayStateController.java b/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayStateController.java index fc71e2fb2329..69e41ba9b284 100644 --- a/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayStateController.java +++ b/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayStateController.java @@ -101,6 +101,9 @@ public class DreamOverlayStateController implements public void addComplication(Complication complication) { mExecutor.execute(() -> { if (mComplications.add(complication)) { + if (DEBUG) { + Log.d(TAG, "addComplication: added " + complication); + } mCallbacks.stream().forEach(callback -> callback.onComplicationsChanged()); } }); @@ -112,6 +115,9 @@ public class DreamOverlayStateController implements public void removeComplication(Complication complication) { mExecutor.execute(() -> { if (mComplications.remove(complication)) { + if (DEBUG) { + Log.d(TAG, "removeComplication: removed " + complication); + } mCallbacks.stream().forEach(callback -> callback.onComplicationsChanged()); } }); diff --git a/packages/SystemUI/src/com/android/systemui/dreams/SmartSpaceComplication.java b/packages/SystemUI/src/com/android/systemui/dreams/SmartSpaceComplication.java index 486fc893732f..be94e5031917 100644 --- a/packages/SystemUI/src/com/android/systemui/dreams/SmartSpaceComplication.java +++ b/packages/SystemUI/src/com/android/systemui/dreams/SmartSpaceComplication.java @@ -82,6 +82,7 @@ public class SmartSpaceComplication implements Complication { mSmartSpaceController.addListener(mSmartspaceListener); } else { mSmartSpaceController.removeListener(mSmartspaceListener); + mDreamOverlayStateController.removeComplication(mComplication); } } }); diff --git a/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationHostViewController.java b/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationHostViewController.java index c1173aea8b15..fd6cfc0700ad 100644 --- a/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationHostViewController.java +++ b/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationHostViewController.java @@ -21,6 +21,7 @@ import static com.android.systemui.dreams.complication.dagger.ComplicationModule import android.graphics.Rect; import android.graphics.Region; +import android.os.Debug; import android.util.Log; import android.view.View; @@ -44,7 +45,8 @@ import javax.inject.Named; * a {@link ComplicationLayoutEngine}. */ public class ComplicationHostViewController extends ViewController { - public static final String TAG = "ComplicationHostVwCtrl"; + private static final String TAG = "ComplicationHostVwCtrl"; + private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); private final ComplicationLayoutEngine mLayoutEngine; private final LifecycleOwner mLifecycleOwner; @@ -90,6 +92,11 @@ public class ComplicationHostViewController extends ViewController complications) { + if (DEBUG) { + Log.d(TAG, "updateComplications called. Callers = " + Debug.getCallers(25)); + Log.d(TAG, " mComplications = " + mComplications.toString()); + Log.d(TAG, " complications = " + complications.toString()); + } final Collection ids = complications.stream() .map(complicationViewModel -> complicationViewModel.getId()) .collect(Collectors.toSet()); diff --git a/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationLayoutEngine.java b/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationLayoutEngine.java index ded61a8b80d9..9cd149b9bfee 100644 --- a/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationLayoutEngine.java +++ b/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationLayoutEngine.java @@ -54,7 +54,7 @@ import javax.inject.Named; */ @DreamOverlayComponent.DreamOverlayScope public class ComplicationLayoutEngine implements Complication.VisibilityController { - public static final String TAG = "ComplicationLayoutEngine"; + public static final String TAG = "ComplicationLayoutEng"; /** * {@link ViewEntry} is an internal container, capturing information necessary for working with @@ -529,7 +529,7 @@ public class ComplicationLayoutEngine implements Complication.VisibilityControll */ public void addComplication(ComplicationId id, View view, ComplicationLayoutParams lp, @Complication.Category int category) { - Log.d(TAG, "engine: " + this + " addComplication"); + Log.d(TAG, "@" + Integer.toHexString(this.hashCode()) + " addComplication: " + id); // If the complication is present, remove. if (mEntries.containsKey(id)) { diff --git a/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationViewModel.java b/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationViewModel.java index f0239371ee63..00cf58c0c665 100644 --- a/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationViewModel.java +++ b/packages/SystemUI/src/com/android/systemui/dreams/complication/ComplicationViewModel.java @@ -64,4 +64,9 @@ public class ComplicationViewModel extends ViewModel { public void exitDream() { mHost.requestExitDream(); } + + @Override + public String toString() { + return mId + "=" + mComplication.toString(); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/dreams/SmartSpaceComplicationTest.java b/packages/SystemUI/tests/src/com/android/systemui/dreams/SmartSpaceComplicationTest.java index dc1ae0e93757..964e6d79f0bf 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/dreams/SmartSpaceComplicationTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/dreams/SmartSpaceComplicationTest.java @@ -72,19 +72,67 @@ public class SmartSpaceComplicationTest extends SysuiTestCase { } /** - * Ensures {@link SmartSpaceComplication} is only registered when it is available. + * Ensures {@link SmartSpaceComplication} isn't registered right away on start. */ @Test - public void testAvailability() { + public void testRegistrantStart_doesNotAddComplication() { + final SmartSpaceComplication.Registrant registrant = getRegistrant(); + registrant.start(); + verify(mDreamOverlayStateController, never()).addComplication(eq(mComplication)); + } - final SmartSpaceComplication.Registrant registrant = new SmartSpaceComplication.Registrant( + private SmartSpaceComplication.Registrant getRegistrant() { + return new SmartSpaceComplication.Registrant( mContext, mDreamOverlayStateController, mComplication, mSmartspaceController); + } + + @Test + public void testOverlayActive_addsTargetListener() { + final SmartSpaceComplication.Registrant registrant = getRegistrant(); registrant.start(); - verify(mDreamOverlayStateController, never()).addComplication(eq(mComplication)); + final ArgumentCaptor dreamCallbackCaptor = + ArgumentCaptor.forClass(DreamOverlayStateController.Callback.class); + verify(mDreamOverlayStateController).addCallback(dreamCallbackCaptor.capture()); + + when(mDreamOverlayStateController.isOverlayActive()).thenReturn(true); + dreamCallbackCaptor.getValue().onStateChanged(); + + // Test + final ArgumentCaptor listenerCaptor = + ArgumentCaptor.forClass(BcSmartspaceDataPlugin.SmartspaceTargetListener.class); + verify(mSmartspaceController).addListener(listenerCaptor.capture()); + } + + @Test + public void testOverlayActive_targetsNonEmpty_addsComplication() { + final SmartSpaceComplication.Registrant registrant = getRegistrant(); + registrant.start(); + + final ArgumentCaptor dreamCallbackCaptor = + ArgumentCaptor.forClass(DreamOverlayStateController.Callback.class); + verify(mDreamOverlayStateController).addCallback(dreamCallbackCaptor.capture()); + + when(mDreamOverlayStateController.isOverlayActive()).thenReturn(true); + dreamCallbackCaptor.getValue().onStateChanged(); + + final ArgumentCaptor listenerCaptor = + ArgumentCaptor.forClass(BcSmartspaceDataPlugin.SmartspaceTargetListener.class); + verify(mSmartspaceController).addListener(listenerCaptor.capture()); + + // Test + final SmartspaceTarget target = Mockito.mock(SmartspaceTarget.class); + listenerCaptor.getValue().onSmartspaceTargetsUpdated(Arrays.asList(target)); + verify(mDreamOverlayStateController).addComplication(eq(mComplication)); + } + + @Test + public void testOverlayActive_targetsEmpty_removesComplication() { + final SmartSpaceComplication.Registrant registrant = getRegistrant(); + registrant.start(); final ArgumentCaptor dreamCallbackCaptor = ArgumentCaptor.forClass(DreamOverlayStateController.Callback.class); @@ -100,10 +148,41 @@ public class SmartSpaceComplicationTest extends SysuiTestCase { final SmartspaceTarget target = Mockito.mock(SmartspaceTarget.class); listenerCaptor.getValue().onSmartspaceTargetsUpdated(Arrays.asList(target)); verify(mDreamOverlayStateController).addComplication(eq(mComplication)); + + // Test + listenerCaptor.getValue().onSmartspaceTargetsUpdated(Arrays.asList()); + verify(mDreamOverlayStateController).removeComplication(eq(mComplication)); + } + + @Test + public void testOverlayInActive_removesTargetListener_removesComplication() { + final SmartSpaceComplication.Registrant registrant = getRegistrant(); + registrant.start(); + + final ArgumentCaptor dreamCallbackCaptor = + ArgumentCaptor.forClass(DreamOverlayStateController.Callback.class); + verify(mDreamOverlayStateController).addCallback(dreamCallbackCaptor.capture()); + + when(mDreamOverlayStateController.isOverlayActive()).thenReturn(true); + dreamCallbackCaptor.getValue().onStateChanged(); + + final ArgumentCaptor listenerCaptor = + ArgumentCaptor.forClass(BcSmartspaceDataPlugin.SmartspaceTargetListener.class); + verify(mSmartspaceController).addListener(listenerCaptor.capture()); + + final SmartspaceTarget target = Mockito.mock(SmartspaceTarget.class); + listenerCaptor.getValue().onSmartspaceTargetsUpdated(Arrays.asList(target)); + verify(mDreamOverlayStateController).addComplication(eq(mComplication)); + + // Test + when(mDreamOverlayStateController.isOverlayActive()).thenReturn(false); + dreamCallbackCaptor.getValue().onStateChanged(); + verify(mSmartspaceController).removeListener(listenerCaptor.getValue()); + verify(mDreamOverlayStateController).removeComplication(eq(mComplication)); } @Test - public void testGetViewReusesSameView() { + public void testGetView_reusesSameView() { final SmartSpaceComplication complication = new SmartSpaceComplication(getContext(), mSmartspaceController); final Complication.ViewHolder viewHolder = complication.createView(mComplicationViewModel); -- cgit v1.2.3-59-g8ed1b