Use ThunkNotificationDelegate in ResolutionNotificationController.

Bug: none
Change-Id: Id4c4d26cd02c673b820a4b2cfcdcb226ac1f2759
Reviewed-on: https://chromium-review.googlesource.com/958092Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542587}

ash/display/resolution_notification_controller.cc

@@ -21,7 +21,6 @@
 #include "ui/display/screen.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/public/cpp/notification.h"
-#include "ui/message_center/public/cpp/notification_delegate.h"
 
 using message_center::Notification;
 
@@ -32,56 +31,6 @@ const char kNotifierDisplayResolutionChange[] = "ash.display.resolution-change";
 
 bool g_use_timer = true;
 
-class ResolutionChangeNotificationDelegate
-    : public message_center::NotificationDelegate {
- public:
-  ResolutionChangeNotificationDelegate(
-      ResolutionNotificationController* controller,
-      bool has_timeout);
-
- protected:
-  ~ResolutionChangeNotificationDelegate() override;
-
- private:
-  // message_center::NotificationDelegate overrides:
-  void Close(bool by_user) override;
-  void Click() override;
-  void ButtonClick(int button_index) override;
-
-  ResolutionNotificationController* controller_;
-  bool has_timeout_;
-
-  DISALLOW_COPY_AND_ASSIGN(ResolutionChangeNotificationDelegate);
-};
-
-ResolutionChangeNotificationDelegate::ResolutionChangeNotificationDelegate(
-    ResolutionNotificationController* controller,
-    bool has_timeout)
-    : controller_(controller), has_timeout_(has_timeout) {
-  DCHECK(controller_);
-}
-
-ResolutionChangeNotificationDelegate::~ResolutionChangeNotificationDelegate() =
-    default;
-
-void ResolutionChangeNotificationDelegate::Close(bool by_user) {
-  if (by_user)
-    controller_->AcceptResolutionChange(false);
-}
-
-void ResolutionChangeNotificationDelegate::Click() {
-  controller_->AcceptResolutionChange(true);
-}
-
-void ResolutionChangeNotificationDelegate::ButtonClick(int button_index) {
-  // If there's the timeout, the first button is "Accept". Otherwise the
-  // button click should be "Revert".
-  if (has_timeout_ && button_index == 0)
-    controller_->AcceptResolutionChange(true);
-  else
-    controller_->RevertResolutionChange(false /* display_was_removed */);
-}
-
 }  // namespace
 
 // static
@@ -151,7 +100,8 @@ ResolutionNotificationController::ResolutionChangeInfo::ResolutionChangeInfo(
 ResolutionNotificationController::ResolutionChangeInfo::
     ~ResolutionChangeInfo() = default;
 
-ResolutionNotificationController::ResolutionNotificationController() {
+ResolutionNotificationController::ResolutionNotificationController()
+    : weak_factory_(this) {
   Shell::Get()->window_tree_host_manager()->AddObserver(this);
   display::Screen::GetScreen()->AddObserver(this);
 }
@@ -212,6 +162,24 @@ bool ResolutionNotificationController::DoesNotificationTimeout() {
   return change_info_ && change_info_->timeout_count > 0;
 }
 
+void ResolutionNotificationController::Close(bool by_user) {
+  if (by_user)
+    AcceptResolutionChange(false);
+}
+
+void ResolutionNotificationController::Click() {
+  AcceptResolutionChange(true);
+}
+
+void ResolutionNotificationController::ButtonClick(int button_index) {
+  // If there's the timeout, the first button is "Accept". Otherwise the
+  // button click should be "Revert".
+  if (DoesNotificationTimeout() && button_index == 0)
+    AcceptResolutionChange(true);
+  else
+    RevertResolutionChange(false /* display_was_removed */);
+}
+
 void ResolutionNotificationController::CreateOrUpdateNotification(
     bool enable_spoken_feedback) {
   message_center::MessageCenter* message_center =
@@ -256,31 +224,20 @@ void ResolutionNotificationController::CreateOrUpdateNotification(
                     change_info_->current_resolution.size().ToString()));
 
   ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
-  std::unique_ptr<Notification> notification(new Notification(
+  auto notification = std::make_unique<Notification>(
       message_center::NOTIFICATION_TYPE_SIMPLE, kNotificationId, message,
       timeout_message, bundle.GetImageNamed(IDR_AURA_NOTIFICATION_DISPLAY),
       base::string16() /* display_source */, GURL(),
       message_center::NotifierId(message_center::NotifierId::SYSTEM_COMPONENT,
                                  kNotifierDisplayResolutionChange),
       data,
-      new ResolutionChangeNotificationDelegate(
-          this, change_info_->timeout_count > 0)));
+      base::MakeRefCounted<message_center::ThunkNotificationDelegate>(
+          weak_factory_.GetWeakPtr()));
   notification->set_clickable(true);
   notification->SetSystemPriority();
   message_center->AddNotification(std::move(notification));
 }
 
-void ResolutionNotificationController::OnTimerTick() {
-  if (!change_info_)
-    return;
-
-  --change_info_->timeout_count;
-  if (change_info_->timeout_count == 0)
-    RevertResolutionChange(false /* display_was_removed */);
-  else
-    CreateOrUpdateNotification(false);
-}
-
 void ResolutionNotificationController::AcceptResolutionChange(
     bool close_notification) {
   if (close_notification) {
@@ -317,6 +274,16 @@ void ResolutionNotificationController::RevertResolutionChange(
   }
 }
 
+void ResolutionNotificationController::OnTimerTick() {
+  if (!change_info_)
+    return;
+
+  if (--change_info_->timeout_count == 0)
+    RevertResolutionChange(false /* display_was_removed */);
+  else
+    CreateOrUpdateNotification(false);
+}
+
 void ResolutionNotificationController::OnDisplayAdded(
     const display::Display& new_display) {}
 

ash/display/resolution_notification_controller.h

@@ -12,9 +12,11 @@
 #include "base/callback.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/timer/timer.h"
 #include "ui/display/display_observer.h"
 #include "ui/gfx/geometry/size.h"
+#include "ui/message_center/public/cpp/notification_delegate.h"
 
 namespace chromeos {
 FORWARD_DECLARE_TEST(DisplayPrefsTest, PreventStore);
@@ -26,7 +28,8 @@ namespace ash {
 // also manages the timeout in case the new resolution is unusable.
 class ASH_EXPORT ResolutionNotificationController
     : public display::DisplayObserver,
-      public WindowTreeHostManager::Observer {
+      public WindowTreeHostManager::Observer,
+      public message_center::NotificationObserver {
  public:
   ResolutionNotificationController();
   ~ResolutionNotificationController() override;
@@ -64,14 +67,10 @@ class ASH_EXPORT ResolutionNotificationController
   // the notification times out.
   bool DoesNotificationTimeout();
 
-  // Called by the notification delegate when the user accepts the display
-  // resolution change. Set |close_notification| to true when the notification
-  // should be removed.
-  void AcceptResolutionChange(bool close_notification);
-
-  // Called by the notification delegate when the user wants to revert the
-  // display resolution change.
-  void RevertResolutionChange(bool display_was_removed);
+  // message_center::NotificationObserver
+  void Close(bool by_user) override;
+  void Click() override;
+  void ButtonClick(int button_index) override;
 
  private:
   friend class ResolutionNotificationControllerTest;
@@ -90,6 +89,13 @@ class ASH_EXPORT ResolutionNotificationController
   // feedback.
   void CreateOrUpdateNotification(bool enable_spoken_feedback);
 
+  // Called when the user accepts the display resolution change. Set
+  // |close_notification| to true when the notification should be removed.
+  void AcceptResolutionChange(bool close_notification);
+
+  // Called when the user wants to revert the display resolution change.
+  void RevertResolutionChange(bool display_was_removed);
+
   // Called every second for timeout.
   void OnTimerTick();
 
@@ -106,6 +112,8 @@ class ASH_EXPORT ResolutionNotificationController
 
   std::unique_ptr<ResolutionChangeInfo> change_info_;
 
+  base::WeakPtrFactory<ResolutionNotificationController> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ResolutionNotificationController);
 };