From 8fd9d5571d6b58fe97cab4d9da0362dbfbf38c3d Mon Sep 17 00:00:00 2001 From: Jernej Virag Date: Fri, 28 Jul 2023 11:50:17 +0200 Subject: [PATCH 1/2] Register Notification Memory Logging after boot completed Based on statsd team recommendations, we probably shouldn't be talking to stats service before LOCKED_BOOT_COMPLETED has triggered. This change moves registration of statsd callbacks after that event. Bug: 283841311 Test: using bugreports on panther Change-Id: I6b3784990ad71b11af33ac790d3cb012f45679ff --- .../dagger/NotificationMemoryModule.kt | 33 +++++++++++++++++++ .../init/NotificationsControllerImpl.kt | 3 -- .../logging/NotificationMemoryMonitor.kt | 5 +-- 3 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationMemoryModule.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationMemoryModule.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationMemoryModule.kt new file mode 100644 index 0000000000000..92e9765936d7d --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/dagger/NotificationMemoryModule.kt @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2023 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.dagger + +import com.android.systemui.CoreStartable +import com.android.systemui.statusbar.notification.logging.NotificationMemoryMonitor +import dagger.Binds +import dagger.Module +import dagger.multibindings.ClassKey +import dagger.multibindings.IntoMap + +@Module +interface NotificationMemoryModule { + + /** Binds memory monitor into startable map. */ + @Binds + @IntoMap + @ClassKey(NotificationMemoryMonitor::class) + fun bindsNotificationMemoryMonitorStartable(monitor: NotificationMemoryMonitor): CoreStartable +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt index f7bd177594a26..106d11f6bcc13 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt @@ -39,7 +39,6 @@ import com.android.systemui.statusbar.notification.collection.notifcollection.No import com.android.systemui.statusbar.notification.collection.render.NotifStackController import com.android.systemui.statusbar.notification.interruption.HeadsUpViewBinder import com.android.systemui.statusbar.notification.logging.NotificationLogger -import com.android.systemui.statusbar.notification.logging.NotificationMemoryMonitor import com.android.systemui.statusbar.notification.row.NotifBindPipelineInitializer import com.android.systemui.statusbar.notification.stack.NotificationListContainer import com.android.wm.shell.bubbles.Bubbles @@ -72,7 +71,6 @@ class NotificationsControllerImpl @Inject constructor( private val peopleSpaceWidgetManager: PeopleSpaceWidgetManager, private val bubblesOptional: Optional, private val fgsNotifListener: ForegroundServiceNotificationListener, - private val memoryMonitor: Lazy, private val featureFlags: FeatureFlags ) : NotificationsController { @@ -108,7 +106,6 @@ class NotificationsControllerImpl @Inject constructor( notificationLogger.setUpWithContainer(listContainer) peopleSpaceWidgetManager.attach(notificationListener) fgsNotifListener.init() - memoryMonitor.get().init() } // TODO: Convert all functions below this line into listeners instead of public methods diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryMonitor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryMonitor.kt index f38c1e557b255..0fdf681aac260 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryMonitor.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryMonitor.kt @@ -18,6 +18,7 @@ package com.android.systemui.statusbar.notification.logging import android.util.Log +import com.android.systemui.CoreStartable import com.android.systemui.dagger.SysUISingleton import com.android.systemui.flags.FeatureFlags import com.android.systemui.flags.Flags @@ -32,13 +33,13 @@ constructor( private val featureFlags: FeatureFlags, private val notificationMemoryDumper: NotificationMemoryDumper, private val notificationMemoryLogger: Lazy, -) { +) : CoreStartable { companion object { private const val TAG = "NotificationMemory" } - fun init() { + override fun start() { Log.d(TAG, "NotificationMemoryMonitor initialized.") notificationMemoryDumper.init() if (featureFlags.isEnabled(Flags.NOTIFICATION_MEMORY_LOGGING_ENABLED)) { From d953cfb2d4b05915e5a8026c4d7d8420939f0e46 Mon Sep 17 00:00:00 2001 From: Jernej Virag Date: Fri, 28 Jul 2023 12:38:41 +0200 Subject: [PATCH 2/2] Enable listener for statsd notification memory collection This reenables registration for notification memory collection. To actually collect the data, server-side flag rollout still needs to happen. Bug: 290620219 Test: unit tests covering the class, manually checked on panther to verify no crashes Change-Id: If58bf46a676b3ea39b47a962853cc5484b9c2054 --- packages/SystemUI/src/com/android/systemui/flags/Flags.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt index b04d9705f857c..faab57cd7bc3c 100644 --- a/packages/SystemUI/src/com/android/systemui/flags/Flags.kt +++ b/packages/SystemUI/src/com/android/systemui/flags/Flags.kt @@ -62,11 +62,12 @@ object Flags { val INSTANT_VOICE_REPLY = unreleasedFlag(111, "instant_voice_reply") /** - * This flag is server-controlled and should stay as [unreleasedFlag] since we never want to - * enable it on release builds. + * This flag controls whether we register a listener for StatsD notification memory reports. + * For statsd to actually call the listener however, a server-side toggle needs to be + * enabled as well. */ val NOTIFICATION_MEMORY_LOGGING_ENABLED = - unreleasedFlag(119, "notification_memory_logging_enabled") + releasedFlag(119, "notification_memory_logging_enabled") // TODO(b/260335638): Tracking Bug @JvmField