Caches the display names in MultiDisplayManager.

ash/display/multi_display_manager.cc

Index: ash/display/multi_display_manager.cc
diff --git a/ash/display/multi_display_manager.cc b/ash/display/multi_display_manager.cc
index 246e0b844c0b2f79da0f93d02772126ff3e92422..27c6654a17f9cb40db1546c76115c66466bfa60c 100644
--- a/ash/display/multi_display_manager.cc
+++ b/ash/display/multi_display_manager.cc
@@ -129,6 +129,17 @@ bool MultiDisplayManager::UpdateWorkAreaOfDisplayNearestWindow(
   return old_work_area != display.work_area();
 }
 
+std::vector<std::string> MultiDisplayManager::GetExternalDisplayNames() const {

[Daniel Erat, 2012/11/02 16:46:17]

this method seems a bit tricky; it returns an ordered list of names but skips
over the internal display.  is there any way that you could change the caller so
it just passes the external display that it cares about to GetDisplayNameFor()
instead?

[Jun Mukai, 2012/11/02 18:04:35]

Thanks for the comment.  Removed this method and changed the code structure a
bit.

+  std::vector<std::string> result;
+  for (std::map<int64, std::string>::const_iterator iter =
+           display_names_.begin(); iter != display_names_.end(); ++iter) {
+    if (!IsInternalDisplayId(iter->first))
+      result.push_back(iter->second);
+  }
+
+  return result;
+}
+
 const gfx::Display& MultiDisplayManager::GetDisplayForId(int64 id) const {
   return const_cast<MultiDisplayManager*>(this)->FindDisplayForId(id);
 }
@@ -283,6 +294,8 @@ void MultiDisplayManager::OnNativeDisplaysChanged(
   }
 
   displays_ = new_displays;
+  RefreshDisplayNames();
+
   // Temporarily add displays to be removed because display object
   // being removed are accessed during shutting down the root.
   displays_.insert(displays_.end(), removed_displays.begin(),
@@ -373,29 +386,11 @@ const gfx::Display& MultiDisplayManager::GetDisplayMatching(
 
 std::string MultiDisplayManager::GetDisplayNameFor(

[Daniel Erat, 2012/11/02 16:46:17]

nit (doesn't need to be addressed here): putting "For" on the end of this method
name is pretty uncommon in the chrome source that i've seen.  i'd rename this to
either GetDisplayName() or GetNameForDisplay()

[Jun Mukai, 2012/11/02 18:04:35]

Can we do that in another CL?  This is used in many places.

[Daniel Erat, 2012/11/02 18:59:04]

Sure, sounds fine.

     const gfx::Display& display) {
-  if (HasInternalDisplay() && IsInternalDisplayId(display.id())) {
-    ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
-    return UTF16ToUTF8(
-        bundle.GetLocalizedString(IDS_ASH_INTERNAL_DISPLAY_NAME));
-  }
+  std::map<int64, std::string>::const_iterator iter =
+      display_names_.find(display.id());
+  if (iter != display_names_.end())
+    return iter->second;
 
-#if defined(USE_X11)
-  std::vector<XID> outputs;
-  if (display.id() != gfx::Display::kInvalidDisplayID &&
-      ui::GetOutputDeviceHandles(&outputs)) {
-    for (size_t i = 0; i < outputs.size(); ++i) {
-      uint16 manufacturer_id = 0;
-      uint32 serial_number = 0;
-      std::string name;
-      if (ui::GetOutputDeviceData(
-              outputs[i], &manufacturer_id, &serial_number, &name) &&
-          display.id() ==
-          gfx::Display::GetID(manufacturer_id, serial_number)) {
-        return name;
-      }
-    }
-  }
-#endif
   return base::StringPrintf("Display %d", static_cast<int>(display.id()));
 }
 
@@ -426,6 +421,8 @@ void MultiDisplayManager::Init() {
   }
 #endif
 
+  RefreshDisplayNames();
+
 #if defined(OS_WIN)
   if (base::win::GetVersion() >= base::win::VERSION_WIN8)
     set_use_fullscreen_host_window(true);
@@ -543,6 +540,36 @@ void MultiDisplayManager::EnsurePointerInDisplays() {
   root_window->MoveCursorTo(target_location);
 }
 
+void MultiDisplayManager::RefreshDisplayNames() {
+  display_names_.clear();
+
+  if (!base::chromeos::IsRunningOnChromeOS())
+    return;
+
+#if defined(USE_X11)
+  std::vector<XID> outputs;
+  if (!ui::GetOutputDeviceHandles(&outputs))
+    return;
+
+  for (size_t i = 0; i < outputs.size(); ++i) {
+    uint16 manufacturer_id = 0;
+    uint32 serial_number = 0;
+    std::string name;
+    if (ui::GetOutputDeviceData(
+            outputs[i], &manufacturer_id, &serial_number, &name)) {
+      int64 id = gfx::Display::GetID(manufacturer_id, serial_number);
+      if (IsInternalDisplayId(id)) {
+        ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+        display_names_[id] = UTF16ToUTF8(
+            bundle.GetLocalizedString(IDS_ASH_INTERNAL_DISPLAY_NAME));
+      } else {
+        display_names_[id] = name;
+      }
+    }
+  }
+#endif
+}
+
 void MultiDisplayManager::SetDisplayIdsForTest(DisplayList* to_update) const {
   DisplayList::iterator iter_to_update = to_update->begin();
   DisplayList::const_iterator iter = displays_.begin();