Include external touchscreen vid/pid in UMA hardware profile

ui/events/x/touch_factory_x11.h

Index: ui/events/x/touch_factory_x11.h
diff --git a/ui/events/x/touch_factory_x11.h b/ui/events/x/touch_factory_x11.h
index ed62c6d247f59ec6cefcd76b775c1e8623ad686d..16e896c989761c61db3d5ab2d80379b6ea3654a2 100644
--- a/ui/events/x/touch_factory_x11.h
+++ b/ui/events/x/touch_factory_x11.h
@@ -7,6 +7,8 @@
 
 #include <bitset>
 #include <map>
+#include <set>
+#include <utility>
 #include <vector>
 
 #include "base/timer/timer.h"
@@ -72,6 +74,11 @@ class EVENTS_BASE_EXPORT TouchFactory {
   // Whether any touch device is currently present and enabled.
   bool IsTouchDevicePresent();
 
+  // Pairs of <vendor id, product id> of external touch screens.
+  const std::set<std::pair<int, int> >& GetTouchscreenIds() const {
+    return touchscreen_ids_;
+  }
+
   // Return maximum simultaneous touch points supported by device.
   int GetMaxTouchPoints() const;
 
@@ -89,6 +96,8 @@ class EVENTS_BASE_EXPORT TouchFactory {
   // Requirement for Singleton
   friend struct DefaultSingletonTraits<TouchFactory>;
 
+  void CacheTouchscreenIds(Display* display, int id);
+
   // NOTE: To keep track of touch devices, we currently maintain a lookup table
   // to quickly decide if a device is a touch device or not. We also maintain a
   // list of the touch devices. Ideally, there will be only one touch device,
@@ -111,6 +120,9 @@ class EVENTS_BASE_EXPORT TouchFactory {
   // Indicates whether touch events are explicitly disabled.
   bool touch_events_disabled_;

[jar (doing other things), 2014/02/05 00:10:13]

nit/pet-peeve:  It is a bad idea to have a negative in the name of a boolean. IT
requires a lot more thought to read the meaning when you often have a double
negative :-/.

  Much better is touch_events_enabled_.  

If you feel the same... it would be nice to clean up this name, maybe even
during this CL.

[tdresser, 2014/02/05 19:29:58]

I definitely agree in general - not sure about this specific instance though.

This boolean is true iff an action has been taken to disable touch events.

This isn't the same as saying touch events have been enabled. If you negated the
current meaning of this variable, you'd end up with a boolean which is true iff
no action has been taken to disable touch events, which feels more confusing to
me.

What do you think?

[jar1, 2014/02/05 20:27:18]

IMO, if you intended this to have 3 states: Never set; Enabled, and Disabled, it
would be better to have a tri-state (enum) than a bool.

That said... I was just looking at a quick improvement (to change polarity).  If
you want to argue it is tri-state, masking as a bool, with special semantics...
it would be  IMO a bigger change than I'd ask to be mixed into the current CL.

I'll leave it to you... but I think you should probably skip it at this point.

 
+  // Indicates whether the device's internal display produces touch events.
+  bool internal_display_supports_touch_;

[jar (doing other things), 2014/02/05 00:10:13]

I couldn't find where this was used... though I did see that it wasn't
initialized in the constructor.

[tdresser, 2014/02/05 19:29:58]

Yikes, that was supposed to have been removed long ago.
Good catch.

+
   // The list of touch devices. For testing/debugging purposes, a single-pointer
   // device (mouse or touch screen without sufficient X/driver support for MT)
   // can sometimes be treated as a touch device. The key in the map represents
@@ -118,6 +130,9 @@ class EVENTS_BASE_EXPORT TouchFactory {
   // capable.
   std::map<int, bool> touch_device_list_;
 
+  // Touch screen <vid, pid>s.
+  std::set<std::pair<int, int> > touchscreen_ids_;
+
   // Maximum simultaneous touch points supported by device. In the case of
   // devices with multiple digitizers (e.g. multiple touchscreens), the value
   // is the maximum of the set of maximum supported contacts by each individual