system: Modify add monitor notification.

Update text and change so exit mirror mode will not trigger it. Partial
revert of crrev.com/c/2340265 as we may keep around some of the
notifications that are currently behind the flag.

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

ash/ash_strings.grd

@@ -1456,6 +1456,9 @@ This file contains the strings for ash.
       <message name="IDS_ASH_STATUS_TRAY_DISPLAY_REMOVED" desc="The text of the notification to show when a display is disconnected from the device.">
         Removed display <ph name="DISPLAY_NAME">$1</ph>
       </message>
+      <message name="IDS_ASH_STATUS_TRAY_DISPLAY_ADDED" desc="The text of the notification to show that a display is connected to the device.">
+        <ph name="DISPLAY_NAME">$1</ph> connected
+      </message>
       <message name="IDS_ASH_STATUS_TRAY_DISPLAY_MIRRORING_NO_INTERNAL" desc="The label used in the tray to show that the current status is mirroring and the device doesn't have the internal display.">
         Mirroring
       </message>

ash/ash_strings_grd/IDS_ASH_STATUS_TRAY_DISPLAY_ADDED.png.sha1

+e601523381ac478f8b11e5da2a8e5617ad1b658e
\ No newline at end of file

ash/system/screen_layout_observer.cc

@@ -85,7 +85,7 @@ void OnNotificationClicked(base::Optional<int> button_index) {
         UMA_STATUS_AREA_DISPLAY_NOTIFICATION_SHOW_SETTINGS);
   }
   message_center::MessageCenter::Get()->RemoveNotification(
-      ScreenLayoutObserver::kNotificationId, true /* by_user */);
+      ScreenLayoutObserver::kNotificationId, /*by_user=*/true);
 }
 
 // Returns the name of the currently connected external display whose ID is
@@ -180,6 +180,12 @@ base::string16 GetDisplayRemovedMessage(
 
 base::string16 GetDisplayAddedMessage(int64_t added_display_id,
                                       base::string16* additional_message_out) {
+  if (features::IsReduceDisplayNotificationsEnabled()) {
+    DCHECK(!display::Display::IsInternalDisplayId(added_display_id));
+    return l10n_util::GetStringFUTF16(IDS_ASH_STATUS_TRAY_DISPLAY_ADDED,
+                                      GetExternalDisplayName(added_display_id));
+  }
+
   if (!display::Display::HasInternalDisplay()) {
     return l10n_util::GetStringUTF16(
         IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED_NO_INTERNAL);
@@ -195,12 +201,12 @@ const char ScreenLayoutObserver::kNotificationId[] =
     "chrome://settings/display";
 
 ScreenLayoutObserver::ScreenLayoutObserver() {
-  Shell::Get()->display_manager()->AddObserver(this);
+  Shell::Get()->window_tree_host_manager()->AddObserver(this);
   UpdateDisplayInfo(nullptr);
 }
 
 ScreenLayoutObserver::~ScreenLayoutObserver() {
-  Shell::Get()->display_manager()->RemoveObserver(this);
+  Shell::Get()->window_tree_host_manager()->RemoveObserver(this);
 }
 
 void ScreenLayoutObserver::SetDisplayChangedFromSettingsUI(int64_t display_id) {
@@ -283,6 +289,12 @@ bool ScreenLayoutObserver::GetDisplayMessageForNotification(
       if (old_info.count(iter.first))
         continue;
 
+      // No notification if the internal display is connected.
+      if (features::IsReduceDisplayNotificationsEnabled() &&
+          display::Display::IsInternalDisplayId(iter.first)) {
+        return false;
+      }
+
       *out_message = GetDisplayAddedMessage(iter.first, out_additional_message);
       return true;
     }
@@ -374,7 +386,7 @@ void ScreenLayoutObserver::CreateOrUpdateNotification(
   // Always remove the notification to make sure the notification appears
   // as a popup in any situation.
   message_center::MessageCenter::Get()->RemoveNotification(kNotificationId,
-                                                           false /* by_user */);
+                                                           /*by_user=*/false);
 
   if (message.empty() && additional_message.empty())
     return;
@@ -413,22 +425,7 @@ void ScreenLayoutObserver::CreateOrUpdateNotification(
       std::move(notification));
 }
 
-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() {
+void ScreenLayoutObserver::OnDisplayConfigurationChanged() {
   DisplayInfoMap old_info;
   UpdateDisplayInfo(&old_info);
 
@@ -467,13 +464,21 @@ void ScreenLayoutObserver::OnDidProcessDisplayChanges() {
   base::string16 additional_message;
   if (!GetDisplayMessageForNotification(old_info,
                                         should_notify_has_unassociated_display,
-                                        &message, &additional_message))
+                                        &message, &additional_message)) {
     return;
+  }
+
+  // Alerting user of display added and unassociated display are allowed even
+  // when suppressed.
+  const bool exited_mirror = old_display_mode_ == DisplayMode::MIRRORING &&
+                             current_display_mode_ != DisplayMode::MIRRORING;
+  const bool display_added =
+      !exited_mirror && old_info.size() < display_info_.size();
+  const bool allowed_notifications =
+      display_added || should_notify_has_unassociated_display;
 
   if (features::IsReduceDisplayNotificationsEnabled() &&
-      !should_notify_has_unassociated_display) {
-    // If display notifications should be suppressed and the notification is not
-    // to alert the user of an unassociated display, do not show a notification.
+      !allowed_notifications) {
     return;
   }
 

ash/system/screen_layout_observer.h

@@ -11,25 +11,24 @@
 #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 display::DisplayObserver {
+class ASH_EXPORT ScreenLayoutObserver : public WindowTreeHostManager::Observer {
  public:
   ScreenLayoutObserver();
   ~ScreenLayoutObserver() override;
 
   static const char kNotificationId[];
 
-  // display::DisplayObserver:
-  void OnDisplayAdded(const display::Display& new_display) override;
-  void OnDidProcessDisplayChanges() override;
+  // WindowTreeHostManager::Observer:
+  void OnDisplayConfigurationChanged() 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

@@ -353,11 +353,8 @@ 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_TRUE(IsNotificationShown());
-  CloseNotification();
+  EXPECT_FALSE(IsNotificationShown());
 
   // Simulate that device can support at most two displays and user connects
   // it with three displays. Because device is in tablet mode, display mode
@@ -425,10 +422,10 @@ TEST_F(ScreenLayoutObserverTest, DisplayConfigurationChangedTwice) {
                 IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED_NO_INTERNAL),
             GetDisplayNotificationText());
 
-  // OnDidProcessDisplayChanges() may be called more than once for a single
+  // OnDisplayConfigurationChanged() 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()->OnDidProcessDisplayChanges();
+  GetScreenLayoutObserver()->OnDisplayConfigurationChanged();
   EXPECT_EQ(l10n_util::GetStringUTF16(
                 IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED_NO_INTERNAL),
             GetDisplayNotificationText());

ash/system/unified/unified_system_tray_bubble.cc

@@ -111,7 +111,6 @@ 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);
 }
@@ -120,14 +119,13 @@ 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(!views::WidgetObserver::IsInObserverList());
+  CHECK(!IsInObserverList());
 }
 
 gfx::Rect UnifiedSystemTrayBubble::GetBoundsInScreen() const {
@@ -263,7 +261,7 @@ void UnifiedSystemTrayBubble::OnMessageCenterActivated() {
   bubble_view_->StopReroutingEvents();
 }
 
-void UnifiedSystemTrayBubble::OnDidProcessDisplayChanges() {
+void UnifiedSystemTrayBubble::OnDisplayConfigurationChanged() {
   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 display::DisplayObserver,
+      public ScreenLayoutObserver,
       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;
 
-  // display::DisplayObserver:
-  void OnDidProcessDisplayChanges() override;
+  // ScreenLayoutObserver:
+  void OnDisplayConfigurationChanged() override;
 
   // views::WidgetObserver:
   void OnWidgetDestroying(views::Widget* widget) override;