Changes how to compute the id of a display.

chromeos/display/output_util.cc

Index: chromeos/display/output_util.cc
diff --git a/chromeos/display/output_util.cc b/chromeos/display/output_util.cc
index aa235b05f3cf248c11ba23f7b04b5b85ef96a092..f19683aa03db76f0f43c9a10a269f27b9f8257db 100644
--- a/chromeos/display/output_util.cc
+++ b/chromeos/display/output_util.cc
@@ -8,6 +8,7 @@
 #include <X11/extensions/Xrandr.h>
 #include <X11/Xatom.h>
 
+#include "base/hash.h"
 #include "base/message_loop.h"
 #include "base/string_util.h"
 #include "base/sys_byteorder.h"
@@ -20,16 +21,16 @@ const char kInternal_LVDS[] = "LVDS";
 const char kInternal_eDP[] = "eDP";
 
 // Returns 64-bit persistent ID for the specified manufacturer's ID and
-// product_code, and the index of the output it is connected to.
+// product_name, and the index of the output it is connected to.

[oshima, 2013/05/21 21:53:31]

keep this name consistent in the file, like product_name_code,
product_code_hash.

[Jun Mukai, 2013/05/22 00:28:05]

Done.

 // |output_index| is used to distinguish the displays of the same type. For
 // example, swapping two identical display between two outputs will not be
 // treated as swap. The 'serial number' field in EDID isn't used here because
 // it is not guaranteed to have unique number and it may have the same fixed
 // value (like 0).
 int64 GetID(uint16 manufacturer_id,
-            uint16 product_code,
+            uint32 product_code,

[oshima, 2013/05/21 21:53:31]

ditto

[Jun Mukai, 2013/05/22 00:28:05]

Done.

             uint8 output_index) {
-  return ((static_cast<int64>(manufacturer_id) << 24) |
+  return ((static_cast<int64>(manufacturer_id) << 40) |
           (static_cast<int64>(product_code) << 8) | output_index);
 }
 
@@ -119,14 +120,42 @@ std::string GetDisplayName(XID output_id) {
 }
 
 bool GetDisplayId(XID output_id, size_t output_index, int64* display_id_out) {
+  unsigned long nitems = 0;
+  unsigned char* prop = NULL;
+  if (!GetEDIDProperty(output_id, &nitems, &prop))
+    return false;
+
+  bool result =
+      GetDisplayIdFromEDID(prop, nitems, output_index, display_id_out);
+  XFree(prop);
+  return result;
+}
+
+bool GetDisplayIdFromEDID(const unsigned char* prop,
+                          unsigned long nitems,
+                          size_t output_index,
+                          int64* display_id_out) {
   uint16 manufacturer_id = 0;
   uint16 product_code = 0;
-  if (GetOutputDeviceData(
-          output_id, &manufacturer_id, &product_code, NULL) &&
-      manufacturer_id != 0) {
+  std::string product_name;
+
+  // ParseOutputDeviceData fails if it doesn't have product_name.
+  ParseOutputDeviceData(
+      prop, nitems, &manufacturer_id, &product_code, &product_name);
+
+  // Generates product specific value from product_name. See crbug.com/240341
+  // for why we use product name string instead of product code in EDID. Use
+  // fallback value to 0 instead of product code just in case that the code
+  // over-classify a device.
+  // TODO(mukai): fix the code when we encounter a vendor which doesn't put
+  // display name but distinguish them by product code.

[oshima, 2013/05/21 21:53:31]

Actually, we can't use product code because we know it's not safe. It's better
to keep this way.

[Jun Mukai, 2013/05/22 00:28:05]

Done.

+  uint32 product_specific_code = product_name.empty() ?
+      0 : base::Hash(product_name);
+  if (manufacturer_id != 0) {
     // An ID based on display's index will be assigned later if this call
     // fails.
-    *display_id_out = GetID(manufacturer_id, product_code, output_index);
+    *display_id_out = GetID(
+        manufacturer_id, product_specific_code, output_index);
     return true;
   }
   return false;