Removes mouse listeners from UserInputMonitor.

media/base/keyboard_event_counter.h

 // 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.
 
 #ifndef MEDIA_BASE_KEYBOARD_EVENT_COUNTER_H_
 #define MEDIA_BASE_KEYBOARD_EVENT_COUNTER_H_
 
 #include <stddef.h>
 
 #include <set>
 
 #include "base/macros.h"
 #include "base/synchronization/lock.h"
 #include "media/base/media_export.h"
 #include "ui/events/event_constants.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 
 namespace media {
 
 // This class tracks the total number of keypresses based on the OnKeyboardEvent
 // calls it receives from the client.
 // Multiple key down events for the same key are counted as one keypress until
 // the same key is released.
 class MEDIA_EXPORT KeyboardEventCounter {
  public:
   KeyboardEventCounter();
   ~KeyboardEventCounter();
 
   // Resets the count to 0. Must be called on the same thread as
   // OnKeyboardEvent.

[xhwang, 2017/01/10 20:13:15]

Can we use a ThreadChecker to enforce this?

[Wez, 2017/01/10 23:19:24]

+1, if this constraint is valid.

However, it's not clear why this constraint makes sense, given that the caller
is using GetKeyPressCount() on some other thread - so they will get a bogus
result if the key-capture thread calls Reset between two GetKeyPressCount()
calls.

It looks like Reset() is only used in the Linux StartMonitor() implementation,
and there is a race-condition between it being called on the IO thread and the
calling thread (or some other thread) continuing on and calling
GetKeyPressCount().

Given that the UserInputMonitor's keypress count API doesn't guarantee that
keypress counts are zero-based, can we just get rid of Reset() entirely?

[CJ, 2017/01/11 23:41:55]

Waiting on the result of this conversation. 

[CJ, 2017/01/12 23:59:37]

Removing Reset(). In the implementation, |total_key_presses_| is cast to an
base::subtle::AtomicWord. Does that at all help with the raise case u were
mentioning (this is just for my understanding).

[Wez, 2017/01/13 01:26:16]

No, but good question.

The base::subtle::Atomic stuff is about ensuring that values are written
atomically to memory - e.g. if you write 0xff0000ff to a location that currently
contains 0x00ffff00 then it is possible (though usually unlikely) for some other
thread reading that location mid-write to "see" a value like 0xff00ff00, because
it's reading while the other thread is mid-write!

There is a separate aspect, memory "barriers", which is basically to do with
whether other threads might still see a stale value because of caching.

The race-condition I was talking about, though, is to do with the fact that the
caller is expecting to call GetKeyEventCount() on e.g. the UI thread, while
OnKeyboardEvent() and Reset() are called on some other background thread - the
caller might start listening for keyboard events and immediately afterwards
check the count and get a non-zero value like 5, because that's what the count
had previously got up to, but then the next time it calls it sees zero, because
Reset() took place on the other thread in the meantime.

Looking at the use of atomics in the implementation, I'm surprised that we're
doing the casts, rather than just using base::subtle::Atomic32 etc as the
counter type directly, and just casting it to a size_t when we want to return
it.

   void Reset();
 
   // Returns the total number of keypresses since its creation or last Reset()
   // call. Can be called on any thread.
   size_t GetKeyPressCount() const;
 
   // The client should call this method on key down or key up events.
   // Must be called on a single thread.
-  void OnKeyboardEvent(ui::EventType event, ui::KeyboardCode key_code);
+  void OnKeyboardEvent(bool down, ui::KeyboardCode key_code);
 
  private:
   // The set of keys currently held down.
   std::set<ui::KeyboardCode> pressed_keys_;
 
   size_t total_key_presses_;
 
   DISALLOW_COPY_AND_ASSIGN(KeyboardEventCounter);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BASE_KEYBOARD_EVENT_COUNTER_H_