Tapping handle shouldn't select misspelled word

Tapping handle shouldn't select misspelled word

After crrev.com/2721813002 tapping an insertion handle triggered a contextmenu event
which led to bug 717337. This CL fixes this bug by keeping track that the insertion
handle tap contextmenu event came from touch.

BUG=717337
Review-Url: https://codereview.chromium.org/2864833002
Cr-Commit-Position: refs/heads/master@{#474912}
Committed: https://chromium.googlesource.com/chromium/src/+/915818ef6101ba84d48f9b359f4418f702bca8e6

[amaralp, 2017-05-06 00:47:07]

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

[commit-bot: I haz the power, 2017-05-06 00:47:51]

Dry run: CQ is trying da patch.

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

[amaralp, 2017-05-06 00:59:01]

Description was changed from

==========
Tapping handle shouldn't select misspelled word

BUG=
==========

to

==========
Tapping handle shouldn't select misspelled word

After crrev.com/??? tapping an insertion handle triggered a contextmenu event
which
led to bug 717337. This CL fixes this bug by keeping track that the insertion
handle tap contextmenu event came from touch.

BUG=717337
==========

[amaralp, 2017-05-06 01:01:02]

Description was changed from

==========
Tapping handle shouldn't select misspelled word

After crrev.com/??? tapping an insertion handle triggered a contextmenu event
which
led to bug 717337. This CL fixes this bug by keeping track that the insertion
handle tap contextmenu event came from touch.

BUG=717337
==========

to

==========
Tapping handle shouldn't select misspelled word

After crrev.com/2721813002 tapping an insertion handle triggered a contextmenu
event
which led to bug 717337. This CL fixes this bug by keeping track that the
insertion
handle tap contextmenu event came from touch.

BUG=717337
==========

[commit-bot: I haz the power, 2017-05-06 01:03:28]

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

[commit-bot: I haz the power, 2017-05-06 01:03:29]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[amaralp, 2017-05-08 18:05:17]

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

[commit-bot: I haz the power, 2017-05-08 18:05:58]

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-05-08 20:19:36]

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

[commit-bot: I haz the power, 2017-05-08 20:19:37]

Dry run: This issue passed the CQ dry run.

[amaralp, 2017-05-09 19:31:21]

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

[commit-bot: I haz the power, 2017-05-09 19:32:15]

Dry run: CQ is trying da patch.

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

[amaralp, 2017-05-09 21:11:16]

amaralp@chromium.org changed reviewers:
+ aelias@chromium.org, bokan@chromium.org, sadrul@chromium.org,
yosin@chromium.org

[amaralp, 2017-05-09 21:11:17]

PTAL,

aelias@ for content/browser/renderer_host, content/renderer, public/web, and
Source/web.
yosin@ for core/editing
bokan@ for core/input
sadrul@ for ui/base

[aelias_OOO_until_Jul13, 2017-05-09 21:30:10]

Looks OK, but could you write a unit test for it?

[commit-bot: I haz the power, 2017-05-09 22:49:25]

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

[commit-bot: I haz the power, 2017-05-09 22:49:26]

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

[bokan, 2017-05-09 23:21:36]

IMO, the real issue here is that we're doing the typo selection on context menu
which isn't always what we want. Could we move that instead to happen on right
click?

https://codereview.chromium.org/2864833002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/2864833002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:1826:
host_->ShowContextMenuAtPoint(point, true);
I'm not sure I understand, why are all context menus on Android always from a
touch handle?

https://codereview.chromium.org/2864833002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):

https://codereview.chromium.org/2864833002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/SelectionController.cpp:1052:
!mev.Event().FromTouch())
This would inadvertently affect context menus coming from a long press too I
think.

[amaralp, 2017-05-09 23:37:01]

On 2017/05/09 at 23:21:36, bokan wrote:
> IMO, the real issue here is that we're doing the typo selection on context
menu which isn't always what we want. Could we move that instead to happen on
right click?
> 
That would make it so the keyboard context menu events also won't create a typo
selection. Not sure if that would be a good or bad thing.

>
https://codereview.chromium.org/2864833002/diff/40001/content/browser/rendere...
> File content/browser/renderer_host/render_widget_host_view_android.cc (right):
> 
>
https://codereview.chromium.org/2864833002/diff/40001/content/browser/rendere...
> content/browser/renderer_host/render_widget_host_view_android.cc:1826:
host_->ShowContextMenuAtPoint(point, true);
> I'm not sure I understand, why are all context menus on Android always from a
touch handle?
>
Android only calls this function for touch handle context menus. Long press
context menus are handled differently.
 
>
https://codereview.chromium.org/2864833002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):
> 
>
https://codereview.chromium.org/2864833002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/SelectionController.cpp:1052:
!mev.Event().FromTouch())
> This would inadvertently affect context menus coming from a long press too I
think.
I don't think this would be a problem for long press because if you long press
any word (doesn't matter if it is misspelled) it will select the word.

[yosin_UTC9, 2017-05-10 04:39:50]

https://codereview.chromium.org/2864833002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/2864833002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:1826:
host_->ShowContextMenuAtPoint(point, true);
On 2017/05/09 at 23:21:36, bokan wrote:
> I'm not sure I understand, why are all context menus on Android always from a
touch handle?

Agree with bokan@. We want to know how context menu invoked, e.g. long tap,
selection handle tap, etc.
It seems pass around a reason of context menu invocation is better than |bool|.

https://codereview.chromium.org/2864833002/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/web/WebView.h (right):

https://codereview.chromium.org/2864833002/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/web/WebView.h:393: virtual void ShowContextMenu(bool
from_touch_handle) = 0;
It seems pass around a reason of context menu invocation is better than |bool|.

[bokan, 2017-05-10 13:16:53]

On 2017/05/09 23:37:01, amaralp wrote:
> On 2017/05/09 at 23:21:36, bokan wrote:
> > IMO, the real issue here is that we're doing the typo selection on context
> menu which isn't always what we want. Could we move that instead to happen on
> right click?
> > 
> That would make it so the keyboard context menu events also won't create a
typo
> selection. Not sure if that would be a good or bad thing.

Right, forgot the keyboard context menu can happen in an editing context. Ok,
scratch that.

[bokan, 2017-05-10 13:23:02]

On 2017/05/10 04:39:50, yosin_UTC9 wrote:
>
https://codereview.chromium.org/2864833002/diff/40001/content/browser/rendere...
> File content/browser/renderer_host/render_widget_host_view_android.cc (right):
> 
>
https://codereview.chromium.org/2864833002/diff/40001/content/browser/rendere...
> content/browser/renderer_host/render_widget_host_view_android.cc:1826:
> host_->ShowContextMenuAtPoint(point, true);
> On 2017/05/09 at 23:21:36, bokan wrote:
> > I'm not sure I understand, why are all context menus on Android always from
a
> touch handle?
> 
> Agree with bokan@. We want to know how context menu invoked, e.g. long tap,
> selection handle tap, etc.
> It seems pass around a reason of context menu invocation is better than
|bool|.
> 
>
https://codereview.chromium.org/2864833002/diff/40001/third_party/WebKit/publ...
> File third_party/WebKit/public/web/WebView.h (right):
> 
>
https://codereview.chromium.org/2864833002/diff/40001/third_party/WebKit/publ...
> third_party/WebKit/public/web/WebView.h:393: virtual void ShowContextMenu(bool
> from_touch_handle) = 0;
> It seems pass around a reason of context menu invocation is better than
|bool|.

That or make the bool "affect_spellcheck_selection". That'd be more explicit and
make more sense as an API parameter.

[amaralp, 2017-05-15 21:41:31]

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

[commit-bot: I haz the power, 2017-05-15 21:42:11]

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-05-15 21:50:19]

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

[commit-bot: I haz the power, 2017-05-15 21:50:20]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[amaralp, 2017-05-15 22:25:14]

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

[commit-bot: I haz the power, 2017-05-15 22:26:15]

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-05-15 23:39:56]

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

[commit-bot: I haz the power, 2017-05-15 23:39:57]

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

[amaralp, 2017-05-17 23:36:23]

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

[commit-bot: I haz the power, 2017-05-17 23:37:22]

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-05-18 02:08:42]

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

[commit-bot: I haz the power, 2017-05-18 02:08:43]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[amaralp, 2017-05-18 17:54:23]

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

[commit-bot: I haz the power, 2017-05-18 17:55:20]

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-05-18 18:01:23]

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

[commit-bot: I haz the power, 2017-05-18 18:01:24]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[amaralp, 2017-05-18 18:15:00]

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

[commit-bot: I haz the power, 2017-05-18 18:16:21]

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-05-18 21:48:11]

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

[commit-bot: I haz the power, 2017-05-18 21:48:12]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[amaralp, 2017-05-18 23:28:57]

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

[commit-bot: I haz the power, 2017-05-18 23:30:47]

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-05-19 03:57:21]

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

[commit-bot: I haz the power, 2017-05-19 03:57:22]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[amaralp, 2017-05-22 21:15:40]

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

[commit-bot: I haz the power, 2017-05-22 21:16:04]

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-05-22 23:05:54]

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

[commit-bot: I haz the power, 2017-05-22 23:05:55]

Dry run: This issue passed the CQ dry run.

[amaralp, 2017-05-22 23:08:26]

On 2017/05/10 at 13:23:02, bokan wrote:
> On 2017/05/10 04:39:50, yosin_UTC9 wrote:
> >
https://codereview.chromium.org/2864833002/diff/40001/content/browser/rendere...
> > File content/browser/renderer_host/render_widget_host_view_android.cc
(right):
> > 
> >
https://codereview.chromium.org/2864833002/diff/40001/content/browser/rendere...
> > content/browser/renderer_host/render_widget_host_view_android.cc:1826:
> > host_->ShowContextMenuAtPoint(point, true);
> > On 2017/05/09 at 23:21:36, bokan wrote:
> > > I'm not sure I understand, why are all context menus on Android always
from a
> > touch handle?
> > 
> > Agree with bokan@. We want to know how context menu invoked, e.g. long tap,
> > selection handle tap, etc.
> > It seems pass around a reason of context menu invocation is better than
|bool|.
> > 
> >
https://codereview.chromium.org/2864833002/diff/40001/third_party/WebKit/publ...
> > File third_party/WebKit/public/web/WebView.h (right):
> > 
> >
https://codereview.chromium.org/2864833002/diff/40001/third_party/WebKit/publ...
> > third_party/WebKit/public/web/WebView.h:393: virtual void
ShowContextMenu(bool
> > from_touch_handle) = 0;
> > It seems pass around a reason of context menu invocation is better than
|bool|.
> 
> That or make the bool "affect_spellcheck_selection". That'd be more explicit
and make more sense as an API parameter.
This will be used a later CL for things other than spell_check so it can't be
that specific. I am now passing
around the cause of the context menu. PTAL

[bokan, 2017-05-23 14:06:02]

https://codereview.chromium.org/2864833002/diff/180001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/2864833002/diff/180001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_android.cc:1861:
host_->ShowContextMenuAtPoint(point, ui::MENU_SOURCE_TOUCH_HANDLE);
Could we plumb this value up so that we specify TOUCH_HANDLE at the place where
we're actually dealing with touch handles?

https://codereview.chromium.org/2864833002/diff/180001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_base.cc (right):

https://codereview.chromium.org/2864833002/diff/180001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_base.cc:219:
impl->ShowContextMenuAtPoint(point, ui::MENU_SOURCE_MOUSE);
I think this shouldn't be mouse - I'm assuming Accessibility means it could come
from accessibility tools which might be voice activated or some other modality.
Lets just use MENU_SOURCE_NONE.

https://codereview.chromium.org/2864833002/diff/180001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WebMenuSourceType.h (right):

https://codereview.chromium.org/2864833002/diff/180001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebMenuSourceType.h:35: enum WebMenuSourceType {
Please add assertions that these match the UI version of the enum in
content/child/assert_matching_enums.cc

[amaralp, 2017-05-23 19:37:53]

https://codereview.chromium.org/2864833002/diff/180001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/2864833002/diff/180001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_android.cc:1861:
host_->ShowContextMenuAtPoint(point, ui::MENU_SOURCE_TOUCH_HANDLE);
On 2017/05/23 at 14:06:02, bokan wrote:
> Could we plumb this value up so that we specify TOUCH_HANDLE at the place
where we're actually dealing with touch handles?

I changed the name of the functions from ShowContextMenuAtPoint to
ShowContextMenuAtTouchHandle since they are only used
for this purpose.

https://codereview.chromium.org/2864833002/diff/180001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_base.cc (right):

https://codereview.chromium.org/2864833002/diff/180001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_base.cc:219:
impl->ShowContextMenuAtPoint(point, ui::MENU_SOURCE_MOUSE);
On 2017/05/23 at 14:06:02, bokan wrote:
> I think this shouldn't be mouse - I'm assuming Accessibility means it could
come from accessibility tools which might be voice activated or some other
modality. Lets just use MENU_SOURCE_NONE.

Done.

https://codereview.chromium.org/2864833002/diff/180001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WebMenuSourceType.h (right):

https://codereview.chromium.org/2864833002/diff/180001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebMenuSourceType.h:35: enum WebMenuSourceType {
On 2017/05/23 at 14:06:02, bokan wrote:
> Please add assertions that these match the UI version of the enum in
content/child/assert_matching_enums.cc

Done.

[yosin_UTC9, 2017-05-24 05:08:39]

https://codereview.chromium.org/2864833002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):

https://codereview.chromium.org/2864833002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/SelectionController.cpp:1050: if
(HitTestResultIsMisspelled(mev.GetHitTestResult()) &&
It is better to check |FromTouch()| before calling |HitTestResultIsMisspelled()|
to reduce CPU usage.

https://codereview.chromium.org/2864833002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WebMenuSourceType.h (right):

https://codereview.chromium.org/2864833002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebMenuSourceType.h:1: /*
Please use the current copyright banner if you introduce |WebMenuSourceType|.

// Copyright 2017 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.

https://codereview.chromium.org/2864833002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebMenuSourceType.h:35: enum WebMenuSourceType {
nit: It is better to have a blank line after |namespace blink|,
since we have a blank line before closing brace.

[amaralp1, 2017-05-24 05:33:26]

amaralp@google.com changed reviewers:
+ amaralp@google.com

[amaralp1, 2017-05-24 05:33:28]

https://codereview.chromium.org/2864833002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):

https://codereview.chromium.org/2864833002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/SelectionController.cpp:1050: if
(HitTestResultIsMisspelled(mev.GetHitTestResult()) &&
On 2017/05/24 at 05:08:38, yosin_UTC9 wrote:
> It is better to check |FromTouch()| before calling
|HitTestResultIsMisspelled()|
> to reduce CPU usage.

Done.

https://codereview.chromium.org/2864833002/diff/200001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WebMenuSourceType.h (right):

https://codereview.chromium.org/2864833002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebMenuSourceType.h:1: /*
On 2017/05/24 at 05:08:38, yosin_UTC9 wrote:
> Please use the current copyright banner if you introduce |WebMenuSourceType|.
> 
> // Copyright 2017 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.

Done.

https://codereview.chromium.org/2864833002/diff/200001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebMenuSourceType.h:35: enum WebMenuSourceType {
On 2017/05/24 at 05:08:38, yosin_UTC9 wrote:
> nit: It is better to have a blank line after |namespace blink|,
> since we have a blank line before closing brace.

Done.

[amaralp1, 2017-05-24 17:48:56]

bokan@ and aelias@ do you have any more comments?

[bokan, 2017-05-24 18:28:22]

lgtm

https://codereview.chromium.org/2864833002/diff/180001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/2864833002/diff/180001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_android.cc:1861:
host_->ShowContextMenuAtPoint(point, ui::MENU_SOURCE_TOUCH_HANDLE);
On 2017/05/23 19:37:53, amaralp wrote:
> On 2017/05/23 at 14:06:02, bokan wrote:
> > Could we plumb this value up so that we specify TOUCH_HANDLE at the place
> where we're actually dealing with touch handles?
> 
> I changed the name of the functions from ShowContextMenuAtPoint to
> ShowContextMenuAtTouchHandle since they are only used
> for this purpose.

Acknowledged.

[amaralp, 2017-05-24 19:09:53]

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

[commit-bot: I haz the power, 2017-05-24 19:10:57]

Dry run: CQ is trying da patch.

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

[aelias_OOO_until_Jul13, 2017-05-24 21:38:51]

lgtm

[amaralp, 2017-05-24 23:29:54]

amaralp@chromium.org changed reviewers:
+ pfeldman@chromium.org, tedchoc@chromium.org

[amaralp, 2017-05-24 23:29:55]

Adding more OWNERS

tedchoc@: content/browser/web_contents
sadrul@: ui/base/ui_base_types.h
pfeldman@: content/child/assert_matching_enums.cc

[Ted C, 2017-05-24 23:31:02]

On 2017/05/24 23:29:55, amaralp wrote:
> Adding more OWNERS
> 
> tedchoc@: content/browser/web_contents
> sadrul@: ui/base/ui_base_types.h
> pfeldman@: content/child/assert_matching_enums.cc

content/browser/web_contents/web_contents_android* - lgtm

[commit-bot: I haz the power, 2017-05-24 23:57:09]

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

[commit-bot: I haz the power, 2017-05-24 23:57:10]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[sadrul, 2017-05-25 04:13:50]

https://codereview.chromium.org/2864833002/diff/220001/content/browser/render...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/2864833002/diff/220001/content/browser/render...
content/browser/renderer_host/render_widget_host_impl.cc:1333: Send(new
ViewMsg_ShowContextMenu(GetRoutingID(), source_type, point));
Can you update aura code [1] to call into ShowContextMenuAtPoint() now, instead
of directly sending the IPC message?

[1]
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/touc...

https://codereview.chromium.org/2864833002/diff/220001/ui/base/ui_base_types.h
File ui/base/ui_base_types.h (right):

https://codereview.chromium.org/2864833002/diff/220001/ui/base/ui_base_types....
ui/base/ui_base_types.h:51: MENU_SOURCE_TOUCH_HANDLE    = 7,
Can you just use MENU_SOURCE_TOUCH_EDIT_MENU? Because that's what aura uses for
this.

[yosin_UTC9, 2017-05-25 04:30:53]

lgtm for core/editing

[amaralp1, 2017-05-25 04:39:53]

https://codereview.chromium.org/2864833002/diff/220001/content/browser/render...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/2864833002/diff/220001/content/browser/render...
content/browser/renderer_host/render_widget_host_impl.cc:1333: Send(new
ViewMsg_ShowContextMenu(GetRoutingID(), source_type, point));
On 2017/05/25 at 04:13:50, sadrul wrote:
> Can you update aura code [1] to call into ShowContextMenuAtPoint() now,
instead of directly sending the IPC message?
> 
> [1]
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/touc...

Done.

https://codereview.chromium.org/2864833002/diff/220001/ui/base/ui_base_types.h
File ui/base/ui_base_types.h (right):

https://codereview.chromium.org/2864833002/diff/220001/ui/base/ui_base_types....
ui/base/ui_base_types.h:51: MENU_SOURCE_TOUCH_HANDLE    = 7,
On 2017/05/25 at 04:13:50, sadrul wrote:
> Can you just use MENU_SOURCE_TOUCH_EDIT_MENU? Because that's what aura uses
for this.

Isn't MENU_SOURCE_TOUCH_EDIT_MENU used when you tap the ellipses option in the
floating menu? This would be used for when you tap the insertion handle and have
the floating paste menu show up. Android now sends a context menu event in this
scenario and I think Aura doesn't. I think it would be better to keep these two
scenarios separate since eventually Aura may want to trigger the floating menus
with context menu events.

[amaralp, 2017-05-25 19:38:23]

amaralp@chromium.org changed reviewers:
+ avi@chromium.org
- pfeldman@chromium.org

[amaralp, 2017-05-25 19:38:25]

Adding avi@ for content/child/assert_matching_enums.cc OWNERS

[Avi (use Gerrit), 2017-05-25 20:17:20]

lgtm

[sadrul, 2017-05-25 21:24:16]

lgtm

https://codereview.chromium.org/2864833002/diff/220001/ui/base/ui_base_types.h
File ui/base/ui_base_types.h (right):

https://codereview.chromium.org/2864833002/diff/220001/ui/base/ui_base_types....
ui/base/ui_base_types.h:51: MENU_SOURCE_TOUCH_HANDLE    = 7,
On 2017/05/25 04:39:53, amaralp1 wrote:
> On 2017/05/25 at 04:13:50, sadrul wrote:
> > Can you just use MENU_SOURCE_TOUCH_EDIT_MENU? Because that's what aura uses
> for this.
> 
> Isn't MENU_SOURCE_TOUCH_EDIT_MENU used when you tap the ellipses option in the
> floating menu? This would be used for when you tap the insertion handle and
have
> the floating paste menu show up. Android now sends a context menu event in
this
> scenario and I think Aura doesn't. I think it would be better to keep these
two
> scenarios separate since eventually Aura may want to trigger the floating
menus
> with context menu events.

Thanks for the explanation. Makes sense to me.

[amaralp, 2017-05-25 21:24:56]

The CQ bit was checked by amaralp@chromium.org

[amaralp, 2017-05-25 21:24:57]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
bokan@chromium.org, tedchoc@chromium.org, yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/2864833002/#ps260001
(title: "Rebasing")

[commit-bot: I haz the power, 2017-05-25 21:25:31]

CQ is trying da patch.

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

[amaralp, 2017-05-25 21:45:48]

The CQ bit was checked by amaralp@chromium.org

[amaralp, 2017-05-25 21:45:48]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
sadrul@chromium.org, bokan@chromium.org, avi@chromium.org, tedchoc@chromium.org,
yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/2864833002/#ps280001
(title: "Fixed naming")

[commit-bot: I haz the power, 2017-05-25 21:46:40]

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-05-25 23:55:40]

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

[commit-bot: I haz the power, 2017-05-25 23:55:42]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[amaralp, 2017-05-26 00:02:24]

The CQ bit was checked by amaralp@chromium.org

[commit-bot: I haz the power, 2017-05-26 00:03:43]

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-05-26 05:00:01]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1495756944567820,
"parent_rev": "44c957b81b919e80612133fc5e9d4847252b1258", "commit_rev":
"915818ef6101ba84d48f9b359f4418f702bca8e6"}

[commit-bot: I haz the power, 2017-05-26 05:00:13]

Description was changed from

==========
Tapping handle shouldn't select misspelled word

After crrev.com/2721813002 tapping an insertion handle triggered a contextmenu
event
which led to bug 717337. This CL fixes this bug by keeping track that the
insertion
handle tap contextmenu event came from touch.

BUG=717337
==========

to

==========
Tapping handle shouldn't select misspelled word

After crrev.com/2721813002 tapping an insertion handle triggered a contextmenu
event
which led to bug 717337. This CL fixes this bug by keeping track that the
insertion
handle tap contextmenu event came from touch.

BUG=717337

Review-Url: https://codereview.chromium.org/2864833002
Cr-Commit-Position: refs/heads/master@{#474912}
Committed:
https://chromium.googlesource.com/chromium/src/+/915818ef6101ba84d48f9b359f44...
==========

[commit-bot: I haz the power, 2017-05-26 05:00:15]

Committed patchset #15 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/915818ef6101ba84d48f9b359f44...