Disable pinch-to-zoom gesture on virtual keyboard

Disable pinch-to-zoom gesture on virtual keyboard

BUG=678997
TEST=Added a unit test and did manual test.

Review-Url: https://codereview.chromium.org/2666843003
Cr-Commit-Position: refs/heads/master@{#448181}
Committed: https://chromium.googlesource.com/chromium/src/+/3e199b05f97cf413fec8e19076d2da6a97865888

[yhanada, 2017-01-31 10:46:24]

The CQ bit was checked by yhanada@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 10:46:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-31 11:48:55]

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

[commit-bot: I haz the power, 2017-01-31 11:48:56]

Dry run: This issue passed the CQ dry run.

[yhanada, 2017-01-31 15:58:50]

The CQ bit was checked by yhanada@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 15:59:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yhanada, 2017-01-31 16:00:08]

yhanada@chromium.org changed reviewers:
+ bshe@chromium.org, oshima@chromium.org

[yhanada, 2017-01-31 16:00:09]

PTAL. Thanks!

[bshe, 2017-01-31 16:39:25]

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_co...
File ui/keyboard/keyboard_controller.h (right):

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_co...
ui/keyboard/keyboard_controller.h:181: KeyboardEventFilter event_filter_;
nit: consider use a unique_ptr

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
File ui/keyboard/keyboard_event_filter.h (right):

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
ui/keyboard/keyboard_event_filter.h:15: // processed by the keyboard window and
excludes pinch gesture events.
"excludes pinch gesture events" doesn't feel like it belongs to a class level
comment?

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
ui/keyboard/keyboard_event_filter.h:19: ~KeyboardEventFilter() override{};
nit: use "= default"?

[commit-bot: I haz the power, 2017-01-31 17:30:23]

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

[commit-bot: I haz the power, 2017-01-31 17:30:25]

Dry run: This issue passed the CQ dry run.

[oshima, 2017-01-31 21:06:53]

lgtm but please wait for bshe@'s review

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_co...
File ui/keyboard/keyboard_controller.h (right):

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_co...
ui/keyboard/keyboard_controller.h:181: KeyboardEventFilter event_filter_;
On 2017/01/31 16:39:25, bshe wrote:
> nit: consider use a unique_ptr

this is actually fine if the lifetime is same as owning class.

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
File ui/keyboard/keyboard_event_filter.h (right):

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
ui/keyboard/keyboard_event_filter.h:18: KeyboardEventFilter(){};
space between ){. Did you run git cl format?

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
File ui/keyboard/keyboard_event_filter_unittest.cc (right):

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
ui/keyboard/keyboard_event_filter_unittest.cc:11: namespace {
no need to put it in anonymous namespace. The test name should be unique anyway.

[yhanada, 2017-02-01 02:48:25]

The CQ bit was checked by yhanada@chromium.org to run a CQ dry run

[yhanada, 2017-02-01 02:48:31]

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
File ui/keyboard/keyboard_event_filter.h (right):

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
ui/keyboard/keyboard_event_filter.h:15: // processed by the keyboard window and
excludes pinch gesture events.
On 2017/01/31 16:39:25, bshe wrote:
> "excludes pinch gesture events" doesn't feel like it belongs to a class level
> comment?

I removed that part.

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
ui/keyboard/keyboard_event_filter.h:18: KeyboardEventFilter(){};
On 2017/01/31 21:06:53, oshima wrote:
> space between ){. Did you run git cl format?

Yes. git cl format removes the spaces.

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
ui/keyboard/keyboard_event_filter.h:19: ~KeyboardEventFilter() override{};
On 2017/01/31 16:39:25, bshe wrote:
> nit: use "= default"?

Done.

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
File ui/keyboard/keyboard_event_filter_unittest.cc (right):

https://codereview.chromium.org/2666843003/diff/20001/ui/keyboard/keyboard_ev...
ui/keyboard/keyboard_event_filter_unittest.cc:11: namespace {
On 2017/01/31 21:06:53, oshima wrote:
> no need to put it in anonymous namespace. The test name should be unique
anyway.

Done.

[commit-bot: I haz the power, 2017-02-01 02:48:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 03:00:47]

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

[commit-bot: I haz the power, 2017-02-01 03:00:47]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yhanada, 2017-02-01 04:21:23]

The CQ bit was checked by yhanada@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-01 04:21:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 05:56:08]

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

[commit-bot: I haz the power, 2017-02-01 05:56:09]

Dry run: This issue passed the CQ dry run.

[oka, 2017-02-03 02:02:09]

Description was changed from

==========
Disable pinch-to-zoom gesture on virtual keyboard

BUG=678997
TEST=Added a unit test and did manual test.
==========

to

==========
Disable pinch-to-zoom gesture on virtual keyboard

BUG=678997
TEST=Added a unit test and did manual test.
==========

[yhanada, 2017-02-03 09:47:09]

bshe@: ping? :)

[bshe, 2017-02-03 15:41:17]

On 2017/02/03 09:47:09, yhanada wrote:
> bshe@: ping? :)

lgtm sorry for the delay

[yhanada, 2017-02-05 07:06:34]

On 2017/02/03 15:41:17, bshe wrote:
> On 2017/02/03 09:47:09, yhanada wrote:
> > bshe@: ping? :)
> 
> lgtm sorry for the delay

No problem. Thank you!

[yhanada, 2017-02-05 07:06:42]

The CQ bit was checked by yhanada@chromium.org

[yhanada, 2017-02-05 07:06:42]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2666843003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-05 07:06:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-05 08:33:19]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1486278402151390,
"parent_rev": "8a97a3c7a309559c6e7a15e371bff2f79defde31", "commit_rev":
"3e199b05f97cf413fec8e19076d2da6a97865888"}

[commit-bot: I haz the power, 2017-02-05 08:34:38]

Description was changed from

==========
Disable pinch-to-zoom gesture on virtual keyboard

BUG=678997
TEST=Added a unit test and did manual test.
==========

to

==========
Disable pinch-to-zoom gesture on virtual keyboard

BUG=678997
TEST=Added a unit test and did manual test.

Review-Url: https://codereview.chromium.org/2666843003
Cr-Commit-Position: refs/heads/master@{#448181}
Committed:
https://chromium.googlesource.com/chromium/src/+/3e199b05f97cf413fec8e19076d2...
==========

[commit-bot: I haz the power, 2017-02-05 08:34:39]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/3e199b05f97cf413fec8e19076d2...