Removes mouse listeners from UserInputMonitor.

media/base/user_input_monitor_linux.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/user_input_monitor.h"
 
 #include <stddef.h>
 #include <sys/select.h>
 #include <unistd.h>
 #define XK_MISCELLANY
@@ skipping 23 matching lines @@
 
 namespace media {
 namespace {
 
 // This is the actual implementation of event monitoring. It's separated from
 // UserInputMonitorLinux since it needs to be deleted on the IO thread.
 class UserInputMonitorLinuxCore
     : public base::SupportsWeakPtr<UserInputMonitorLinuxCore>,
       public base::MessageLoop::DestructionObserver {
  public:
-  enum EventType {
-    MOUSE_EVENT,
-    KEYBOARD_EVENT
-  };
-
   explicit UserInputMonitorLinuxCore(
-      const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
-      const scoped_refptr<UserInputMonitor::MouseListenerList>&
-          mouse_listeners);
+      const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner);
   ~UserInputMonitorLinuxCore() override;
 
   // DestructionObserver overrides.
   void WillDestroyCurrentMessageLoop() override;
 
   size_t GetKeyPressCount() const;
-  void StartMonitor(EventType type);
-  void StopMonitor(EventType type);
+  void StartMonitor();
+  void StopMonitor();
 
  private:
   void OnXEvent();
 
-  // Processes key and mouse events.
+  // Processes key events.
   void ProcessXEvent(xEvent* event);
   static void ProcessReply(XPointer self, XRecordInterceptData* data);
 
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
-  scoped_refptr<base::ObserverListThreadSafe<
-      UserInputMonitor::MouseEventListener>> mouse_listeners_;
 
   //
   // The following members should only be accessed on the IO thread.
   //
   std::unique_ptr<base::FileDescriptorWatcher::Controller> watch_controller_;
   Display* x_control_display_;
   Display* x_record_display_;
-  XRecordRange* x_record_range_[2];
+  XRecordRange* x_record_range_;
   XRecordContext x_record_context_;
   KeyboardEventCounter counter_;
 
   DISALLOW_COPY_AND_ASSIGN(UserInputMonitorLinuxCore);
 };
 
 class UserInputMonitorLinux : public UserInputMonitor {
  public:
   explicit UserInputMonitorLinux(
       const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner);
   ~UserInputMonitorLinux() override;
 
   // Public UserInputMonitor overrides.
   size_t GetKeyPressCount() const override;
 
  private:
   // Private UserInputMonitor overrides.
   void StartKeyboardMonitoring() override;
   void StopKeyboardMonitoring() override;
-  void StartMouseMonitoring() override;
-  void StopMouseMonitoring() override;
 
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
   UserInputMonitorLinuxCore* core_;
 
   DISALLOW_COPY_AND_ASSIGN(UserInputMonitorLinux);
 };
 
 UserInputMonitorLinuxCore::UserInputMonitorLinuxCore(
-    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
-    const scoped_refptr<UserInputMonitor::MouseListenerList>& mouse_listeners)
+    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner)
     : io_task_runner_(io_task_runner),
-      mouse_listeners_(mouse_listeners),
       x_control_display_(NULL),
       x_record_display_(NULL),
-      x_record_context_(0) {
-  x_record_range_[0] = NULL;
-  x_record_range_[1] = NULL;
-}
+      x_record_range_(NULL),
+      x_record_context_(0) {}
 
 UserInputMonitorLinuxCore::~UserInputMonitorLinuxCore() {
   DCHECK(!x_control_display_);
   DCHECK(!x_record_display_);
-  DCHECK(!x_record_range_[0]);
-  DCHECK(!x_record_range_[1]);
+  DCHECK(!x_record_range_);
   DCHECK(!x_record_context_);
 }
 
 void UserInputMonitorLinuxCore::WillDestroyCurrentMessageLoop() {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
-  StopMonitor(MOUSE_EVENT);
-  StopMonitor(KEYBOARD_EVENT);
+  StopMonitor();
+  StopMonitor();
 }
 
 size_t UserInputMonitorLinuxCore::GetKeyPressCount() const {
   return counter_.GetKeyPressCount();
 }
 
-void UserInputMonitorLinuxCore::StartMonitor(EventType type) {
+void UserInputMonitorLinuxCore::StartMonitor() {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
 
-  if (type == KEYBOARD_EVENT)
-    counter_.Reset();
+  counter_.Reset();
 
   // TODO(jamiewalch): We should pass the display in. At that point, since
   // XRecord needs a private connection to the X Server for its data channel
   // and both channels are used from a separate thread, we'll need to duplicate
   // them with something like the following:
   //   XOpenDisplay(DisplayString(display));
   if (!x_control_display_)
     x_control_display_ = gfx::OpenNewXDisplay();
 
   if (!x_record_display_)
     x_record_display_ = gfx::OpenNewXDisplay();
 
   if (!x_control_display_ || !x_record_display_) {
     LOG(ERROR) << "Couldn't open X display";
-    StopMonitor(type);
+    StopMonitor();
     return;
   }
 
   int xr_opcode, xr_event, xr_error;
   if (!XQueryExtension(
            x_control_display_, "RECORD", &xr_opcode, &xr_event, &xr_error)) {
     LOG(ERROR) << "X Record extension not available.";
-    StopMonitor(type);
+    StopMonitor();
     return;
   }
 
-  if (!x_record_range_[type])
-    x_record_range_[type] = XRecordAllocRange();
+  if (!x_record_range_)
+    x_record_range_ = XRecordAllocRange();
 
-  if (!x_record_range_[type]) {
+  if (!x_record_range_) {
     LOG(ERROR) << "XRecordAllocRange failed.";
-    StopMonitor(type);
+    StopMonitor();
     return;
   }
 
-  if (type == MOUSE_EVENT) {
-    x_record_range_[type]->device_events.first = MotionNotify;
-    x_record_range_[type]->device_events.last = MotionNotify;
-  } else {
-    DCHECK_EQ(KEYBOARD_EVENT, type);
-    x_record_range_[type]->device_events.first = KeyPress;
-    x_record_range_[type]->device_events.last = KeyRelease;
-  }
+  x_record_range_->device_events.first = KeyPress;
+  x_record_range_->device_events.last = KeyRelease;
 
   if (x_record_context_) {
     XRecordDisableContext(x_control_display_, x_record_context_);
     XFlush(x_control_display_);
     XRecordFreeContext(x_record_display_, x_record_context_);
     x_record_context_ = 0;
   }
-  XRecordRange** record_range_to_use =
-      (x_record_range_[0] && x_record_range_[1]) ? x_record_range_
-                                                 : &x_record_range_[type];
-  int number_of_ranges = (x_record_range_[0] && x_record_range_[1]) ? 2 : 1;
+  XRecordRange** record_range_to_use = &x_record_range_;

[Wez, 2017/01/03 23:59:40]

nit: You can just use &x_record_range_ directly in the XRecordCreateContext
call, I think.

[CJ, 2017/01/07 00:12:35]

Done.

+  int number_of_ranges = 1;
 
   XRecordClientSpec client_spec = XRecordAllClients;
   x_record_context_ = XRecordCreateContext(x_record_display_,
                                            0,
                                            &client_spec,
                                            1,
                                            record_range_to_use,
                                            number_of_ranges);
   if (!x_record_context_) {
     LOG(ERROR) << "XRecordCreateContext failed.";
-    StopMonitor(type);
+    StopMonitor();
     return;
   }
 
   if (!XRecordEnableContextAsync(x_record_display_,
                                  x_record_context_,
                                  &UserInputMonitorLinuxCore::ProcessReply,
                                  reinterpret_cast<XPointer>(this))) {
     LOG(ERROR) << "XRecordEnableContextAsync failed.";
-    StopMonitor(type);
+    StopMonitor();
     return;
   }
 
-  if (!x_record_range_[0] || !x_record_range_[1]) {
+  if (!x_record_range_) {

[Wez, 2017/01/03 23:59:40]

This is the wrong way around, and by the time you reach here, x_record_range_ is
definitely non-null (see line 155) - you just want to watch the record FD
unconditionally.

[CJ, 2017/01/07 00:12:35]

Done.

     // Register OnXEvent() to be called every time there is something to read
     // from |x_record_display_|.
     watch_controller_ = base::FileDescriptorWatcher::WatchReadable(
         ConnectionNumber(x_record_display_),
         base::Bind(&UserInputMonitorLinuxCore::OnXEvent,
                    base::Unretained(this)));
 
     // Start observing message loop destruction if we start monitoring the first
     // event.
     base::MessageLoop::current()->AddDestructionObserver(this);
   }
 
   // Fetch pending events if any.
   OnXEvent();
 }
 
-void UserInputMonitorLinuxCore::StopMonitor(EventType type) {
+void UserInputMonitorLinuxCore::StopMonitor() {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
 
-  if (x_record_range_[type]) {
-    XFree(x_record_range_[type]);
-    x_record_range_[type] = NULL;
+  if (x_record_range_) {
+    XFree(x_record_range_);
+    x_record_range_ = NULL;
   }
-  if (x_record_range_[0] || x_record_range_[1])
+  if (x_record_range_)
     return;
 
   // Context must be disabled via the control channel because we can't send
   // any X protocol traffic over the data channel while it's recording.
   if (x_record_context_) {
     XRecordDisableContext(x_control_display_, x_record_context_);
     XFlush(x_control_display_);
     XRecordFreeContext(x_record_display_, x_record_context_);
     x_record_context_ = 0;
 
@@ skipping 15 matching lines @@
   DCHECK(io_task_runner_->BelongsToCurrentThread());
   XEvent event;
   // Fetch pending events if any.
   while (XPending(x_record_display_)) {
     XNextEvent(x_record_display_, &event);
   }
 }
 
 void UserInputMonitorLinuxCore::ProcessXEvent(xEvent* event) {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
-  if (event->u.u.type == MotionNotify) {
-    SkIPoint position(SkIPoint::Make(event->u.keyButtonPointer.rootX,
-                                     event->u.keyButtonPointer.rootY));
-    mouse_listeners_->Notify(
-        FROM_HERE, &UserInputMonitor::MouseEventListener::OnMouseMoved,
-        position);
+
+  ui::EventType type;

[Wez, 2017/01/03 23:59:40]

nit: We only need this (and the corresponding code in the Windows version)
because we use ui::EventType as the |type| parameter to OnKeyboardEvent, whereas
in practice OnKeyboardEvent only cares about whether the event is a press or a
release - it would simplify both callers and the KeyboardEventCounter itself if
we switched the API to take a bool |down|  parameter instead.

[CJ, 2017/01/07 00:12:35]

Done.

[Wez, 2017/01/09 22:38:43]

nit: Consider leaving a DCHECK in place to verify that type is KeyPress or
KeyRelease.

[CJ, 2017/01/09 23:35:49]

Done.

+  if (event->u.u.type == KeyPress) {
+    type = ui::ET_KEY_PRESSED;
+  } else if (event->u.u.type == KeyRelease) {
+    type = ui::ET_KEY_RELEASED;
   } else {
-    ui::EventType type;
-    if (event->u.u.type == KeyPress) {
-      type = ui::ET_KEY_PRESSED;
-    } else if (event->u.u.type == KeyRelease) {
-      type = ui::ET_KEY_RELEASED;
-    } else {
-      NOTREACHED();
-      return;
-    }
+    NOTREACHED();
+    return;
+  }
 
-    KeySym key_sym =
-        XkbKeycodeToKeysym(x_control_display_, event->u.u.detail, 0, 0);
-    ui::KeyboardCode key_code = ui::KeyboardCodeFromXKeysym(key_sym);
-    counter_.OnKeyboardEvent(type, key_code);
-  }
+  KeySym key_sym =
+      XkbKeycodeToKeysym(x_control_display_, event->u.u.detail, 0, 0);
+  ui::KeyboardCode key_code = ui::KeyboardCodeFromXKeysym(key_sym);
+  counter_.OnKeyboardEvent(type, key_code);
 }
 
 // static
 void UserInputMonitorLinuxCore::ProcessReply(XPointer self,
                                              XRecordInterceptData* data) {
   if (data->category == XRecordFromServer) {
     xEvent* event = reinterpret_cast<xEvent*>(data->data);
     reinterpret_cast<UserInputMonitorLinuxCore*>(self)->ProcessXEvent(event);
   }
   XRecordFreeData(data);
 }
 
 //
 // Implementation of UserInputMonitorLinux.
 //
 
 UserInputMonitorLinux::UserInputMonitorLinux(
     const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner)
     : io_task_runner_(io_task_runner),
-      core_(new UserInputMonitorLinuxCore(io_task_runner, mouse_listeners())) {}
+      core_(new UserInputMonitorLinuxCore(io_task_runner)) {}
 
 UserInputMonitorLinux::~UserInputMonitorLinux() {
   if (!io_task_runner_->DeleteSoon(FROM_HERE, core_))
     delete core_;
 }
 
 size_t UserInputMonitorLinux::GetKeyPressCount() const {
   return core_->GetKeyPressCount();
 }
 
 void UserInputMonitorLinux::StartKeyboardMonitoring() {
   io_task_runner_->PostTask(
       FROM_HERE,
-      base::Bind(&UserInputMonitorLinuxCore::StartMonitor,
-                 core_->AsWeakPtr(),
-                 UserInputMonitorLinuxCore::KEYBOARD_EVENT));
+      base::Bind(&UserInputMonitorLinuxCore::StartMonitor, core_->AsWeakPtr()));
 }
 
 void UserInputMonitorLinux::StopKeyboardMonitoring() {
   io_task_runner_->PostTask(
       FROM_HERE,
-      base::Bind(&UserInputMonitorLinuxCore::StopMonitor,
-                 core_->AsWeakPtr(),
-                 UserInputMonitorLinuxCore::KEYBOARD_EVENT));
-}
-
-void UserInputMonitorLinux::StartMouseMonitoring() {
-  io_task_runner_->PostTask(FROM_HERE,
-                            base::Bind(&UserInputMonitorLinuxCore::StartMonitor,
-                                       core_->AsWeakPtr(),
-                                       UserInputMonitorLinuxCore::MOUSE_EVENT));
-}
-
-void UserInputMonitorLinux::StopMouseMonitoring() {
-  io_task_runner_->PostTask(FROM_HERE,
-                            base::Bind(&UserInputMonitorLinuxCore::StopMonitor,
-                                       core_->AsWeakPtr(),
-                                       UserInputMonitorLinuxCore::MOUSE_EVENT));
+      base::Bind(&UserInputMonitorLinuxCore::StopMonitor, core_->AsWeakPtr()));
 }
 
 }  // namespace
 
 std::unique_ptr<UserInputMonitor> UserInputMonitor::Create(
     const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
     const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner) {
   return base::WrapUnique(new UserInputMonitorLinux(io_task_runner));
 }
 
 }  // namespace media