Tweak XInput2 event subscription

base/message_pump_glib_x.cc

Index: base/message_pump_glib_x.cc
diff --git a/base/message_pump_glib_x.cc b/base/message_pump_glib_x.cc
index 35cfc1a533e158cd71b31f57b5501b88feac7c64..3c8b843c6a6cc2a3193624ab4544b7e4112b6129 100644
--- a/base/message_pump_glib_x.cc
+++ b/base/message_pump_glib_x.cc
@@ -79,8 +79,7 @@ namespace base {
 MessagePumpGlibX::MessagePumpGlibX() : base::MessagePumpForUI(),
 #if defined(HAVE_XINPUT2)
     xiopcode_(-1),
-    masters_(),
-    floats_(),
+    pointer_devices_(),
 #endif
     gdksource_(NULL),
     dispatching_event_(false),
@@ -112,29 +111,17 @@ void MessagePumpGlibX::SetupXInput2ForXWindow(Window xwindow) {
   XISetMask(mask, XI_ButtonRelease);
   XISetMask(mask, XI_Motion);
 
-  // It is not necessary to select for slave devices. XInput2 provides enough
-  // information to the event callback to decide which slave device triggered
-  // the event, thus decide whether the 'pointer event' is a 'mouse event' or a
-  // 'touch event'.
-  // If the touch device has 'GrabDevice' set and 'SendCoreEvents' unset (which
-  // is possible), then the device is detected as a floating device, and a
-  // floating device is not connected to a master device. So it is necessary to
-  // also select on the floating devices.
-  std::set<int> devices;
-  std::set_union(masters_.begin(), masters_.end(),
-                 floats_.begin(), floats_.end(),
-                 std::inserter(devices, devices.begin()));
-  XIEventMask evmasks[devices.size()];
+  XIEventMask evmasks[pointer_devices_.size()];
   int count = 0;
-  for (std::set<int>::const_iterator iter = devices.begin();
-       iter != devices.end();
+  for (std::set<int>::const_iterator iter = pointer_devices_.begin();
+       iter != pointer_devices_.end();
        ++iter, ++count) {
     evmasks[count].deviceid = *iter;
     evmasks[count].mask_len = sizeof(mask);
     evmasks[count].mask = mask;
   }
 
-  XISelectEvents(xdisplay, xwindow, evmasks, devices.size());
+  XISelectEvents(xdisplay, xwindow, evmasks, pointer_devices_.size());
 
   // TODO(sad): Setup masks for keyboard events.
 
@@ -297,15 +284,21 @@ void MessagePumpGlibX::InitializeXInput2(void) {
   SetupGtkWidgetRealizeNotifier(this);
 
   // Instead of asking X for the list of devices all the time, let's maintain a
-  // list of slave (physical) and master (virtual) pointer devices.
+  // list of pointer devices we care about.
+  // It is not necessary to select for slave devices. XInput2 provides enough
+  // information to the event callback to decide which slave device triggered
+  // the event, thus decide whether the 'pointer event' is a 'mouse event' or a
+  // 'touch event'.
+  // If the touch device has 'GrabDevice' set and 'SendCoreEvents' unset (which
+  // is possible), then the device is detected as a floating device, and a
+  // floating device is not connected to a master device. So it is necessary to
+  // also select on the floating devices.
   int count = 0;
   XIDeviceInfo* devices = XIQueryDevice(xdisplay, XIAllDevices, &count);
   for (int i = 0; i < count; i++) {
     XIDeviceInfo* devinfo = devices + i;
-    if (devinfo->use == XIFloatingSlave) {
-      floats_.insert(devinfo->deviceid);
-    } else if (devinfo->use == XIMasterPointer) {
-      masters_.insert(devinfo->deviceid);
+    if (devinfo->use == XIFloatingSlave || devinfo->use == XIMasterPointer) {
+      pointer_devices_.insert(devinfo->deviceid);

[sadrul, 2011/03/25 15:49:41]

Braces aren't necessary here (there's nothing about this in the style guide, I
think, and there are examples of both in chromium code, but I have been
requested to not use braces in such cases a few times). Sorry I missed this in
the earlier review.

[Rick Byers, 2011/03/25 17:25:49]

Done.

     }
   }
   XIFreeDeviceInfo(devices);

[sadrul, 2011/03/25 15:49:41]

Check for 'count' or 'devices' here too?

[Rick Byers, 2011/03/25 17:25:49]

I don't think so - there should be no legitimate reason as far as I know for
XIQueryDevice to return null here.  We've already verified above that XInput2 is
supported.   

That said, if you think it's better to be defensive then I'd be ok with that
(though I generally feel it's better to let it crash when something you're not
prepared for occurs).

[sadrul, 2011/03/25 18:04:57]

I thought this is where you were getting the crash with NX?