cros: Remove notification reporter.

Notification reporter intends to treat notification as user activity for
screen dim and idle timeouts. Thus it notifies powerd whenever a new
notification (priority > DEFAULT) is drawn or the one that is already
shown is updated.

When a notification of system priority is drawn, chrome keeps updating
the notification until is cleared by user which results in
regular HandleWakeNotification calls to powerd. Thus this can result in
screen staying on forever. Let us not do this.

In the follow up CL, I intend to notify powerd directly where the
notification is created. This way we do not use notification priority
that is already overloaded to determine if the screen should stay on.

BUG=chromium:964470
TEST=Connect external monitor and make sure screen turns off.

Change-Id: I616926994f66982ecb5886f17af85813ee34efe4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1618047
Commit-Queue: Ravi Chandra Sadineni <ravisadineni@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Auto-Submit: Ravi Chandra Sadineni <ravisadineni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661116}

ash/BUILD.gn

@@ -824,8 +824,6 @@ component("ash") {
     "system/power/battery_notification.h",
     "system/power/dual_role_notification.cc",
     "system/power/dual_role_notification.h",
-    "system/power/notification_reporter.cc",
-    "system/power/notification_reporter.h",
     "system/power/peripheral_battery_notifier.cc",
     "system/power/peripheral_battery_notifier.h",
     "system/power/power_button_controller.cc",
@@ -1771,7 +1769,6 @@ test("ash_unittests") {
     "system/palette/tools/metalayer_unittest.cc",
     "system/palette/tools/screenshot_unittest.cc",
     "system/power/backlights_forced_off_setter_unittest.cc",
-    "system/power/notification_reporter_unittest.cc",
     "system/power/peripheral_battery_notifier_unittest.cc",
     "system/power/power_button_controller_unittest.cc",
     "system/power/power_button_screenshot_controller_unittest.cc",

ash/shell.cc

@@ -106,7 +106,6 @@
 #include "ash/system/network/vpn_list.h"
 #include "ash/system/night_light/night_light_controller.h"
 #include "ash/system/power/backlights_forced_off_setter.h"
-#include "ash/system/power/notification_reporter.h"
 #include "ash/system/power/peripheral_battery_notifier.h"
 #include "ash/system/power/power_button_controller.h"
 #include "ash/system/power/power_event_observer.h"
@@ -833,7 +832,6 @@ Shell::~Shell() {
   PowerStatus::Shutdown();
   // Depends on SessionController.
   power_event_observer_.reset();
-  notification_reporter_.reset();
 
   session_controller_->RemoveObserver(this);
   // BluetoothPowerController depends on the PrefService and must be destructed
@@ -1162,8 +1160,6 @@ void Shell::Init(
 
   user_metrics_recorder_->OnShellInitialized();
 
-  notification_reporter_ = std::make_unique<NotificationReporter>();
-
   // Initialize the D-Bus bus and services for ash.
   if (!::features::IsMultiProcessMash())
     ash_dbus_helper_ = AshDBusHelper::CreateWithExistingBus(dbus_bus);

ash/shell.h

@@ -156,7 +156,6 @@ class PersistentWindowController;
 class PolicyRecommendationRestorer;
 class PowerButtonController;
 class PowerEventObserver;
-class NotificationReporter;
 class PowerPrefs;
 class ProjectingObserver;
 class ResizeShadowController;
@@ -431,9 +430,6 @@ class ASH_EXPORT Shell : public SessionObserver,
   NoteTakingController* note_taking_controller() {
     return note_taking_controller_.get();
   }
-  NotificationReporter* notification_reporter() {
-    return notification_reporter_.get();
-  }
   OverlayEventFilter* overlay_filter() { return overlay_filter_.get(); }
   PartialMagnificationController* partial_magnification_controller() {
     return partial_magnification_controller_.get();
@@ -834,8 +830,6 @@ class ASH_EXPORT Shell : public SessionObserver,
 
   std::unique_ptr<MessageCenterController> message_center_controller_;
 
-  std::unique_ptr<NotificationReporter> notification_reporter_;
-
   base::ObserverList<ShellObserver>::Unchecked shell_observers_;
 
   base::WeakPtrFactory<Shell> weak_factory_;

ash/system/power/notification_reporter.cc

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "ash/system/power/notification_reporter.h"
-
-#include "chromeos/dbus/power/power_manager_client.h"
-#include "ui/message_center/message_center.h"
-#include "ui/message_center/public/cpp/notification.h"
-#include "ui/message_center/public/cpp/notification_types.h"
-
-namespace ash {
-
-NotificationReporter::NotificationReporter() {
-  message_center::MessageCenter::Get()->AddObserver(this);
-}
-
-NotificationReporter::~NotificationReporter() {
-  message_center::MessageCenter::Get()->RemoveObserver(this);
-}
-
-void NotificationReporter::OnNotificationAdded(
-    const std::string& notification_id) {
-  MaybeNotifyPowerManager(notification_id);
-}
-
-void NotificationReporter::OnNotificationUpdated(
-    const std::string& notification_id) {
-  MaybeNotifyPowerManager(notification_id);
-}
-
-void NotificationReporter::MaybeNotifyPowerManager(
-    const std::string& notification_id) {
-  // Guard against notification removals before observers are notified.
-  message_center::Notification* notification =
-      message_center::MessageCenter::Get()->FindVisibleNotificationById(
-          notification_id);
-  if (!notification)
-    return;
-
-  // If this is a high priority notification wake the display up if the
-  // system is in dark resume i.e. transition to full resume.
-  if (notification->priority() >
-      message_center::NotificationPriority::DEFAULT_PRIORITY) {
-    chromeos::PowerManagerClient::Get()->NotifyWakeNotification();
-  }
-}
-
-}  // namespace ash

ash/system/power/notification_reporter.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef ASH_SYSTEM_POWER_NOTIFICATION_REPORTER_H_
-#define ASH_SYSTEM_POWER_NOTIFICATION_REPORTER_H_
-
-#include <string>
-
-#include "ash/ash_export.h"
-#include "base/macros.h"
-#include "ui/message_center/message_center_observer.h"
-
-namespace ash {
-
-// This class notifies the power manager when a high priority notification i.e.
-// one that can wake up the device (alarms, push notifications etc.), is created
-// or updated. This is required for doing a full resume if the device woke up in
-// dark resume in response to a notification.
-class ASH_EXPORT NotificationReporter
-    : public message_center::MessageCenterObserver {
- public:
-  NotificationReporter();
-  ~NotificationReporter() override;
-
-  // Overridden from MessageCenterObserver:
-  void OnNotificationAdded(const std::string& notification_id) override;
-  void OnNotificationUpdated(const std::string& notification_id) override;
-
- private:
-  // Notifies power manager if the notification corresponding to
-  // |notification_id| has high priority.
-  void MaybeNotifyPowerManager(const std::string& notification_id);
-
-  DISALLOW_COPY_AND_ASSIGN(NotificationReporter);
-};
-
-}  // namespace ash
-
-#endif  // ASH_SYSTEM_POWER_NOTIFICATION_REPORTER_H_

ash/system/power/notification_reporter_unittest.cc

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "ash/system/power/notification_reporter.h"
-
-#include <memory>
-#include <string>
-#include <utility>
-
-#include "ash/shell.h"
-#include "ash/test/ash_test_base.h"
-#include "chromeos/dbus/power/fake_power_manager_client.h"
-#include "ui/message_center/fake_message_center.h"
-#include "ui/message_center/public/cpp/notification.h"
-
-namespace ash {
-
-using NotificationReporterTest = AshTestBase;
-
-TEST_F(NotificationReporterTest, CheckNotifyWakeNotification) {
-  // Create a high priority notification and check that the power manager got
-  // called.
-  message_center::Notification notification(
-      message_center::NOTIFICATION_TYPE_SIMPLE, "notification-id", {}, {}, {},
-      {}, {}, {}, {}, nullptr);
-  notification.set_priority(
-      static_cast<int>(message_center::NotificationPriority::HIGH_PRIORITY));
-  message_center::MessageCenter::Get()->AddNotification(
-      std::make_unique<message_center::Notification>(notification));
-  EXPECT_EQ(
-      1,
-      chromeos::FakePowerManagerClient::Get()->num_wake_notification_calls());
-
-  // Update the old notification. Check if the power manager got called again.
-  message_center::MessageCenter::Get()->UpdateNotification(
-      notification.id(),
-      std::make_unique<message_center::Notification>(notification));
-  EXPECT_EQ(
-      2,
-      chromeos::FakePowerManagerClient::Get()->num_wake_notification_calls());
-
-  // A low priority notification should not result in any calls to the power
-  // manager.
-  notification.set_priority(
-      static_cast<int>(message_center::NotificationPriority::LOW_PRIORITY));
-  message_center::MessageCenter::Get()->AddNotification(
-      std::make_unique<message_center::Notification>("different-id",
-                                                     notification));
-  EXPECT_EQ(
-      2,
-      chromeos::FakePowerManagerClient::Get()->num_wake_notification_calls());
-}
-
-TEST_F(NotificationReporterTest, CheckDismissedNotification) {
-  // Create a high priority notification and check that the power manager got
-  // called.
-  message_center::Notification notification(
-      message_center::NOTIFICATION_TYPE_SIMPLE, "notification-id", {}, {}, {},
-      {}, {}, {}, {}, nullptr);
-  notification.set_priority(
-      static_cast<int>(message_center::NotificationPriority::HIGH_PRIORITY));
-  message_center::MessageCenter::Get()->AddNotification(
-      std::make_unique<message_center::Notification>(notification));
-  EXPECT_EQ(
-      1,
-      chromeos::FakePowerManagerClient::Get()->num_wake_notification_calls());
-
-  // Remove the notification from the message center and then directly call the
-  // observer API. This shouldn't call the power manager as NotificationReporter
-  // won't be able to find the notification and consequently determine it's
-  // priority.
-  message_center::MessageCenter::Get()->RemoveNotification(notification.id(),
-                                                           false /* by_user */);
-  Shell::Get()->notification_reporter()->OnNotificationUpdated(
-      notification.id());
-  EXPECT_EQ(
-      1,
-      chromeos::FakePowerManagerClient::Get()->num_wake_notification_calls());
-}
-
-}  // namespace ash