system: Add a notification to shown when display is added.

This is UI review request to replace the previous open display settings
on context menu. This adds a notification that disappears after 6s
(notifications API default). The notification is shown when a display is
added, all other display related notifications remain hidden if the flag
is off.

Test: manual
Bug: 1111479
Change-Id: Ib3cf08f5a3a6a129a3457605e0b5aefc21063dbc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2340265
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795663}

ash/system/screen_layout_observer.cc

@@ -33,6 +33,7 @@
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/public/cpp/notification.h"
 #include "ui/message_center/public/cpp/notification_delegate.h"
+#include "ui/message_center/public/cpp/notification_types.h"
 #include "ui/strings/grit/ui_strings.h"
 
 using message_center::Notification;
@@ -194,12 +195,12 @@ const char ScreenLayoutObserver::kNotificationId[] =
     "chrome://settings/display";
 
 ScreenLayoutObserver::ScreenLayoutObserver() {
-  Shell::Get()->window_tree_host_manager()->AddObserver(this);
+  Shell::Get()->display_manager()->AddObserver(this);
   UpdateDisplayInfo(nullptr);
 }
 
 ScreenLayoutObserver::~ScreenLayoutObserver() {
-  Shell::Get()->window_tree_host_manager()->RemoveObserver(this);
+  Shell::Get()->display_manager()->RemoveObserver(this);
 }
 
 void ScreenLayoutObserver::SetDisplayChangedFromSettingsUI(int64_t display_id) {
@@ -398,7 +399,13 @@ void ScreenLayoutObserver::CreateOrUpdateNotification(
           base::BindRepeating(&OnNotificationClicked)),
       kNotificationScreenIcon,
       message_center::SystemNotificationWarningLevel::NORMAL);
-  notification->set_priority(message_center::SYSTEM_PRIORITY);
+  // With the reduce display notifications feature enabled, we only want to show
+  // an add display notification, and it should disappear after a couple
+  // seconds.
+  auto priority = features::IsReduceDisplayNotificationsEnabled()
+                      ? message_center::DEFAULT_PRIORITY
+                      : message_center::SYSTEM_PRIORITY;
+  notification->set_priority(priority);
 
   Shell::Get()->metrics()->RecordUserMetricsAction(
       UMA_STATUS_AREA_DISPLAY_NOTIFICATION_CREATED);
@@ -406,7 +413,22 @@ void ScreenLayoutObserver::CreateOrUpdateNotification(
       std::move(notification));
 }
 
-void ScreenLayoutObserver::OnDisplayConfigurationChanged() {
+void ScreenLayoutObserver::OnDisplayAdded(const display::Display& new_display) {
+  if (!show_notifications_for_testing_)
+    return;
+
+  // The older way handles showing display added notifications as well. We will
+  // use that if display notifications are not suppressed.
+  if (!features::IsReduceDisplayNotificationsEnabled())
+    return;
+
+  base::string16 additional_message;
+  base::string16 message =
+      GetDisplayAddedMessage(new_display.id(), &additional_message);
+  CreateOrUpdateNotification(message, additional_message);
+}
+
+void ScreenLayoutObserver::OnDidProcessDisplayChanges() {
   DisplayInfoMap old_info;
   UpdateDisplayInfo(&old_info);
 

ash/system/screen_layout_observer.h

@@ -11,24 +11,25 @@
 #include <set>
 
 #include "ash/ash_export.h"
-#include "ash/display/window_tree_host_manager.h"
 #include "base/macros.h"
 #include "base/strings/string16.h"
+#include "ui/display/display_observer.h"
 #include "ui/display/manager/managed_display_info.h"
 
 namespace ash {
 
 // ScreenLayoutObserver is responsible to send notification to users when screen
 // resolution changes or screen rotation changes.
-class ASH_EXPORT ScreenLayoutObserver : public WindowTreeHostManager::Observer {
+class ASH_EXPORT ScreenLayoutObserver : public display::DisplayObserver {
  public:
   ScreenLayoutObserver();
   ~ScreenLayoutObserver() override;
 
   static const char kNotificationId[];
 
-  // WindowTreeHostManager::Observer:
-  void OnDisplayConfigurationChanged() override;
+  // display::DisplayObserver:
+  void OnDisplayAdded(const display::Display& new_display) override;
+  void OnDidProcessDisplayChanges() override;
 
   // No notification will be shown only for the next ui scale change for the
   // display with |display_id|. This state will be consumed and subsequent

ash/system/screen_layout_observer_unittest.cc

@@ -37,6 +37,9 @@ class ScreenLayoutObserverTest : public AshTestBase {
   ScreenLayoutObserverTest();
   ~ScreenLayoutObserverTest() override;
 
+  // AshTestBase:
+  void SetUp() override;
+
  protected:
   ScreenLayoutObserver* GetScreenLayoutObserver();
   void CheckUpdate();
@@ -45,6 +48,7 @@ class ScreenLayoutObserverTest : public AshTestBase {
   void ClickNotification();
   base::string16 GetDisplayNotificationText() const;
   base::string16 GetDisplayNotificationAdditionalText() const;
+  int GetDisplayNotificationPriority() const;
 
   base::string16 GetFirstDisplayName();
 
@@ -71,6 +75,11 @@ ScreenLayoutObserverTest::ScreenLayoutObserverTest() {
 
 ScreenLayoutObserverTest::~ScreenLayoutObserverTest() = default;
 
+void ScreenLayoutObserverTest::SetUp() {
+  AshTestBase::SetUp();
+  GetScreenLayoutObserver()->set_show_notifications_for_testing(true);
+}
+
 ScreenLayoutObserver* ScreenLayoutObserverTest::GetScreenLayoutObserver() {
   return Shell::Get()->screen_layout_observer();
 }
@@ -97,6 +106,12 @@ base::string16 ScreenLayoutObserverTest::GetDisplayNotificationAdditionalText()
   return notification ? notification->message() : base::string16();
 }
 
+int ScreenLayoutObserverTest::GetDisplayNotificationPriority() const {
+  constexpr int kInvalidPriority = 71;
+  const message_center::Notification* notification = GetDisplayNotification();
+  return notification ? notification->priority() : kInvalidPriority;
+}
+
 base::string16 ScreenLayoutObserverTest::GetFirstDisplayName() {
   return base::UTF8ToUTF16(display_manager()->GetDisplayNameForId(
       display_manager()->first_display_id()));
@@ -144,9 +159,6 @@ ScreenLayoutObserverTest::GetDisplayNotification() const {
 }
 
 TEST_F(ScreenLayoutObserverTest, DisplayNotifications) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
-
   UpdateDisplay("400x400");
   display::Display::SetInternalDisplayId(display_manager()->first_display_id());
   EXPECT_TRUE(GetDisplayNotificationText().empty());
@@ -294,8 +306,6 @@ TEST_F(ScreenLayoutObserverTest, DisplayNotificationsDisabled) {
   scoped_feature_list_.Reset();
   scoped_feature_list_.InitAndEnableFeature(
       features::kReduceDisplayNotifications);
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
 
   UpdateDisplay("400x400");
   display::Display::SetInternalDisplayId(display_manager()->first_display_id());
@@ -305,9 +315,11 @@ TEST_F(ScreenLayoutObserverTest, DisplayNotificationsDisabled) {
   UpdateDisplay("400x400/r");
   EXPECT_FALSE(IsNotificationShown());
 
-  // Extended.
+  // Adding a display still shows a notification.
   UpdateDisplay("400x400,200x200");
-  EXPECT_FALSE(IsNotificationShown());
+  EXPECT_TRUE(IsNotificationShown());
+  EXPECT_EQ(message_center::DEFAULT_PRIORITY, GetDisplayNotificationPriority());
+  CloseNotification();
 
   const int64_t first_display_id =
       display::Screen::GetScreen()->GetPrimaryDisplay().id();
@@ -341,8 +353,11 @@ TEST_F(ScreenLayoutObserverTest, DisplayNotificationsDisabled) {
   base::RunLoop().RunUntilIdle();
 
   // Exit mirror mode manually. Now display mode should be extending mode.
+  // Exiting mirror mode counts as adding a display, which will show a
+  // notification.
   display_manager()->SetMirrorMode(display::MirrorMode::kOff, base::nullopt);
-  EXPECT_FALSE(IsNotificationShown());
+  EXPECT_TRUE(IsNotificationShown());
+  CloseNotification();
 
   // Simulate that device can support at most two displays and user connects
   // it with three displays. Because device is in tablet mode, display mode
@@ -363,8 +378,6 @@ TEST_F(ScreenLayoutObserverTest, DisplayNotificationsDisabled) {
 // in the UI scales, in which case, we still want to show a notification when
 // the source of change is not the settings ui.
 TEST_F(ScreenLayoutObserverTest, ZoomingInUnifiedModeNotification) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
   UpdateDisplay("400x400,200x200");
 
   // Enter unified mode.
@@ -407,17 +420,15 @@ TEST_F(ScreenLayoutObserverTest, ZoomingInUnifiedModeNotification) {
 // Verify that notification shows up when display is switched from dock mode to
 // extend mode.
 TEST_F(ScreenLayoutObserverTest, DisplayConfigurationChangedTwice) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
   UpdateDisplay("400x400,200x200");
   EXPECT_EQ(l10n_util::GetStringUTF16(
                 IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED_NO_INTERNAL),
             GetDisplayNotificationText());
 
-  // OnDisplayConfigurationChanged() may be called more than once for a single
+  // OnDidProcessDisplayChanges() may be called more than once for a single
   // update display in case of primary is swapped or recovered from dock mode.
   // Should not remove the notification in such case.
-  GetScreenLayoutObserver()->OnDisplayConfigurationChanged();
+  GetScreenLayoutObserver()->OnDidProcessDisplayChanges();
   EXPECT_EQ(l10n_util::GetStringUTF16(
                 IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED_NO_INTERNAL),
             GetDisplayNotificationText());
@@ -436,9 +447,6 @@ TEST_F(ScreenLayoutObserverTest, DisplayConfigurationChangedTwice) {
 TEST_F(ScreenLayoutObserverTest, UpdateAfterSuppressDisplayNotification) {
   UpdateDisplay("400x400,200x200");
 
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
-
   // Rotate the second.
   UpdateDisplay("400x400,200x200/r");
   EXPECT_EQ(l10n_util::GetStringFUTF16(
@@ -451,8 +459,8 @@ TEST_F(ScreenLayoutObserverTest, UpdateAfterSuppressDisplayNotification) {
 // Verify that no notification is shown when overscan of a screen is changed.
 TEST_F(ScreenLayoutObserverTest, OverscanDisplay) {
   UpdateDisplay("400x400, 300x300");
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
+  // Close the notification that is shown from initially adding a monitor.
+  CloseNotification();
   display::Display::SetInternalDisplayId(display_manager()->first_display_id());
 
   // /o creates the default overscan.
@@ -471,8 +479,6 @@ TEST_F(ScreenLayoutObserverTest, OverscanDisplay) {
 // Tests that exiting mirror mode by closing the lid shows the correct "exiting
 // mirror mode" message.
 TEST_F(ScreenLayoutObserverTest, ExitMirrorModeBecauseOfDockedModeMessage) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
   UpdateDisplay("400x400,200x200");
   display::Display::SetInternalDisplayId(
       display::test::DisplayManagerTestApi(display_manager())
@@ -498,8 +504,6 @@ TEST_F(ScreenLayoutObserverTest, ExitMirrorModeBecauseOfDockedModeMessage) {
 // correct message.
 TEST_F(ScreenLayoutObserverTest,
        ExitMirrorModeNoInternalDisplayBecauseOfDisplayRemovedMessage) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
   UpdateDisplay("400x400,200x200");
   display::Display::SetInternalDisplayId(
       display::test::DisplayManagerTestApi(display_manager())
@@ -523,8 +527,6 @@ TEST_F(ScreenLayoutObserverTest,
 // Tests notification messages shown when adding and removing displays in
 // extended mode.
 TEST_F(ScreenLayoutObserverTest, AddingRemovingDisplayExtendedModeMessage) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
   UpdateDisplay("400x400");
   EXPECT_TRUE(GetDisplayNotificationText().empty());
 
@@ -547,8 +549,6 @@ TEST_F(ScreenLayoutObserverTest, AddingRemovingDisplayExtendedModeMessage) {
 // Tests notification messages shown when entering and exiting unified desktop
 // mode.
 TEST_F(ScreenLayoutObserverTest, EnteringExitingUnifiedModeMessage) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
   UpdateDisplay("400x400");
   EXPECT_TRUE(GetDisplayNotificationText().empty());
 
@@ -587,8 +587,6 @@ TEST_F(ScreenLayoutObserverTest, EnteringExitingUnifiedModeMessage) {
 // Special case: Tests notification messages shown when entering docked mode
 // by closing the lid and the internal display is the secondary display.
 TEST_F(ScreenLayoutObserverTest, DockedModeWithExternalPrimaryDisplayMessage) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
   UpdateDisplay("400x400,200x200");
   EXPECT_EQ(l10n_util::GetStringUTF16(
                 IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED_NO_INTERNAL),
@@ -612,8 +610,6 @@ TEST_F(ScreenLayoutObserverTest, DockedModeWithExternalPrimaryDisplayMessage) {
 // Tests that rotation notifications are only shown when the rotation source is
 // a user action. The accelerometer source nevber produces any notifications.
 TEST_F(ScreenLayoutObserverTest, RotationNotification) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
   UpdateDisplay("400x400");
   const int64_t primary_id =
       display_manager()->GetPrimaryDisplayCandidate().id();
@@ -671,9 +667,6 @@ TEST_F(ScreenLayoutObserverTest, RotationNotification) {
 }
 
 TEST_F(ScreenLayoutObserverTest, MirrorModeAddOrRemoveDisplayMessage) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
-
   const int64_t internal_display_id =
       display::test::DisplayManagerTestApi(display_manager())
           .SetFirstDisplayAsInternalDisplay();
@@ -740,9 +733,6 @@ TEST_F(ScreenLayoutObserverTest, MirrorModeAddOrRemoveDisplayMessage) {
 }
 
 TEST_F(ScreenLayoutObserverTest, ClickNotification) {
-  Shell::Get()->screen_layout_observer()->set_show_notifications_for_testing(
-      true);
-
   // Create notification.
   UpdateDisplay("400x400/r");
   EXPECT_FALSE(GetDisplayNotificationAdditionalText().empty());

ash/system/unified/unified_system_tray_bubble.cc

@@ -111,6 +111,7 @@ UnifiedSystemTrayBubble::UnifiedSystemTrayBubble(UnifiedSystemTray* tray,
 
   tray->tray_event_filter()->AddBubble(this);
   tray->shelf()->AddObserver(this);
+  Shell::Get()->display_manager()->AddObserver(this);
   Shell::Get()->tablet_mode_controller()->AddObserver(this);
   Shell::Get()->activation_client()->AddObserver(this);
 }
@@ -119,13 +120,14 @@ UnifiedSystemTrayBubble::~UnifiedSystemTrayBubble() {
   Shell::Get()->activation_client()->RemoveObserver(this);
   if (Shell::Get()->tablet_mode_controller())
     Shell::Get()->tablet_mode_controller()->RemoveObserver(this);
+  Shell::Get()->display_manager()->RemoveObserver(this);
   tray_->tray_event_filter()->RemoveBubble(this);
   tray_->shelf()->RemoveObserver(this);
   if (bubble_widget_) {
     bubble_widget_->RemoveObserver(this);
     bubble_widget_->Close();
   }
-  CHECK(!IsInObserverList());
+  CHECK(!views::WidgetObserver::IsInObserverList());
 }
 
 gfx::Rect UnifiedSystemTrayBubble::GetBoundsInScreen() const {
@@ -261,7 +263,7 @@ void UnifiedSystemTrayBubble::OnMessageCenterActivated() {
   bubble_view_->StopReroutingEvents();
 }
 
-void UnifiedSystemTrayBubble::OnDisplayConfigurationChanged() {
+void UnifiedSystemTrayBubble::OnDidProcessDisplayChanges() {
   UpdateBubbleBounds();
 }
 

ash/system/unified/unified_system_tray_bubble.h

@@ -9,12 +9,12 @@
 
 #include "ash/public/cpp/tablet_mode_observer.h"
 #include "ash/shelf/shelf_observer.h"
-#include "ash/system/screen_layout_observer.h"
 #include "ash/system/tray/time_to_click_recorder.h"
 #include "ash/system/tray/tray_bubble_base.h"
 #include "base/macros.h"
 #include "base/optional.h"
 #include "base/time/time.h"
+#include "ui/display/display_observer.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/views/widget/widget_observer.h"
 #include "ui/wm/public/activation_change_observer.h"
@@ -35,7 +35,7 @@ class UnifiedSystemTrayView;
 // case, this class calls UnifiedSystemTray::CloseBubble() to delete itself.
 class ASH_EXPORT UnifiedSystemTrayBubble
     : public TrayBubbleBase,
-      public ScreenLayoutObserver,
+      public display::DisplayObserver,
       public views::WidgetObserver,
       public ShelfObserver,
       public ::wm::ActivationChangeObserver,
@@ -104,8 +104,8 @@ class ASH_EXPORT UnifiedSystemTrayBubble
   TrayBubbleView* GetBubbleView() const override;
   views::Widget* GetBubbleWidget() const override;
 
-  // ash::ScreenLayoutObserver:
-  void OnDisplayConfigurationChanged() override;
+  // display::DisplayObserver:
+  void OnDidProcessDisplayChanges() override;
 
   // views::WidgetObserver:
   void OnWidgetDestroying(views::Widget* widget) override;