Updates the display message in the uber tray.

ash/system/chromeos/tray_display.cc

Index: ash/system/chromeos/tray_display.cc
diff --git a/ash/system/chromeos/tray_display.cc b/ash/system/chromeos/tray_display.cc
index cf3ca84b80f8b5e85030f65b7962728daeb3c9c3..71ec5651ce972ec8081518dee8e6a0e857373861 100644
--- a/ash/system/chromeos/tray_display.cc
+++ b/ash/system/chromeos/tray_display.cc
@@ -24,26 +24,34 @@ namespace ash {
 namespace internal {
 namespace {
 
-TrayDisplayMode GetCurrentTrayDisplayMode() {
-  DisplayManager* display_manager = Shell::GetInstance()->display_manager();
-  if (display_manager->GetNumDisplays() > 1)
-    return TRAY_DISPLAY_EXTENDED;
+bool display_notifications_enabled = true;

[stevenjb, 2013/06/21 01:08:35]

This should really be 'display_notifications_disabled' with the default set to
false. The test setter should also be renamed appropriately.

[Jun Mukai, 2013/06/21 04:48:50]

Done.

 
-  if (display_manager->IsMirrored())
-    return TRAY_DISPLAY_MIRRORED;
+DisplayManager* GetDisplayManager() {
+  return Shell::GetInstance()->display_manager();
+}
 
-  int64 first_id = display_manager->first_display_id();
-  if (display_manager->HasInternalDisplay() &&
-      !display_manager->IsInternalDisplayId(first_id)) {
-    return TRAY_DISPLAY_DOCKED;
-  }
+base::string16 GetDisplayName(int64 display_id) {
+  return UTF8ToUTF16(GetDisplayManager()->GetDisplayNameForId(display_id));
+}
+
+base::string16 GetDisplaySize(int64 display_id) {
+  return UTF8ToUTF16(
+      GetDisplayManager()->GetDisplayForId(display_id).size().ToString());
+}
 
-  return TRAY_DISPLAY_SINGLE;
+bool ShouldShowResolution(int64 display_id) {
+  if (!GetDisplayManager()->GetDisplayForId(display_id).is_valid())
+    return false;
+
+  const DisplayInfo& display_info =
+      GetDisplayManager()->GetDisplayInfo(display_id);
+  return display_info.rotation() != gfx::Display::ROTATE_0 ||
+      display_info.ui_scale() != 1.0f;
 }
 
 // Returns the name of the currently connected external display.
 base::string16 GetExternalDisplayName() {
-  DisplayManager* display_manager = Shell::GetInstance()->display_manager();
+  DisplayManager* display_manager = GetDisplayManager();
   int64 external_id = display_manager->mirrored_display().id();
 
   if (external_id == gfx::Display::kInvalidDisplayID) {
@@ -56,155 +64,164 @@ base::string16 GetExternalDisplayName() {
       }
     }
   }
-  if (external_id != gfx::Display::kInvalidDisplayID)
-    return UTF8ToUTF16(display_manager->GetDisplayNameForId(external_id));
-  return l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_UNKNOWN_DISPLAY_NAME);
-}
 
-class DisplayViewBase {
- public:
-  DisplayViewBase(user::LoginStatus login_status)
-      : login_status_(login_status) {
-    label_ = new views::Label();
-    label_->SetMultiLine(true);
-    label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  }
+  if (external_id == gfx::Display::kInvalidDisplayID)
+    return l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_UNKNOWN_DISPLAY_NAME);
 
-  virtual ~DisplayViewBase() {
-  }
+  // The external display name may have an annotation of "(width x height)" in
+  // case that the display is rotated or its resolution is changed.
+  base::string16 name = GetDisplayName(external_id);
+  if (ShouldShowResolution(external_id))
+    name += UTF8ToUTF16(" (") + GetDisplaySize(external_id) + UTF8ToUTF16(")");
+
+  return name;
+}
 
- protected:
-  void OpenSettings() {
-    if (login_status_ == ash::user::LOGGED_IN_USER ||
-        login_status_ == ash::user::LOGGED_IN_OWNER ||
-        login_status_ == ash::user::LOGGED_IN_GUEST) {
-      ash::Shell::GetInstance()->system_tray_delegate()->ShowDisplaySettings();
+base::string16 GetTrayDisplayMessage() {
+  DisplayManager* display_manager = GetDisplayManager();
+  if (display_manager->GetNumDisplays() > 1) {
+    if (GetDisplayManager()->HasInternalDisplay()) {
+      return l10n_util::GetStringFUTF16(
+          IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED, GetExternalDisplayName());
     }
+    return l10n_util::GetStringUTF16(
+        IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED_NO_INTERNAL);
   }
 
-  bool UpdateLabelText() {
-    switch (GetCurrentTrayDisplayMode()) {
-      case TRAY_DISPLAY_SINGLE:
-        // TODO(oshima|mukai): Support single display mode for overscan
-        // alignment.
-        return false;
-      case TRAY_DISPLAY_EXTENDED:
-        if (Shell::GetInstance()->display_manager()->HasInternalDisplay()) {
-          label_->SetText(l10n_util::GetStringFUTF16(
-              IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED, GetExternalDisplayName()));
-        } else {
-          label_->SetText(l10n_util::GetStringUTF16(
-              IDS_ASH_STATUS_TRAY_DISPLAY_EXTENDED_NO_INTERNAL));
-        }
-        break;
-      case TRAY_DISPLAY_MIRRORED:
-        if (Shell::GetInstance()->display_manager()->HasInternalDisplay()) {
-          label_->SetText(l10n_util::GetStringFUTF16(
-              IDS_ASH_STATUS_TRAY_DISPLAY_MIRRORING, GetExternalDisplayName()));
-        } else {
-          label_->SetText(l10n_util::GetStringUTF16(
-              IDS_ASH_STATUS_TRAY_DISPLAY_MIRRORING_NO_INTERNAL));
-        }
-        break;
-      case TRAY_DISPLAY_DOCKED:
-        label_->SetText(l10n_util::GetStringUTF16(
-            IDS_ASH_STATUS_TRAY_DISPLAY_DOCKED));
-        break;
+  if (display_manager->IsMirrored()) {
+    if (GetDisplayManager()->HasInternalDisplay()) {
+      return l10n_util::GetStringFUTF16(
+          IDS_ASH_STATUS_TRAY_DISPLAY_MIRRORING, GetExternalDisplayName());
     }
-    return true;
+    return l10n_util::GetStringUTF16(
+        IDS_ASH_STATUS_TRAY_DISPLAY_MIRRORING_NO_INTERNAL);
   }
 
-  views::Label* label() { return label_; }
+  int64 first_id = display_manager->first_display_id();
+  if (display_manager->HasInternalDisplay() &&
+      !display_manager->IsInternalDisplayId(first_id)) {
+    return l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_DISPLAY_DOCKED);
+  }
 
- private:
-  user::LoginStatus login_status_;
-  views::Label* label_;
+  return base::string16();
+}
 
-  DISALLOW_COPY_AND_ASSIGN(DisplayViewBase);
-};
+void OpenSettings(user::LoginStatus login_status) {
+  if (login_status == ash::user::LOGGED_IN_USER ||
+      login_status == ash::user::LOGGED_IN_OWNER ||
+      login_status == ash::user::LOGGED_IN_GUEST) {
+    ash::Shell::GetInstance()->system_tray_delegate()->ShowDisplaySettings();
+  }
+}
 
 }  // namespace
 
-class DisplayView : public DisplayViewBase,
-                    public ash::internal::ActionableView {
- public:
-  explicit DisplayView(user::LoginStatus login_status)
-      : DisplayViewBase(login_status) {
-    SetLayoutManager(new
-        views::BoxLayout(views::BoxLayout::kHorizontal,
-        ash::kTrayPopupPaddingHorizontal, 0,
-        ash::kTrayPopupPaddingBetweenItems));
-
-    ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
-    image_ =
-        new ash::internal::FixedSizedImageView(0, ash::kTrayPopupItemHeight);
-    image_->SetImage(
-        bundle.GetImageNamed(IDR_AURA_UBER_TRAY_DISPLAY).ToImageSkia());
-    AddChildView(image_);
-    AddChildView(label());
-    Update();
-  }
+DisplayView::DisplayView(user::LoginStatus login_status)
+    : login_status_(login_status) {
+  SetLayoutManager(new views::BoxLayout(
+      views::BoxLayout::kHorizontal,
+      ash::kTrayPopupPaddingHorizontal, 0,
+      ash::kTrayPopupPaddingBetweenItems));
+
+  ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+  image_ = new ash::internal::FixedSizedImageView(0, ash::kTrayPopupItemHeight);
+  image_->SetImage(
+      bundle.GetImageNamed(IDR_AURA_UBER_TRAY_DISPLAY).ToImageSkia());
+  AddChildView(image_);
+
+  label_ = new views::Label();
+  label_->SetMultiLine(true);
+  label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+  AddChildView(label_);
+  Update();
+}
 
-  virtual ~DisplayView() {}
+DisplayView::~DisplayView() {
+}
 
-  void Update() {
-    SetVisible(UpdateLabelText());
-  }
+void DisplayView::Update() {
+  base::string16 message = GetTrayDisplayMessage();
+  if (message.empty())
+    message = GetInternalDisplayInfo();
+  SetVisible(!message.empty());
+  label_->SetText(message);
+}
 
- private:
-  // Overridden from ActionableView.
-  virtual bool PerformAction(const ui::Event& event) OVERRIDE {
-    OpenSettings();
-    return true;
-  }
+bool DisplayView::GetTooltipText(const gfx::Point& p,
+                                 string16* tooltip) const {
+  base::string16 tray_message = GetTrayDisplayMessage();
+  base::string16 internal_message = GetInternalDisplayInfo();
+  if (tray_message.empty() && internal_message.empty())
+    return false;
 
-  virtual void OnBoundsChanged(const gfx::Rect& previous_bounds) OVERRIDE {
-    int label_max_width = bounds().width() - kTrayPopupPaddingHorizontal * 2 -
-        kTrayPopupPaddingBetweenItems - image_->GetPreferredSize().width();
-    label()->SizeToFit(label_max_width);
-    PreferredSizeChanged();
-  }
+  *tooltip = tray_message + ASCIIToUTF16("\n") + internal_message;
+  return true;
+}
 
-  views::ImageView* image_;
+base::string16 DisplayView::GetInternalDisplayInfo() const {
+  int64 first_id = GetDisplayManager()->first_display_id();
+  if (!ShouldShowResolution(first_id))
+    return base::string16();
 
-  DISALLOW_COPY_AND_ASSIGN(DisplayView);
-};
+  return l10n_util::GetStringFUTF16(
+      IDS_ASH_STATUS_TRAY_DISPLAY_SINGLE_DISPLAY,
+      GetDisplayName(first_id),
+      GetDisplaySize(first_id));
+}
 
-class DisplayNotificationView : public DisplayViewBase,
-                                public TrayNotificationView {
+bool DisplayView::PerformAction(const ui::Event& event) {
+  OpenSettings(login_status_);
+  return true;
+}
+
+void DisplayView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
+  int label_max_width = bounds().width() - kTrayPopupPaddingHorizontal * 2 -
+      kTrayPopupPaddingBetweenItems - image_->GetPreferredSize().width();
+  label_->SizeToFit(label_max_width);
+  PreferredSizeChanged();
+}
+
+class DisplayNotificationView : public TrayNotificationView {
  public:
   DisplayNotificationView(user::LoginStatus login_status,
-                          TrayDisplay* tray_item)
-      : DisplayViewBase(login_status),
-        TrayNotificationView(tray_item, IDR_AURA_UBER_TRAY_DISPLAY) {
-    InitView(label());
+                          TrayDisplay* tray_item,
+                          const base::string16& message)
+      : TrayNotificationView(tray_item, IDR_AURA_UBER_TRAY_DISPLAY),
+        login_status_(login_status) {
     StartAutoCloseTimer(kTrayPopupAutoCloseDelayForTextInSeconds);
-    Update();
+    Update(message);
   }
 
   virtual ~DisplayNotificationView() {}
 
-  void Update() {
-    if (UpdateLabelText())
-      RestartAutoCloseTimer();
-    else
+  void Update(const base::string16& message) {
+    if (message.empty()) {
       owner()->HideNotificationView();
+    } else {
+      views::Label* label = new views::Label(message);
+      label->SetMultiLine(true);
+      label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+      UpdateView(label);
+      RestartAutoCloseTimer();
+    }
   }
 
   // Overridden from TrayNotificationView:
   virtual void OnClickAction() OVERRIDE {
-    OpenSettings();
+    OpenSettings(login_status_);
   }
 
  private:
+  user::LoginStatus login_status_;
+
   DISALLOW_COPY_AND_ASSIGN(DisplayNotificationView);
 };
 
 TrayDisplay::TrayDisplay(SystemTray* system_tray)
     : SystemTrayItem(system_tray),
       default_(NULL),
-      notification_(NULL),
-      current_mode_(GetCurrentTrayDisplayMode()) {
+      notification_(NULL) {
+  current_message_ = GetDisplayMessageForNotification();
   Shell::GetInstance()->display_controller()->AddObserver(this);
 }
 
@@ -212,6 +229,39 @@ TrayDisplay::~TrayDisplay() {
   Shell::GetInstance()->display_controller()->RemoveObserver(this);
 }
 
+base::string16 TrayDisplay::GetDisplayMessageForNotification() {
+  DisplayManager* display_manager = GetDisplayManager();
+  std::map<int64, DisplayInfo> old_info;
+  old_info.swap(display_info_);
+  for (size_t i = 0; i < display_manager->GetNumDisplays(); ++i) {
+    int64 id = display_manager->GetDisplayAt(i)->id();
+    display_info_[id] = display_manager->GetDisplayInfo(id);
+  }
+
+  if (display_info_.size() == old_info.size()) {

[stevenjb, 2013/06/21 01:08:35]

I would make this an early exit with a comment; the logic here is kind of
confusing and less nesting would help a bit.

[Jun Mukai, 2013/06/21 04:48:50]

Done.

+    for (std::map<int64, DisplayInfo>::const_iterator iter =
+             display_info_.begin(); iter != display_info_.end(); ++iter) {
+      std::map<int64, DisplayInfo>::const_iterator old_iter =
+          old_info.find(iter->first);
+      if (old_iter == old_info.end())
+        break;

[stevenjb, 2013/06/21 01:08:35]

I'm not sure I follow this logic. Do we really want to break the first time an
entry in display_info_ is not in old_info? If so, can you add a comment
explaining the logic?

[Jun Mukai, 2013/06/21 04:48:50]

Done.

[stevenjb, 2013/06/21 20:17:51]

Much clearer now, thanks.

+
+      if (iter->second.ui_scale() != old_iter->second.ui_scale()) {
+        return l10n_util::GetStringFUTF16(
+            IDS_ASH_STATUS_TRAY_DISPLAY_RESOLUTION_CHANGED,
+            GetDisplayName(iter->first),
+            GetDisplaySize(iter->first));
+      }
+      if (iter->second.rotation() != old_iter->second.rotation()) {
+        return l10n_util::GetStringFUTF16(
+            IDS_ASH_STATUS_TRAY_DISPLAY_ROTATED, GetDisplayName(iter->first));
+      }
+    }
+  }
+
+  return GetTrayDisplayMessage();
+}
+
 views::View* TrayDisplay::CreateDefaultView(user::LoginStatus status) {
   DCHECK(default_ == NULL);
   default_ = new DisplayView(status);
@@ -220,7 +270,7 @@ views::View* TrayDisplay::CreateDefaultView(user::LoginStatus status) {
 
 views::View* TrayDisplay::CreateNotificationView(user::LoginStatus status) {
   DCHECK(notification_ == NULL);
-  notification_ = new DisplayNotificationView(status, this);
+  notification_ = new DisplayNotificationView(status, this, current_message_);
   return notification_;
 }
 
@@ -237,14 +287,21 @@ bool TrayDisplay::ShouldShowLauncher() const {
 }
 
 void TrayDisplay::OnDisplayConfigurationChanged() {
-  TrayDisplayMode new_mode = GetCurrentTrayDisplayMode();
-  if (current_mode_ != new_mode && new_mode != TRAY_DISPLAY_SINGLE) {
-    if (notification_)
-      notification_->Update();
-    else
-      ShowNotificationView();
-  }
-  current_mode_ = new_mode;
+  if (!display_notifications_enabled)
+    return;
+
+  // TODO(mukai): do not show the notification when the configuration changed
+  // due to the user operation on display settings page.
+  current_message_ = GetDisplayMessageForNotification();
+  if (notification_)
+    notification_->Update(current_message_);
+  else if (!current_message_.empty())
+    ShowNotificationView();
+}
+
+// static
+void TrayDisplay::SetDisplayNotificationsEnabledForTest(bool enabled) {

[stevenjb, 2013/06/21 01:08:35]

I would make this 'DisableDisplayNotificationsForTest()' unless we actually need
to toggle back and forth.

[Jun Mukai, 2013/06/21 04:48:50]

it actually needs to be toggled.  AshTestBase does the disable by default, but
only TrayDisplayTest wants to re-enable it.

+  display_notifications_enabled = enabled;
 }
 
 }  // namespace internal