From a5170d5dd84004398869a34c6a565fa8ae00d2ba Mon Sep 17 00:00:00 2001 From: Xiaowen Lei Date: Fri, 20 Jan 2023 21:05:16 -0800 Subject: [PATCH] Add standalone weather view to lockscreen if decoupling isenabled. At the same time, filters the WEATHER target for the existing BcSmartspaceView. The standalone weather view is also controlled via LockscreenSmartspaceController, alongside the existing BcSmartspaceView. Together they replace the keyguard_slice_view when the SMARTSPACE flag is set. Note: keyguard_slice_view is the "legacy" smart space prior to the introduction of BcSmartspace. Reusing LockscreenSmartspaceController to control the new view has other advantages as well. 1. We can keep just one SmartspaceSession and one session listener. 2. We can reuse all the existing logic. For example, the logic to update the text color when the theme changes, or via region sampling. Also: cleaned up the unused @Named(LOCKSCREEN_SMARTSPACE_TARGET_FILTER) from SmartspaceModule.kt. Bug: 261752855 Bug: 265840309 Test: manual Test: atest LockscreenSmartspaceControllerTest Change-Id: Ib19b3178f3f7208693de973d0c7ff20ae4310e0a --- .../KeyguardClockSwitchController.java | 27 ++- .../systemui/dagger/SystemUIModule.java | 6 + .../smartspace/dagger/SmartspaceModule.kt | 17 +- .../LockscreenSmartspaceController.kt | 75 ++++++-- .../LockscreenSmartspaceControllerTest.kt | 173 ++++++++++++++++-- 5 files changed, 250 insertions(+), 48 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java index 71cd18dd97033..7f1f941ebadd5 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java @@ -88,7 +88,9 @@ public class KeyguardClockSwitchController extends ViewController, optionalPlugin: Optional, optionalConfigPlugin: Optional, ) { @@ -90,6 +94,7 @@ constructor( } private var session: SmartspaceSession? = null + private val weatherPlugin: BcSmartspaceDataPlugin? = optionalWeatherPlugin.orElse(null) private val plugin: BcSmartspaceDataPlugin? = optionalPlugin.orElse(null) private val configPlugin: BcSmartspaceConfigPlugin? = optionalConfigPlugin.orElse(null) @@ -131,6 +136,10 @@ constructor( private val sessionListener = SmartspaceSession.OnTargetsAvailableListener { targets -> execution.assertIsMainThread() + + // The weather data plugin takes unfiltered targets and performs the filtering internally. + weatherPlugin?.onTargetsAvailable(targets) + val filteredTargets = targets.filter(::filterSmartspaceTarget) plugin?.onTargetsAvailable(filteredTargets) if (!isContentUpdatedOnce) { @@ -209,11 +218,37 @@ constructor( return plugin != null } + fun isDateWeatherDecoupled(): Boolean { + execution.assertIsMainThread() + + return featureFlags.isEnabled(Flags.SMARTSPACE_DATE_WEATHER_DECOUPLED) && + weatherPlugin != null + } + private fun updateBypassEnabled() { val bypassEnabled = bypassController.bypassEnabled smartspaceViews.forEach { it.setKeyguardBypassEnabled(bypassEnabled) } } + /** + * Constructs the weather view and connects it to the smartspace service. + */ + fun buildAndConnectWeatherView(parent: ViewGroup): View? { + execution.assertIsMainThread() + + if (!isEnabled()) { + throw RuntimeException("Cannot build view when not enabled") + } + if (!isDateWeatherDecoupled()) { + throw RuntimeException("Cannot build weather view when not decoupled") + } + + val view = buildView(parent, weatherPlugin) + connectSession() + + return view + } + /** * Constructs the smartspace view and connects it to the smartspace service. */ @@ -224,17 +259,17 @@ constructor( throw RuntimeException("Cannot build view when not enabled") } - val view = buildView(parent) + val view = buildView(parent, plugin, configPlugin) connectSession() return view } - fun requestSmartspaceUpdate() { - session?.requestSmartspaceUpdate() - } - - private fun buildView(parent: ViewGroup): View? { + private fun buildView( + parent: ViewGroup, + plugin: BcSmartspaceDataPlugin?, + configPlugin: BcSmartspaceConfigPlugin? = null + ): View? { if (plugin == null) { return null } @@ -242,7 +277,7 @@ constructor( val ssView = plugin.getView(parent) ssView.setUiSurface(BcSmartspaceDataPlugin.UI_SURFACE_LOCK_SCREEN_AOD) ssView.registerDataProvider(plugin) - ssView.registerConfigProvider(configPlugin) + configPlugin?.let { ssView.registerConfigProvider(it) } ssView.setIntentStarter(object : BcSmartspaceDataPlugin.IntentStarter { override fun startIntent(view: View, intent: Intent, showOnLockscreen: Boolean) { @@ -273,7 +308,8 @@ constructor( } private fun connectSession() { - if (plugin == null || session != null || smartspaceViews.isEmpty()) { + if (weatherPlugin == null && plugin == null) return + if (session != null || smartspaceViews.isEmpty()) { return } @@ -310,14 +346,20 @@ constructor( statusBarStateController.addCallback(statusBarStateListener) bypassController.registerOnBypassStateChangedListener(bypassStateChangedListener) - plugin.registerSmartspaceEventNotifier { - e -> session?.notifySmartspaceEvent(e) - } + weatherPlugin?.registerSmartspaceEventNotifier { e -> session?.notifySmartspaceEvent(e) } + plugin?.registerSmartspaceEventNotifier { e -> session?.notifySmartspaceEvent(e) } updateBypassEnabled() reloadSmartspace() } + /** + * Requests the smartspace session for an update. + */ + fun requestSmartspaceUpdate() { + session?.requestSmartspaceUpdate() + } + /** * Disconnects the smartspace view from the smartspace service and cleans up any resources. */ @@ -341,9 +383,13 @@ constructor( bypassController.unregisterOnBypassStateChangedListener(bypassStateChangedListener) session = null + weatherPlugin?.registerSmartspaceEventNotifier(null) + weatherPlugin?.onTargetsAvailable(emptyList()) + plugin?.registerSmartspaceEventNotifier(null) plugin?.onTargetsAvailable(emptyList()) - Log.d(TAG, "Ending smartspace session for lockscreen") + + Log.d(TAG, "Ended smartspace session for lockscreen") } fun addListener(listener: SmartspaceTargetListener) { @@ -357,8 +403,11 @@ constructor( } private fun filterSmartspaceTarget(t: SmartspaceTarget): Boolean { + if (isDateWeatherDecoupled()) { + return t.featureType != SmartspaceTarget.FEATURE_WEATHER + } if (!showNotifications) { - return t.getFeatureType() == SmartspaceTarget.FEATURE_WEATHER + return t.featureType == SmartspaceTarget.FEATURE_WEATHER } return when (t.userHandle) { userTracker.userHandle -> { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/lockscreen/LockscreenSmartspaceControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/lockscreen/LockscreenSmartspaceControllerTest.kt index 43b6e4144a2e2..2423f13e6494a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/lockscreen/LockscreenSmartspaceControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/lockscreen/LockscreenSmartspaceControllerTest.kt @@ -34,6 +34,7 @@ import android.widget.FrameLayout import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.flags.FeatureFlags +import com.android.systemui.flags.Flags import com.android.systemui.plugins.ActivityStarter import com.android.systemui.plugins.BcSmartspaceConfigPlugin import com.android.systemui.plugins.BcSmartspaceDataPlugin @@ -111,6 +112,9 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { @Mock private lateinit var handler: Handler + @Mock + private lateinit var weatherPlugin: BcSmartspaceDataPlugin + @Mock private lateinit var plugin: BcSmartspaceDataPlugin @@ -151,6 +155,7 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { KeyguardBypassController.OnBypassStateChangedListener private lateinit var deviceProvisionedListener: DeviceProvisionedListener + private lateinit var weatherSmartspaceView: SmartspaceView private lateinit var smartspaceView: SmartspaceView private val clock = FakeSystemClock() @@ -176,16 +181,22 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { fun setUp() { MockitoAnnotations.initMocks(this) + // Todo(b/261760571): flip the flag value here when feature is launched, and update relevant + // tests. + `when`(featureFlags.isEnabled(Flags.SMARTSPACE_DATE_WEATHER_DECOUPLED)).thenReturn(false) + `when`(secureSettings.getUriFor(PRIVATE_LOCKSCREEN_SETTING)) .thenReturn(fakePrivateLockscreenSettingUri) `when`(secureSettings.getUriFor(NOTIF_ON_LOCKSCREEN_SETTING)) .thenReturn(fakeNotifOnLockscreenSettingUri) `when`(smartspaceManager.createSmartspaceSession(any())).thenReturn(smartspaceSession) + `when`(weatherPlugin.getView(any())).thenReturn( + createWeatherSmartspaceView(), createWeatherSmartspaceView()) `when`(plugin.getView(any())).thenReturn(createSmartspaceView(), createSmartspaceView()) `when`(userTracker.userProfiles).thenReturn(userList) `when`(statusBarStateController.dozeAmount).thenReturn(0.5f) - `when`(deviceProvisionedController.isDeviceProvisioned()).thenReturn(true) - `when`(deviceProvisionedController.isCurrentUserSetup()).thenReturn(true) + `when`(deviceProvisionedController.isDeviceProvisioned).thenReturn(true) + `when`(deviceProvisionedController.isCurrentUserSetup).thenReturn(true) setActiveUser(userHandlePrimary) setAllowPrivateNotifications(userHandlePrimary, true) @@ -210,6 +221,7 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { executor, bgExecutor, handler, + Optional.of(weatherPlugin), Optional.of(plugin), Optional.of(configPlugin), ) @@ -218,11 +230,22 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { deviceProvisionedListener = deviceProvisionedCaptor.value } + @Test(expected = RuntimeException::class) + fun testBuildAndConnectWeatherView_throwsIfDecouplingDisabled() { + // GIVEN the feature flag is disabled + `when`(featureFlags.isEnabled(Flags.SMARTSPACE_DATE_WEATHER_DECOUPLED)).thenReturn(false) + + // WHEN we try to build the view + controller.buildAndConnectWeatherView(fakeParent) + + // THEN an exception is thrown + } + @Test - fun connectOnlyAfterDeviceIsProvisioned() { + fun testBuildAndConnectView_connectsOnlyAfterDeviceIsProvisioned() { // GIVEN an unprovisioned device and an attempt to connect - `when`(deviceProvisionedController.isDeviceProvisioned()).thenReturn(false) - `when`(deviceProvisionedController.isCurrentUserSetup()).thenReturn(false) + `when`(deviceProvisionedController.isDeviceProvisioned).thenReturn(false) + `when`(deviceProvisionedController.isCurrentUserSetup).thenReturn(false) // WHEN a connection attempt is made and view is attached val view = controller.buildAndConnectView(fakeParent) @@ -232,8 +255,8 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { verify(smartspaceManager, never()).createSmartspaceSession(any()) // WHEN it does become provisioned - `when`(deviceProvisionedController.isDeviceProvisioned()).thenReturn(true) - `when`(deviceProvisionedController.isCurrentUserSetup()).thenReturn(true) + `when`(deviceProvisionedController.isDeviceProvisioned).thenReturn(true) + `when`(deviceProvisionedController.isCurrentUserSetup).thenReturn(true) deviceProvisionedListener.onUserSetupChanged() // THEN the session is created @@ -243,7 +266,7 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { } @Test - fun testListenersAreRegistered() { + fun testAddListener_registersListenersForPlugin() { // GIVEN a listener is added after a session is created connectSession() @@ -252,10 +275,12 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { // THEN the listener is registered to the underlying plugin verify(plugin).registerListener(controllerListener) + // The listener is registered only for the plugin, not the weather plugin. + verify(weatherPlugin, never()).registerListener(any()) } @Test - fun testEarlyRegisteredListenersAreAttachedAfterConnected() { + fun testAddListener_earlyRegisteredListenersAreAttachedAfterConnected() { // GIVEN a listener that is registered before the session is created controller.addListener(controllerListener) @@ -264,10 +289,12 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { // THEN the listener is subsequently registered verify(plugin).registerListener(controllerListener) + // The listener is registered only for the plugin, not the weather plugin. + verify(weatherPlugin, never()).registerListener(any()) } @Test - fun testEmptyListIsEmittedAndNotifierRemovedAfterDisconnect() { + fun testDisconnect_emitsEmptyListAndRemovesNotifier() { // GIVEN a registered listener on an active session connectSession() clearInvocations(plugin) @@ -279,10 +306,12 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { // THEN the listener receives an empty list of targets and unregisters the notifier verify(plugin).onTargetsAvailable(emptyList()) verify(plugin).registerSmartspaceEventNotifier(null) + verify(weatherPlugin).onTargetsAvailable(emptyList()) + verify(weatherPlugin).registerSmartspaceEventNotifier(null) } @Test - fun testUserChangeReloadsSmartspace() { + fun testUserChange_reloadsSmartspace() { // GIVEN a connected smartspace session connectSession() @@ -294,7 +323,7 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { } @Test - fun testSettingsChangeReloadsSmartspace() { + fun testSettingsChange_reloadsSmartspace() { // GIVEN a connected smartspace session connectSession() @@ -306,7 +335,7 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { } @Test - fun testThemeChangeUpdatesTextColor() { + fun testThemeChange_updatesTextColor() { // GIVEN a connected smartspace session connectSession() @@ -318,7 +347,22 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { } @Test - fun testDozeAmountChangeUpdatesView() { + fun testThemeChange_ifDecouplingEnabled_updatesTextColor() { + `when`(featureFlags.isEnabled(Flags.SMARTSPACE_DATE_WEATHER_DECOUPLED)).thenReturn(true) + + // GIVEN a connected smartspace session + connectSession() + + // WHEN the theme changes + configChangeListener.onThemeChanged() + + // We update the new text color to match the wallpaper color + verify(weatherSmartspaceView).setPrimaryTextColor(anyInt()) + verify(smartspaceView).setPrimaryTextColor(anyInt()) + } + + @Test + fun testDozeAmountChange_updatesView() { // GIVEN a connected smartspace session connectSession() @@ -330,7 +374,22 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { } @Test - fun testKeyguardBypassEnabledUpdatesView() { + fun testDozeAmountChange_ifDecouplingEnabled_updatesViews() { + `when`(featureFlags.isEnabled(Flags.SMARTSPACE_DATE_WEATHER_DECOUPLED)).thenReturn(true) + + // GIVEN a connected smartspace session + connectSession() + + // WHEN the doze amount changes + statusBarStateListener.onDozeAmountChanged(0.1f, 0.7f) + + // We pass that along to the view + verify(weatherSmartspaceView).setDozeAmount(0.7f) + verify(smartspaceView).setDozeAmount(0.7f) + } + + @Test + fun testKeyguardBypassEnabled_updatesView() { // GIVEN a connected smartspace session connectSession() `when`(keyguardBypassController.bypassEnabled).thenReturn(true) @@ -424,6 +483,27 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { ))) } + @Test + fun testSessionListener_ifDecouplingEnabled_weatherTargetIsFilteredOut() { + `when`(featureFlags.isEnabled(Flags.SMARTSPACE_DATE_WEATHER_DECOUPLED)).thenReturn(true) + connectSession() + + // WHEN we receive a list of targets + val targets = listOf( + makeTarget(1, userHandlePrimary, isSensitive = true), + makeTarget(2, userHandlePrimary), + makeTarget(3, userHandleManaged), + makeTarget(4, userHandlePrimary, featureType = SmartspaceTarget.FEATURE_WEATHER) + ) + + sessionListener.onTargetsAvailable(targets) + + // THEN all non-sensitive content is still shown + verify(plugin).onTargetsAvailable(eq(listOf(targets[0], targets[1], targets[2]))) + // No filtering is applied for the weather plugin + verify(weatherPlugin).onTargetsAvailable(eq(targets)) + } + @Test fun testSettingsAreReloaded() { // GIVEN a connected session where the privacy settings later flip to false @@ -514,6 +594,16 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { verify(smartspaceView2).registerConfigProvider(configPlugin) } + @Test + fun testWeatherViewUsesSameSession() { + `when`(featureFlags.isEnabled(Flags.SMARTSPACE_DATE_WEATHER_DECOUPLED)).thenReturn(true) + // GIVEN a connected session + connectSession() + + // No checks is needed here, since connectSession() already checks internally that + // createSmartspaceSession is invoked only once. + } + @Test fun testViewGetInitializedWithBypassEnabledState() { // GIVEN keyguard bypass is enabled. @@ -531,8 +621,8 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { fun testConnectAttemptBeforeInitializationShouldNotCreateSession() { // GIVEN an uninitalized smartspaceView // WHEN the device is provisioned - `when`(deviceProvisionedController.isDeviceProvisioned()).thenReturn(true) - `when`(deviceProvisionedController.isCurrentUserSetup()).thenReturn(true) + `when`(deviceProvisionedController.isDeviceProvisioned).thenReturn(true) + `when`(deviceProvisionedController.isCurrentUserSetup).thenReturn(true) deviceProvisionedListener.onDeviceProvisionedChanged() // THEN no calls to createSmartspaceSession should occur @@ -542,9 +632,23 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { } private fun connectSession() { + if (controller.isDateWeatherDecoupled()) { + val weatherView = controller.buildAndConnectWeatherView(fakeParent) + weatherSmartspaceView = weatherView as SmartspaceView + fakeParent.addView(weatherView) + controller.stateChangeListener.onViewAttachedToWindow(weatherView) + + verify(weatherSmartspaceView).setUiSurface( + BcSmartspaceDataPlugin.UI_SURFACE_LOCK_SCREEN_AOD) + verify(weatherSmartspaceView).registerDataProvider(weatherPlugin) + + verify(weatherSmartspaceView).setPrimaryTextColor(anyInt()) + verify(weatherSmartspaceView).setDozeAmount(0.5f) + } + val view = controller.buildAndConnectView(fakeParent) smartspaceView = view as SmartspaceView - + fakeParent.addView(view) controller.stateChangeListener.onViewAttachedToWindow(view) verify(smartspaceView).setUiSurface(BcSmartspaceDataPlugin.UI_SURFACE_LOCK_SCREEN_AOD) @@ -554,6 +658,8 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { .addOnTargetsAvailableListener(any(), capture(sessionListenerCaptor)) sessionListener = sessionListenerCaptor.value + verify(smartspaceManager).createSmartspaceSession(any()) + verify(userTracker).addCallback(capture(userTrackerCaptor), any()) userListener = userTrackerCaptor.value @@ -578,9 +684,11 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { verify(smartspaceView).setPrimaryTextColor(anyInt()) verify(smartspaceView).setDozeAmount(0.5f) - clearInvocations(view) - fakeParent.addView(view) + if (controller.isDateWeatherDecoupled()) { + clearInvocations(weatherSmartspaceView) + } + clearInvocations(smartspaceView) } private fun setActiveUser(userHandle: UserHandle) { @@ -626,6 +734,31 @@ class LockscreenSmartspaceControllerTest : SysuiTestCase() { ).thenReturn(if (value) 1 else 0) } + // Separate function for the weather view, which doesn't implement all functions in interface. + private fun createWeatherSmartspaceView(): SmartspaceView { + return spy(object : View(context), SmartspaceView { + override fun registerDataProvider(plugin: BcSmartspaceDataPlugin?) { + } + + override fun setPrimaryTextColor(color: Int) { + } + + override fun setIsDreaming(isDreaming: Boolean) { + } + + override fun setUiSurface(uiSurface: String) { + } + + override fun setDozeAmount(amount: Float) { + } + + override fun setIntentStarter(intentStarter: BcSmartspaceDataPlugin.IntentStarter?) { + } + + override fun setFalsingManager(falsingManager: FalsingManager?) { + } + }) + } private fun createSmartspaceView(): SmartspaceView { return spy(object : View(context), SmartspaceView { override fun registerDataProvider(plugin: BcSmartspaceDataPlugin?) {