Associate internal touchscreen to internal display

ui/display/chromeos/x11/touchscreen_device_manager_x11.cc

Index: ui/display/chromeos/x11/touchscreen_device_manager_x11.cc
diff --git a/ui/display/chromeos/x11/touchscreen_device_manager_x11.cc b/ui/display/chromeos/x11/touchscreen_device_manager_x11.cc
index 6c8137b1cb47e7893fbd3d33c8181ed6dba931f7..d9239316b813b37aaaf76b0f9c0bd23da6bd96c9 100644
--- a/ui/display/chromeos/x11/touchscreen_device_manager_x11.cc
+++ b/ui/display/chromeos/x11/touchscreen_device_manager_x11.cc
@@ -10,8 +10,51 @@
 #include <cmath>
 #include <set>
 
+#include "base/command_line.h"
+#include "base/logging.h"
+#include "base/process/launch.h"
+#include "base/sys_info.h"
 #include "ui/gfx/x/x11_types.h"
 
+namespace {
+
+bool IsTouchscreenInternal(Display* dpy, int device_id) {
+  bool ret = false;

[Daniel Erat, 2014/07/16 02:59:46]

i don't see any benefit from declaring this so early -- just return false in all
of the failure conditions below and declare this variable right before you
actually need it

[Yufeng Shen (Slow to review), 2014/07/16 20:20:28]

Done.

+  if (!base::SysInfo::IsRunningOnChromeOS())
+    return ret;
+
+  // Input device has a property "Device Node" pointing to its dev input node,
+  // e.g.   Device Node (250): "/dev/input/event8"
+  Atom device_node = XInternAtom(dpy, "Device Node", False);
+  if (device_node == None)
+    return ret;
+
+  Atom act_type;
+  int  act_format;

[Daniel Erat, 2014/07/16 02:59:46]

nit: remove double-space between int and act_format

[Yufeng Shen (Slow to review), 2014/07/16 20:20:28]

Done.

+  unsigned long nitems, bytes_after;
+  unsigned char *data;

[Daniel Erat, 2014/07/16 02:59:46]

* goes on left side of space

[Yufeng Shen (Slow to review), 2014/07/16 20:20:28]

Done.

+  XDevice* dev = XOpenDevice(dpy, device_id);
+  if (!dev)
+    return ret;
+
+  if (XGetDeviceProperty(dpy, dev, device_node, 0, 1000, False,
+                         AnyPropertyType, &act_type, &act_format,
+                         &nitems, &bytes_after, &data) == Success) {
+    // Script is_touchscreen_internal takes in the dev input node and
+    // returns if the according touchscreen is an internal touchscreen.
+    CommandLine command(
+        base::FilePath("/opt/google/touchscreen/is_touchscreen_internal"));

[Daniel Erat, 2014/07/16 02:59:46]

why is this a separate script? putting the logic in chrome seems like it'll make
the code easier to read and be faster...

[Yufeng Shen (Slow to review), 2014/07/16 20:20:28]

yeah, why not.
done.

+    command.AppendArg(reinterpret_cast<char*>(data));
+    std::string output;
+    ret = GetAppOutput(command, &output);
+    XFree(data);
+    XCloseDevice(dpy, dev);
+  }
+  return ret;
+}
+
+}
+
 namespace ui {
 
 TouchscreenDeviceManagerX11::TouchscreenDeviceManagerX11()
@@ -70,8 +113,10 @@ std::vector<TouchscreenDevice> TouchscreenDeviceManagerX11::GetDevices() {
     // Touchscreens should have absolute X and Y axes, and be direct touch
     // devices.
     if (width > 0.0 && height > 0.0 && is_direct_touch) {
+      bool is_internal = IsTouchscreenInternal(display_, info[i].deviceid);
       devices.push_back(TouchscreenDevice(info[i].deviceid,
-                                          gfx::Size(width, height)));
+                                          gfx::Size(width, height),
+                                          is_internal));
     }
   }