Block internal PlatformEvents before they are dispatched in touchview.

Block internal PlatformEvents before they are dispatched in touchview.

This prevents updating the key state.

BUG=374002, 368669
TEST=EventsXTest.Disable*
TEST=On device opened past 180 degrees, hold shift and click on link. Link
should open in same window.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279732

[flackr, 2014-06-04 16:18:12]

Hey, can you take a look at this approach. This is effectively moving the
blocking to an earlier place. As we discussed in person, I've made it such that
the order of filters does not matter, they are always all notified and any one
blocking the event will prevent it going through. Let me know what you think.

Ben, FYI, this fixes blocking the keyboard volume keys but not the side volume
keys as it happens before the rewriting.

[bruthig, 2014-06-04 19:16:04]

@Sadrul, this fix makes some of the following code review redundant and I will
not submit it.  However my review also has a fix for www.crbug.com/368710
"Identify Fn-media keys", do we still want to submit my fix?  Are there any
other use cases for it?

https://codereview.chromium.org/313913004/diff/40001/ash/wm/maximize_mode/int...
File ash/wm/maximize_mode/internal_input_device_list.h (right):

https://codereview.chromium.org/313913004/diff/40001/ash/wm/maximize_mode/int...
ash/wm/maximize_mode/internal_input_device_list.h:13: class Event;
Is this forward declaration needed any more?

[sadrul, 2014-06-05 16:03:46]

On 2014/06/04 19:16:04, bruthig wrote:
> @Sadrul, this fix makes some of the following code review redundant and I will
> not submit it.  However my review also has a fix for
http://www.crbug.com/368710
> "Identify Fn-media keys", do we still want to submit my fix?  Are there any
> other use cases for it?

bruthig@: I think your CL is still good. We should try to land it.

[sadrul, 2014-06-05 16:04:39]

I am not a fan of adding yet another list of general purpose
observers/filters/dispatchers for event-dispatch. I would much rather we special
case this, for example, through the DeviceDataManager. The API will likely be a
little clunky (e.g. DDM::DisableInternalKeyboard(std::vector<KeyCode>
allowed_codes), or something better), but that would still be better (isolated
to just touch-view, just on X11/ChromeOS).

[flackr, 2014-06-10 19:10:28]

On 2014/06/05 16:04:39, sadrul wrote:
> I am not a fan of adding yet another list of general purpose
> observers/filters/dispatchers for event-dispatch. I would much rather we
special
> case this, for example, through the DeviceDataManager. The API will likely be
a
> little clunky (e.g. DDM::DisableInternalKeyboard(std::vector<KeyCode>
> allowed_codes), or something better), but that would still be better (isolated
> to just touch-view, just on X11/ChromeOS).

Updated as per our discussion. Please take a look, thanks.

[flackr, 2014-06-11 22:16:48]

https://codereview.chromium.org/313913004/diff/50001/ash/wm/maximize_mode/sco...
File ash/wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_x11.cc
(right):

https://codereview.chromium.org/313913004/diff/50001/ash/wm/maximize_mode/sco...
ash/wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_x11.cc:63:
keyboard_device_id_(0),
I believe 0 is a valid device id, I will default these to -1 instead.

[sadrul, 2014-06-12 21:02:27]

LGTM

https://codereview.chromium.org/313913004/diff/50001/ui/aura/window_tree_host...
File ui/aura/window_tree_host_x11.cc (right):

https://codereview.chromium.org/313913004/diff/50001/ui/aura/window_tree_host...
ui/aura/window_tree_host_x11.cc:423: }
Maybe this could move out of the switch/case (like you have done below for
XI_Motion).

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
File ui/events/x/device_data_manager.cc (right):

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
ui/events/x/device_data_manager.cc:660: DCHECK(excepted_keys);
This is probably not necessary? It should be possible to disable all keys.

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
File ui/events/x/device_data_manager.h (right):

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
ui/events/x/device_data_manager.h:244: // known location from an unblocked
event.
Can you clarify the comment about mouse events? (calling this function does not
restore the mouse-cursor to the last know position, right?)

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
ui/events/x/device_data_manager.h:245: bool EventBlocked(const
base::NativeEvent& native_event);
IsEventBlocked()?

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
ui/events/x/device_data_manager.h:284: // be blocked.
+should

[flackr, 2014-06-18 05:28:32]

https://codereview.chromium.org/313913004/diff/50001/ash/wm/maximize_mode/sco...
File ash/wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_x11.cc
(right):

https://codereview.chromium.org/313913004/diff/50001/ash/wm/maximize_mode/sco...
ash/wm/maximize_mode/scoped_disable_internal_mouse_and_keyboard_x11.cc:63:
keyboard_device_id_(0),
On 2014/06/11 22:16:47, flackr wrote:
> I believe 0 is a valid device id, I will default these to -1 instead.

Done.

https://codereview.chromium.org/313913004/diff/50001/ui/aura/window_tree_host...
File ui/aura/window_tree_host_x11.cc (right):

https://codereview.chromium.org/313913004/diff/50001/ui/aura/window_tree_host...
ui/aura/window_tree_host_x11.cc:423: }
On 2014/06/12 21:02:27, sadrul wrote:
> Maybe this could move out of the switch/case (like you have done below for
> XI_Motion).

Done.

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
File ui/events/x/device_data_manager.cc (right):

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
ui/events/x/device_data_manager.cc:660: DCHECK(excepted_keys);
On 2014/06/12 21:02:27, sadrul wrote:
> This is probably not necessary? It should be possible to disable all keys.

Done.

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
File ui/events/x/device_data_manager.h (right):

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
ui/events/x/device_data_manager.h:244: // known location from an unblocked
event.
On 2014/06/12 21:02:27, sadrul wrote:
> Can you clarify the comment about mouse events? (calling this function does
not
> restore the mouse-cursor to the last know position, right?)

Done.

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
ui/events/x/device_data_manager.h:245: bool EventBlocked(const
base::NativeEvent& native_event);
On 2014/06/12 21:02:27, sadrul wrote:
> IsEventBlocked()?

Done.

https://codereview.chromium.org/313913004/diff/50001/ui/events/x/device_data_...
ui/events/x/device_data_manager.h:284: // be blocked.
On 2014/06/12 21:02:27, sadrul wrote:
> +should

Done.

[flackr, 2014-06-18 05:28:58]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-06-18 05:31:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/flackr@chromium.org/313913004/70001

[commit-bot: I haz the power, 2014-06-18 16:43:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-18 16:43:18]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/18422)

[flackr, 2014-06-19 21:18:56]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-06-19 21:21:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/flackr@chromium.org/313913004/130001

[commit-bot: I haz the power, 2014-06-20 00:52:38]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/8...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/19241)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/22707)

[commit-bot: I haz the power, 2014-06-20 00:55:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-20 00:55:38]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/19288)

[flackr, 2014-06-20 06:50:20]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-06-20 06:52:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/flackr@chromium.org/313913004/130001

[commit-bot: I haz the power, 2014-06-20 09:53:52]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/8...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/19459)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/22942)

[commit-bot: I haz the power, 2014-06-20 09:56:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-20 09:56:39]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/19478)

[flackr, 2014-06-23 17:30:21]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-06-23 17:31:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/flackr@chromium.org/313913004/170001

[commit-bot: I haz the power, 2014-06-23 17:43:56]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/8...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/20403)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/24242)

[commit-bot: I haz the power, 2014-06-23 17:45:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-23 17:45:41]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/20410)

[flackr, 2014-06-24 15:43:36]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-06-24 15:44:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/flackr@chromium.org/313913004/190001

[flackr, 2014-06-24 17:44:43]

The CQ bit was unchecked by flackr@chromium.org

[flackr, 2014-06-24 17:46:48]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-06-24 17:48:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/flackr@chromium.org/313913004/210001

[commit-bot: I haz the power, 2014-06-24 21:30:44]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)

[commit-bot: I haz the power, 2014-06-24 22:35:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-24 22:35:35]

Try jobs failed on following builders:
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)

[flackr, 2014-06-25 14:36:53]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2014-06-25 14:38:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/flackr@chromium.org/313913004/230001

[commit-bot: I haz the power, 2014-06-25 16:32:22]

Change committed as 279732

[Avi (use Gerrit), 2014-06-25 17:01:39]

I reverted this with 279734. This appears to have broken a bot:

http://build.chromium.org/p/chromium.linux/builders/Linux%20Builder%20%28dbg%...

[809/2026 | 40.939] CXX
obj/remoting/host/remoting_unittests.json_host_config_unittest.o
FAILED: /b/build/goma/gomacc c++ -Wl,-z,now -Wl,-z,relro -Wl,--fatal-warnings
-pthread -Wl,-z,noexecstack -fPIC -Wl,-uIsHeapProfilerRunning,-uProfilerStart
-Wl,-u_Z21InitialM...buf_lite.so lib/libnet.so lib/libmedia.so
lib/libbase_i18n.so lib/libcrcrypto.so -lrt -ldl -lasound -lnss3 -lnssutil3
-lsmime3 -lplds4 -lplc4 -lnspr4
lib//libevents_base.so: undefined reference to
`ui::KeyboardCodeFromNative(_XEvent* const&)'
collect2: ld returned 1 exit status
ninja: build stopped: subcommand failed.

[Avi (use Gerrit), 2014-06-25 17:11:46]

Yeah, this appears to have been the issue.