From e33987ccd5912b520d61c783895f9e181a8ea980 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Tue, 13 Jun 2023 15:16:55 +0000 Subject: [PATCH] [CS] Make NSSLC and NotificationListContainer singletons. Note on the Dagger module changes: Previously, NSSLC and its dependencies were part of CentralSurfacesComponent, so the submodules that included NSSLC's dependencies only needed to be in CentralSurfacesComponent. Now that NSSLC is a singleton, those dependencies need to be available *outside* of CentralSurfacesComponent. NotificationsModule is the logical place for them, because they're notifications-related. Now, when NSSLC is injected into CentralSurfacesImpl, it can pull its dependencies from any modules that SysUI has, which includes NotificationsModule. Bug: 277762009 Test: compiles Test: notifications smoke test (posting new notifs, expanding & collapsing notifs, swiping notifs away, changing font size & display scale) Test: atest CentralSurfacesImplTest Test: `m out/soong/.intermediates/vendor/google_arc/packages/system/ArcSystemUI/ArcSystemUI-tests/android_common/kapt/kapt-sources.jar` succeeds Change-Id: Ia9c629c7dab06204568a4f04ab1a092db28be467 --- .../inflation/NotificationRowBinderImpl.java | 1 - .../dagger/NotificationsModule.java | 14 ++++++++ .../ExpandableNotificationRowComponent.java | 3 -- .../viewmodel/NotificationShelfViewModel.kt | 4 +-- ...tificationStackScrollLayoutController.java | 6 ++-- ...nStackScrollLayoutListContainerModule.java | 32 ------------------- .../ui/viewmodel/NotificationListViewModel.kt | 2 ++ .../statusbar/phone/CentralSurfacesImpl.java | 15 ++++----- .../dagger/CentralSurfacesComponent.java | 9 ------ .../phone/dagger/StatusBarViewModule.java | 15 +++++---- .../phone/CentralSurfacesImplTest.java | 3 +- 11 files changed, 37 insertions(+), 67 deletions(-) delete mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutListContainerModule.java diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java index 38c37239a7d3a..189608072ec7a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java @@ -140,7 +140,6 @@ public class NotificationRowBinderImpl implements NotificationRowBinder { .expandableNotificationRow(row) .notificationEntry(entry) .onExpandClickListener(mPresenter) - .listContainer(mListContainer) .build(); ExpandableNotificationRowController rowController = component.getExpandableNotificationRowController(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java index b100d44bbc5df..31d4ab99098e0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationsModule.java @@ -63,8 +63,12 @@ import com.android.systemui.statusbar.notification.logging.NotificationPanelLogg import com.android.systemui.statusbar.notification.logging.NotificationPanelLoggerImpl; import com.android.systemui.statusbar.notification.row.NotificationGutsManager; import com.android.systemui.statusbar.notification.row.OnUserInteractionCallback; +import com.android.systemui.statusbar.notification.row.ui.viewmodel.ActivatableNotificationViewModelModule; +import com.android.systemui.statusbar.notification.stack.NotificationListContainer; import com.android.systemui.statusbar.notification.stack.NotificationSectionsManager; +import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController; import com.android.systemui.statusbar.notification.stack.StackScrollAlgorithm; +import com.android.systemui.statusbar.notification.stack.ui.viewmodel.NotificationListViewModelModule; import com.android.systemui.statusbar.phone.KeyguardBypassController; import dagger.Binds; @@ -85,6 +89,8 @@ import javax.inject.Provider; ShadeEventsModule.class, NotifPipelineChoreographerModule.class, NotificationSectionHeadersModule.class, + NotificationListViewModelModule.class, + ActivatableNotificationViewModelModule.class, }) public interface NotificationsModule { @Binds @@ -159,6 +165,14 @@ public interface NotificationsModule { } } + /** Provides the container for the notification list. */ + @Provides + @SysUISingleton + static NotificationListContainer provideListContainer( + NotificationStackScrollLayoutController nsslController) { + return nsslController.getNotificationListContainer(); + } + /** * Provide the active notification collection managing the notifications to render. */ diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/dagger/ExpandableNotificationRowComponent.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/dagger/ExpandableNotificationRowComponent.java index 1a7417a781865..3588621b12ad6 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/dagger/ExpandableNotificationRowComponent.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/dagger/ExpandableNotificationRowComponent.java @@ -25,7 +25,6 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.row.ActivatableNotificationView; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRowController; -import com.android.systemui.statusbar.notification.stack.NotificationListContainer; import com.android.systemui.statusbar.phone.CentralSurfaces; import dagger.Binds; @@ -59,8 +58,6 @@ public interface ExpandableNotificationRowComponent { Builder notificationEntry(NotificationEntry entry); @BindsInstance Builder onExpandClickListener(ExpandableNotificationRow.OnExpandClickListener presenter); - @BindsInstance - Builder listContainer(NotificationListContainer listContainer); ExpandableNotificationRowComponent build(); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/shelf/ui/viewmodel/NotificationShelfViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/shelf/ui/viewmodel/NotificationShelfViewModel.kt index fb1944315e104..5ca8b53d07044 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/shelf/ui/viewmodel/NotificationShelfViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/shelf/ui/viewmodel/NotificationShelfViewModel.kt @@ -16,16 +16,16 @@ package com.android.systemui.statusbar.notification.shelf.ui.viewmodel +import com.android.systemui.dagger.SysUISingleton import com.android.systemui.statusbar.NotificationShelf import com.android.systemui.statusbar.notification.row.ui.viewmodel.ActivatableNotificationViewModel import com.android.systemui.statusbar.notification.shelf.domain.interactor.NotificationShelfInteractor -import com.android.systemui.statusbar.phone.dagger.CentralSurfacesComponent.CentralSurfacesScope import javax.inject.Inject import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map /** ViewModel for [NotificationShelf]. */ -@CentralSurfacesScope +@SysUISingleton class NotificationShelfViewModel @Inject constructor( diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java index ad7cdc4eb50f8..86b88ccf3ca0d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutController.java @@ -59,6 +59,7 @@ import com.android.systemui.Gefingerpoken; import com.android.systemui.bouncer.domain.interactor.PrimaryBouncerInteractor; import com.android.systemui.classifier.Classifier; import com.android.systemui.classifier.FalsingCollector; +import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.dump.DumpManager; import com.android.systemui.flags.FeatureFlags; @@ -118,7 +119,6 @@ import com.android.systemui.statusbar.phone.HeadsUpTouchHelper; import com.android.systemui.statusbar.phone.KeyguardBypassController; import com.android.systemui.statusbar.phone.NotificationIconAreaController; import com.android.systemui.statusbar.phone.ScrimController; -import com.android.systemui.statusbar.phone.dagger.CentralSurfacesComponent; import com.android.systemui.statusbar.policy.ConfigurationController; import com.android.systemui.statusbar.policy.ConfigurationController.ConfigurationListener; import com.android.systemui.statusbar.policy.DeviceProvisionedController; @@ -141,7 +141,7 @@ import javax.inject.Named; /** * Controller for {@link NotificationStackScrollLayout}. */ -@CentralSurfacesComponent.CentralSurfacesScope +@SysUISingleton public class NotificationStackScrollLayoutController { private static final String TAG = "StackScrollerController"; private static final boolean DEBUG = Compile.IS_DEBUG && Log.isLoggable(TAG, Log.DEBUG); @@ -1462,7 +1462,7 @@ public class NotificationStackScrollLayoutController { return mNotificationRoundnessManager; } - NotificationListContainer getNotificationListContainer() { + public NotificationListContainer getNotificationListContainer() { return mNotificationListContainer; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutListContainerModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutListContainerModule.java deleted file mode 100644 index 3dcaae28e1819..0000000000000 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutListContainerModule.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright (C) 2022 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.statusbar.notification.stack; - -import com.android.systemui.statusbar.phone.dagger.CentralSurfacesComponent; - -import dagger.Module; -import dagger.Provides; - -@Module -public abstract class NotificationStackScrollLayoutListContainerModule { - @Provides - @CentralSurfacesComponent.CentralSurfacesScope - static NotificationListContainer provideListContainer( - NotificationStackScrollLayoutController nsslController) { - return nsslController.getNotificationListContainer(); - } -} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModel.kt index aab1c2b877e8b..11f68e0f663b6 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/ui/viewmodel/NotificationListViewModel.kt @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.notification.stack.ui.viewmodel +import com.android.systemui.dagger.SysUISingleton import com.android.systemui.flags.FeatureFlags import com.android.systemui.flags.Flags import com.android.systemui.statusbar.notification.shelf.ui.viewmodel.NotificationShelfViewModel @@ -33,6 +34,7 @@ class NotificationListViewModel( object NotificationListViewModelModule { @JvmStatic @Provides + @SysUISingleton fun maybeProvideViewModel( featureFlags: FeatureFlags, shelfViewModel: Provider, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java index bba581b7f8586..40cdf3c2ee004 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java @@ -312,7 +312,7 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { private final DeviceStateManager mDeviceStateManager; private CentralSurfacesCommandQueueCallbacks mCommandQueueCallbacks; private float mTransitionToFullShadeProgress = 0f; - private NotificationListContainer mNotifListContainer; + private final NotificationListContainer mNotifListContainer; private boolean mIsShortcutListSearchEnabled; private final KeyguardStateController.Callback mKeyguardStateControllerCallback = @@ -660,8 +660,7 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { private final ActivityIntentHelper mActivityIntentHelper; - @VisibleForTesting - protected NotificationStackScrollLayoutController mStackScrollerController; + private final NotificationStackScrollLayoutController mStackScrollerController; private final ColorExtractor.OnColorsChangedListener mOnColorsChangedListener = (extractor, which) -> updateTheme(); @@ -749,6 +748,7 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { ConfigurationController configurationController, NotificationShadeWindowController notificationShadeWindowController, NotificationShelfController notificationShelfController, + NotificationStackScrollLayoutController notificationStackScrollLayoutController, DozeParameters dozeParameters, ScrimController scrimController, Lazy lockscreenWallpaperLazy, @@ -849,6 +849,9 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { mConfigurationController = configurationController; mNotificationShadeWindowController = notificationShadeWindowController; mNotificationShelfController = notificationShelfController; + mStackScrollerController = notificationStackScrollLayoutController; + mStackScroller = mStackScrollerController.getView(); + mNotifListContainer = mStackScrollerController.getNotificationListContainer(); mDozeServiceHost = dozeServiceHost; mPowerManager = powerManager; mDozeParameters = dozeParameters; @@ -1658,12 +1661,8 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { mShadeController.setNotificationPanelViewController(npvc); mShadeController.setNotificationShadeWindowViewController( mNotificationShadeWindowViewController); - mStackScrollerController = - mCentralSurfacesComponent.getNotificationStackScrollLayoutController(); mQsController = mCentralSurfacesComponent.getQuickSettingsController(); mBackActionInteractor.setup(mQsController, mShadeSurface); - mStackScroller = mStackScrollerController.getView(); - mNotifListContainer = mCentralSurfacesComponent.getNotificationListContainer(); mPresenter = mCentralSurfacesComponent.getNotificationPresenter(); mNotificationActivityStarter = mCentralSurfacesComponent.getNotificationActivityStarter(); @@ -3369,7 +3368,7 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { protected IStatusBarService mBarService; // all notifications - protected NotificationStackScrollLayout mStackScroller; + private final NotificationStackScrollLayout mStackScroller; protected AccessibilityManager mAccessibilityManager; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesComponent.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesComponent.java index 64c798b99a18c..c618be843832e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesComponent.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/CentralSurfacesComponent.java @@ -28,9 +28,6 @@ import com.android.systemui.shade.QuickSettingsController; import com.android.systemui.shade.ShadeHeaderController; import com.android.systemui.statusbar.NotificationPresenter; import com.android.systemui.statusbar.notification.NotificationActivityStarter; -import com.android.systemui.statusbar.notification.stack.NotificationListContainer; -import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController; -import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutListContainerModule; import com.android.systemui.statusbar.phone.CentralSurfacesCommandQueueCallbacks; import com.android.systemui.statusbar.phone.CentralSurfacesImpl; import com.android.systemui.statusbar.phone.StatusBarHeadsUpChangeListener; @@ -56,7 +53,6 @@ import javax.inject.Scope; * outside the component. Should more items be moved *into* this component to avoid so many getters? */ @Subcomponent(modules = { - NotificationStackScrollLayoutListContainerModule.class, StatusBarViewModule.class, StatusBarNotificationActivityStarterModule.class, StatusBarNotificationPresenterModule.class, @@ -87,9 +83,6 @@ public interface CentralSurfacesComponent { */ NotificationShadeWindowView getNotificationShadeWindowView(); - /** */ - NotificationStackScrollLayoutController getNotificationStackScrollLayoutController(); - /** * Creates a NotificationShadeWindowViewController. */ @@ -128,6 +121,4 @@ public interface CentralSurfacesComponent { NotificationActivityStarter getNotificationActivityStarter(); NotificationPresenter getNotificationPresenter(); - - NotificationListContainer getNotificationListContainer(); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/StatusBarViewModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/StatusBarViewModule.java index 260d986db6c0d..6b0746f08df09 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/StatusBarViewModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/StatusBarViewModule.java @@ -31,8 +31,6 @@ import com.android.systemui.shade.ShadeViewController; import com.android.systemui.statusbar.CommandQueue; import com.android.systemui.statusbar.OperatorNameViewController; import com.android.systemui.statusbar.events.SystemStatusAnimationScheduler; -import com.android.systemui.statusbar.notification.row.ui.viewmodel.ActivatableNotificationViewModelModule; -import com.android.systemui.statusbar.notification.stack.ui.viewmodel.NotificationListViewModelModule; import com.android.systemui.statusbar.phone.KeyguardBottomAreaView; import com.android.systemui.statusbar.phone.NotificationIconAreaController; import com.android.systemui.statusbar.phone.StatusBarBoundsProvider; @@ -60,11 +58,14 @@ import java.util.concurrent.Executor; import javax.inject.Named; -@Module(subcomponents = StatusBarFragmentComponent.class, - includes = { - ActivatableNotificationViewModelModule.class, - NotificationListViewModelModule.class, - }) +/** + * A module for {@link CentralSurfacesComponent.CentralSurfacesScope} components. + * + * @deprecated CentralSurfacesScope will be removed shortly (b/277762009). Classes should be + * annotated with @SysUISingleton instead. + */ +@Module(subcomponents = StatusBarFragmentComponent.class) +@Deprecated public abstract class StatusBarViewModule { public static final String STATUS_BAR_FRAGMENT = "status_bar_fragment"; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java index cc8324b220272..953ea3d1b2815 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java @@ -510,6 +510,7 @@ public class CentralSurfacesImplTest extends SysuiTestCase { configurationController, mNotificationShadeWindowController, mNotificationShelfController, + mStackScrollerController, mDozeParameters, mScrimController, mLockscreenWallpaperLazy, @@ -593,8 +594,6 @@ public class CentralSurfacesImplTest extends SysuiTestCase { mCentralSurfaces.mPresenter = mNotificationPresenter; mCentralSurfaces.mKeyguardIndicationController = mKeyguardIndicationController; mCentralSurfaces.mBarService = mBarService; - mCentralSurfaces.mStackScrollerController = mStackScrollerController; - mCentralSurfaces.mStackScroller = mStackScroller; mCentralSurfaces.mGestureWakeLock = mPowerManager.newWakeLock( PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "sysui:GestureWakeLock"); mCentralSurfaces.startKeyguard();