summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Kweku Adams <kwekua@google.com> 2019-12-10 16:32:55 -0800
committer Kweku Adams <kwekua@google.com> 2019-12-10 17:10:22 -0800
commit79a0faf869c24ba7b86116f28c4e431fe717da8c (patch)
tree12bde44bdc2ae880c68c87ce74de0eb519a2330a
parent4324208bfaf9993e54c01773d24c6b2add652f44 (diff)
Avoid lock inversion.
The PowerManager lock (which is used in the Battery Saver code) sits low in the lock hierarchy. NotificationManagerService is higher, so nothing should call into NotificationManagerService with the PowerManager lock held. This change calls into NMS on a background thread so that the call occurs without the PM lock held. Also fixing the cancel notification bug. We create the notifications using notifyAsUser(... UserHandle.ALL). The cancel() call wasn't properly dismissing the notification. Switching to cancelAsUser(UserHandle.ALL) fixes the issue. Bug: 145886051 Test: atest BatterySaverStateMachineTest Test: Manually ensure notification disappears when BS turns on Change-Id: I3315357e810d92938e4a34929235d233b07deebb
-rw-r--r--services/core/java/com/android/server/power/batterysaver/BatterySaverStateMachine.java67
1 files changed, 40 insertions, 27 deletions
diff --git a/services/core/java/com/android/server/power/batterysaver/BatterySaverStateMachine.java b/services/core/java/com/android/server/power/batterysaver/BatterySaverStateMachine.java
index ff9129956a3b..a62bb74730f8 100644
--- a/services/core/java/com/android/server/power/batterysaver/BatterySaverStateMachine.java
+++ b/services/core/java/com/android/server/power/batterysaver/BatterySaverStateMachine.java
@@ -773,9 +773,9 @@ public class BatterySaverStateMachine {
// Handle triggering the notification to show/hide when appropriate
if (intReason == BatterySaverController.REASON_DYNAMIC_POWER_SAVINGS_AUTOMATIC_ON) {
- runOnBgThread(this::triggerDynamicModeNotification);
+ triggerDynamicModeNotification();
} else if (!enable) {
- runOnBgThread(this::hideDynamicModeNotification);
+ hideDynamicModeNotification();
}
if (DEBUG) {
@@ -787,33 +787,42 @@ public class BatterySaverStateMachine {
@VisibleForTesting
void triggerDynamicModeNotification() {
- NotificationManager manager = mContext.getSystemService(NotificationManager.class);
- ensureNotificationChannelExists(manager, DYNAMIC_MODE_NOTIF_CHANNEL_ID,
- R.string.dynamic_mode_notification_channel_name);
-
- manager.notifyAsUser(TAG, DYNAMIC_MODE_NOTIFICATION_ID,
- buildNotification(DYNAMIC_MODE_NOTIF_CHANNEL_ID,
- mContext.getResources().getString(R.string.dynamic_mode_notification_title),
- R.string.dynamic_mode_notification_summary,
- Intent.ACTION_POWER_USAGE_SUMMARY),
- UserHandle.ALL);
+ // The current lock is the PowerManager lock, which sits very low in the service lock
+ // hierarchy. We shouldn't call out to NotificationManager with the PowerManager lock.
+ runOnBgThread(() -> {
+ NotificationManager manager = mContext.getSystemService(NotificationManager.class);
+ ensureNotificationChannelExists(manager, DYNAMIC_MODE_NOTIF_CHANNEL_ID,
+ R.string.dynamic_mode_notification_channel_name);
+
+ manager.notifyAsUser(TAG, DYNAMIC_MODE_NOTIFICATION_ID,
+ buildNotification(DYNAMIC_MODE_NOTIF_CHANNEL_ID,
+ mContext.getResources().getString(
+ R.string.dynamic_mode_notification_title),
+ R.string.dynamic_mode_notification_summary,
+ Intent.ACTION_POWER_USAGE_SUMMARY),
+ UserHandle.ALL);
+ });
}
@VisibleForTesting
void triggerStickyDisabledNotification() {
- NotificationManager manager = mContext.getSystemService(NotificationManager.class);
- ensureNotificationChannelExists(manager, BATTERY_SAVER_NOTIF_CHANNEL_ID,
- R.string.battery_saver_notification_channel_name);
-
- final String percentage = NumberFormat.getPercentInstance()
- .format((double) mBatteryLevel / 100.0);
- manager.notifyAsUser(TAG, STICKY_AUTO_DISABLED_NOTIFICATION_ID,
- buildNotification(BATTERY_SAVER_NOTIF_CHANNEL_ID,
- mContext.getResources().getString(
- R.string.battery_saver_charged_notification_title, percentage),
- R.string.battery_saver_off_notification_summary,
- Settings.ACTION_BATTERY_SAVER_SETTINGS),
- UserHandle.ALL);
+ // The current lock is the PowerManager lock, which sits very low in the service lock
+ // hierarchy. We shouldn't call out to NotificationManager with the PowerManager lock.
+ runOnBgThread(() -> {
+ NotificationManager manager = mContext.getSystemService(NotificationManager.class);
+ ensureNotificationChannelExists(manager, BATTERY_SAVER_NOTIF_CHANNEL_ID,
+ R.string.battery_saver_notification_channel_name);
+
+ final String percentage = NumberFormat.getPercentInstance()
+ .format((double) mBatteryLevel / 100.0);
+ manager.notifyAsUser(TAG, STICKY_AUTO_DISABLED_NOTIFICATION_ID,
+ buildNotification(BATTERY_SAVER_NOTIF_CHANNEL_ID,
+ mContext.getResources().getString(
+ R.string.battery_saver_charged_notification_title, percentage),
+ R.string.battery_saver_off_notification_summary,
+ Settings.ACTION_BATTERY_SAVER_SETTINGS),
+ UserHandle.ALL);
+ });
}
private void ensureNotificationChannelExists(NotificationManager manager,
@@ -854,8 +863,12 @@ public class BatterySaverStateMachine {
}
private void hideNotification(int notificationId) {
- NotificationManager manager = mContext.getSystemService(NotificationManager.class);
- manager.cancel(notificationId);
+ // The current lock is the PowerManager lock, which sits very low in the service lock
+ // hierarchy. We shouldn't call out to NotificationManager with the PowerManager lock.
+ runOnBgThread(() -> {
+ NotificationManager manager = mContext.getSystemService(NotificationManager.class);
+ manager.cancelAsUser(TAG, notificationId, UserHandle.ALL);
+ });
}
private void setStickyActive(boolean active) {