Look Up on Force Touch

Implementation for Look Up on Force Touch.
If the user force touched a word in the web content the Look Up dialog
should pop up for the word.

BUG=481890
Committed: https://crrev.com/927f27305d77ff600a505aa33d586ca8246f1614
Cr-Commit-Position: refs/heads/master@{#368481}

[spqchan, 2015-12-16 02:36:00]

Description was changed from

==========
Implement support for Look Up on Force Touch

BUG=481890
==========

to

==========
Implementation for support for Look Up on Force Touch.

The following changes are made in this CL:
- Handle NSPressure events on the browser UI and convert them into Force Touch
events.
- Force Touch events are handled for Look Up
- Added "selectWordIfAnyAtCoordinate" methods in renderer and blink.



BUG=481890
==========

[spqchan, 2015-12-16 02:37:17]

Description was changed from

==========
Implementation for support for Look Up on Force Touch.

The following changes are made in this CL:
- Handle NSPressure events on the browser UI and convert them into Force Touch
events.
- Force Touch events are handled for Look Up
- Added "selectWordIfAnyAtCoordinate" methods in renderer and blink.



BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.

The following changes are made in this CL:
- Added support for NSPressure events in the browser UI and by converting them
into Force Touch events.
- The browser thread then handles the UI 
- Added "selectWordIfAnyAtCoordinate" methods in renderer and blink.



BUG=481890
==========

[spqchan, 2015-12-16 02:39:33]

Description was changed from

==========
Implementation for Look Up on Force Touch.

The following changes are made in this CL:
- Added support for NSPressure events in the browser UI and by converting them
into Force Touch events.
- The browser thread then handles the UI 
- Added "selectWordIfAnyAtCoordinate" methods in renderer and blink.



BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.

In this CL:
- Browser UI handles NSPressure events by converting them into Force Touch
events
- 


The following changes are made in this CL:
- Added support for NSPressure events in the browser UI and by converting them
into Force Touch events.
- The browser thread then handles the UI 
- Added "selectWordIfAnyAtCoordinate" methods in renderer and blink.



BUG=481890
==========

[spqchan, 2015-12-16 02:42:20]

Description was changed from

==========
Implementation for Look Up on Force Touch.

In this CL:
- Browser UI handles NSPressure events by converting them into Force Touch
events
- 


The following changes are made in this CL:
- Added support for NSPressure events in the browser UI and by converting them
into Force Touch events.
- The browser thread then handles the UI 
- Added "selectWordIfAnyAtCoordinate" methods in renderer and blink.



BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.

Quick rundown of the implementation
- Browser UI handles NSPressure events by converting them into Force
 Touch events in base_view. 
- RenderWidgetHostView will then handle the Force Touch event by
- 

 render_widget_host_view


The following changes are made in this CL:
- Added support for NSPressure events in the browser UI and by converting them
into Force Touch events.
- The browser thread then handles the UI 
- Added "selectWordIfAnyAtCoordinate" methods in renderer and blink.



BUG=481890
==========

[spqchan, 2015-12-16 02:46:50]

Description was changed from

==========
Implementation for Look Up on Force Touch.

Quick rundown of the implementation
- Browser UI handles NSPressure events by converting them into Force
 Touch events in base_view. 
- RenderWidgetHostView will then handle the Force Touch event by
- 

 render_widget_host_view


The following changes are made in this CL:
- Added support for NSPressure events in the browser UI and by converting them
into Force Touch events.
- The browser thread then handles the UI 
- Added "selectWordIfAnyAtCoordinate" methods in renderer and blink.



BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If Force Touch occurred on a word in the web content, the word should 
be selected and the Look Up dialog should popup.

Quick rundown of the implementation
- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordIfAnyAtCoordinates" message to the 
  renderer to select a word, if there's any.
- Renderer calls on blink's API "selectWordIfAnyAtCoordinates" to
   select the text



BUG=481890
==========

[spqchan, 2015-12-16 02:53:56]

Description was changed from

==========
Implementation for Look Up on Force Touch.
If Force Touch occurred on a word in the web content, the word should 
be selected and the Look Up dialog should popup.

Quick rundown of the implementation
- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordIfAnyAtCoordinates" message to the 
  renderer to select a word, if there's any.
- Renderer calls on blink's API "selectWordIfAnyAtCoordinates" to
   select the text



BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If Force Touch occurred on a word in the web content, the word should 
be selected and the Look Up dialog should popup.

Quick rundown of the implementation
- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordIfAnyAtCoordinates" message to the 
  renderer to select a word, if there's any.
- Renderer calls on blink's API "selectWordIfAnyAtCoordinates" to
   select the text. If text is selected, it'll send a  
   DidSelectWordAtCoordinate message which will cause the browser to
   show the LookUp dialog



BUG=481890
==========

[spqchan, 2015-12-16 02:54:45]

Description was changed from

==========
Implementation for Look Up on Force Touch.
If Force Touch occurred on a word in the web content, the word should 
be selected and the Look Up dialog should popup.

Quick rundown of the implementation
- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordIfAnyAtCoordinates" message to the 
  renderer to select a word, if there's any.
- Renderer calls on blink's API "selectWordIfAnyAtCoordinates" to
   select the text. If text is selected, it'll send a  
   DidSelectWordAtCoordinate message which will cause the browser to
   show the LookUp dialog



BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content, the word should 
be selected and the Look Up dialog should pop up.

Quick rundown of the implementation
- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordIfAnyAtCoordinates" message to the 
  renderer to select a word, if there's any.
- Renderer calls on blink's API "selectWordIfAnyAtCoordinates" to
   select the text. If text is selected, it'll send a  
   DidSelectWordAtCoordinate message which will cause the browser to
   show the LookUp dialog



BUG=481890
==========

[spqchan, 2015-12-16 02:55:10]

spqchan@chromium.org changed reviewers:
+ ccameron@chromium.org, rbyers@chromium.org, rsesek@chromium.org

[spqchan, 2015-12-16 02:59:30]

Description was changed from

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content, the word should 
be selected and the Look Up dialog should pop up.

Quick rundown of the implementation
- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordIfAnyAtCoordinates" message to the 
  renderer to select a word, if there's any.
- Renderer calls on blink's API "selectWordIfAnyAtCoordinates" to
   select the text. If text is selected, it'll send a  
   DidSelectWordAtCoordinate message which will cause the browser to
   show the LookUp dialog



BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content, the word should 
be selected and the Look Up dialog should pop up.

Quick rundown of the implementation
- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordIfAnyAtCoordinates" message to the 
  renderer to select a word, if there's any.
- Renderer calls on blink's API "selectWordIfAnyAtCoordinates" to
   select the text. If text is selected, it'll send a  
   DidSelectWordAtCoordinate message which will cause the browser to
   show the LookUp dialog


BUG=481890
==========

[spqchan, 2015-12-16 03:01:22]

+ rbyers for changes in blink

+ rsesek for changes in base_view

+ ccameron for changes involving the renderer


PTAL

[Rick Byers, 2015-12-21 16:31:09]

The architecture of the blink side now looks great (nice and contained - pretty
simple) - thanks!  I just have a few minor suggestions.

/cc yosin@ (editing/OWNER).  He (and all other editing/OWNERS) are out until Jan
5th, so I wouldn't block on them (but if he's got suggestions you can always
land a follow-up patch).

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/SelectionController.cpp:550:
m_frame->document()->layoutView()->hitTest(result);
You just did a hit-test up in WebViewImpl, it's a shame to do another one (hit
tests are expensive).  Can you instead do a single hit-test in WebViewImpl and
take a HitTestResults in your SelectionController API?  That would also make it
a lot closer to existing APIs like selectClosestWordFromMouseEvent and
handleMousePressEventDoubleClick.  I think there's been some effort to eliminate
unnecessary hit tests from inside the editing code.

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebViewImpl.cpp:2265: // Find the right target
frame. See issue 1186900.
This comment seems to used elsewhere in android-specific code (and indeed refers
to a google internal bug number for early chrome-android work).  You can
probably just remove reference to the issue (but see below).

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/public/w...
File third_party/WebKit/public/web/WebWidget.h (right):

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/WebWidget.h:128: virtual bool
selectWordIfAnyAtCoordinates(float x, float y)
Looking at this in more detail, it seems that it probably fits better into
WebView than WebWidget - would you mind moving it?  Eg. it seems to fit with the
other "data exchange" methods like "saveImageAt".

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/WebWidget.h:128: virtual bool
selectWordIfAnyAtCoordinates(float x, float y)
For consistency with similar APIs, please use WebPoint as the argument type
instead of taking two bare floats, and then name it something like
"selectWordIfAnyAt".

WebFloatPoint is also an option, but mouse input is currently handled in
integers (and it looks like you're converting back and forth between floats and
ints) so WebPoint is probably better.

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/WebWidget.h:128: virtual bool
selectWordIfAnyAtCoordinates(float x, float y)
please add a short description (we try to keep the blink public API pretty well
documented).  For example, you need to say what the return value indicates.

[Rick Byers, 2015-12-21 16:59:48]

Oh also, you'll need a unit test of the new blink API.  You can either unit-test
the C++ directly, eg. in Source/web/tests/WebViewTest.cpp, or you write a
"LayoutTest" in JavaScript by exposing your new API in Internals.idl.  I suspect
the former (WebViewTest) is probably simpler in your case.  Please include at
least one test case where the word being selected is inside of an iframe.

[Robert Sesek, 2015-12-21 22:30:22]

ui/base LGTM

https://codereview.chromium.org/1528153004/diff/1/content/common/input_messag...
File content/common/input_messages.h (right):

https://codereview.chromium.org/1528153004/diff/1/content/common/input_messag...
content/common/input_messages.h:122: float /* x */,
I defer to the more experienced input reviewers, but should this be a
gfx::PointF (or gfx::Point, since this gets truncated to an IntPoint in Blink)?

https://codereview.chromium.org/1528153004/diff/1/ui/base/cocoa/base_view.mm
File ui/base/cocoa/base_view.mm (right):

https://codereview.chromium.org/1528153004/diff/1/ui/base/cocoa/base_view.mm#...
ui/base/cocoa/base_view.mm:179: // 2, which is the value for force touch
nit: punctuation.

[yosin_UTC9, 2016-01-05 04:12:58]

yosin@chromium.org changed reviewers:
+ yosin@chromium.org

[yosin_UTC9, 2016-01-05 04:12:58]

I don't have enough context about Force Touch handling in Blink.
But, I feel that it is too generic to exposing select word at point in WebView.
Can we handle force touch as other touch event handling in EventHandler?
"selecting word" for
force touch seems to be like a default action. Web authors may want to action
for force touch
like other events.

[Rick Byers, 2016-01-05 16:22:56]

On 2016/01/05 04:12:58, Yosi_UTC9 wrote:
> I don't have enough context about Force Touch handling in Blink.
> But, I feel that it is too generic to exposing select word at point in
WebView.
> Can we handle force touch as other touch event handling in EventHandler?
> "selecting word" for
> force touch seems to be like a default action. Web authors may want to action
> for force touch
> like other events.

"Force touch" is a Mac-specific concept that we don't plan to expose to the web
in any way (Safari has some webkit-prefixed APIs but there's no plans for
anything standard).  So, IMHO, from blink's perspective this has nothing to do
with "force touch" or necessarily even "mouse" - it's just that the UI wants to
trigger it's "lookup word" feature at a specific point.

We could define this as a new type of mouse event (spqchan@ started with
something more like that), but it would be weird for blink because it would be a
type of mouse event that exists only for Mac.  I thought it was better to leave
all that at the UI layer and just have the UI layer tell blink what it wants
done.  But if you feel strongly that "select word as point" is too low-level to
expose from blink, then I'd also be OK with using a new type of mouse event and
having EventHandler.cpp implement that directly by calling into selection code.

WDYT?

[spqchan, 2016-01-06 22:52:31]

I just got back and caught up with the comments. 

I agree with rbyers@ regarding Force Touch in blink.
Adding force touch handling in blink would be unnecessary, since there's nothing
we want to do with it beyond selecting a word.
Otherwise, I'll be fine with handling Force Touch as a mouse event.

Also, I'm working on creating a new unit test for this API

https://codereview.chromium.org/1528153004/diff/1/content/common/input_messag...
File content/common/input_messages.h (right):

https://codereview.chromium.org/1528153004/diff/1/content/common/input_messag...
content/common/input_messages.h:122: float /* x */,
On 2015/12/21 22:30:21, Robert Sesek wrote:
> I defer to the more experienced input reviewers, but should this be a
> gfx::PointF (or gfx::Point, since this gets truncated to an IntPoint in
Blink)?

Changed to WebPoint

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/SelectionController.cpp:550:
m_frame->document()->layoutView()->hitTest(result);
On 2015/12/21 16:31:08, Rick Byers wrote:
> You just did a hit-test up in WebViewImpl, it's a shame to do another one (hit
> tests are expensive).  Can you instead do a single hit-test in WebViewImpl and
> take a HitTestResults in your SelectionController API?  That would also make
it
> a lot closer to existing APIs like selectClosestWordFromMouseEvent and
> handleMousePressEventDoubleClick.  I think there's been some effort to
eliminate
> unnecessary hit tests from inside the editing code.

Done.

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebViewImpl.cpp:2265: // Find the right target
frame. See issue 1186900.
On 2015/12/21 16:31:09, Rick Byers wrote:
> This comment seems to used elsewhere in android-specific code (and indeed
refers
> to a google internal bug number for early chrome-android work).  You can
> probably just remove reference to the issue (but see below).

I removed it but I'm a bit confused by what you mean by "but see below".

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/public/w...
File third_party/WebKit/public/web/WebWidget.h (right):

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/WebWidget.h:128: virtual bool
selectWordIfAnyAtCoordinates(float x, float y)
On 2015/12/21 16:31:09, Rick Byers wrote:
> Looking at this in more detail, it seems that it probably fits better into
> WebView than WebWidget - would you mind moving it?  Eg. it seems to fit with
the
> other "data exchange" methods like "saveImageAt".

Sure thing. I also moved the code from render_widget to render_view_impl in
addition to this.

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/WebWidget.h:128: virtual bool
selectWordIfAnyAtCoordinates(float x, float y)
On 2015/12/21 16:31:09, Rick Byers wrote:
> For consistency with similar APIs, please use WebPoint as the argument type
> instead of taking two bare floats, and then name it something like
> "selectWordIfAnyAt".
> 
> WebFloatPoint is also an option, but mouse input is currently handled in
> integers (and it looks like you're converting back and forth between floats
and
> ints) so WebPoint is probably better.

Done, switched to WebPoint

https://codereview.chromium.org/1528153004/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/WebWidget.h:128: virtual bool
selectWordIfAnyAtCoordinates(float x, float y)
On 2015/12/21 16:31:09, Rick Byers wrote:
> please add a short description (we try to keep the blink public API pretty
well
> documented).  For example, you need to say what the return value indicates.

Done.

https://codereview.chromium.org/1528153004/diff/1/ui/base/cocoa/base_view.mm
File ui/base/cocoa/base_view.mm (right):

https://codereview.chromium.org/1528153004/diff/1/ui/base/cocoa/base_view.mm#...
ui/base/cocoa/base_view.mm:179: // 2, which is the value for force touch
On 2015/12/21 22:30:21, Robert Sesek wrote:
> nit: punctuation.

Done.

[yosin_UTC9, 2016-01-07 08:45:53]

On 2016/01/05 at 16:22:56, rbyers wrote:
> On 2016/01/05 04:12:58, Yosi_UTC9 wrote:
> > I don't have enough context about Force Touch handling in Blink.
> > But, I feel that it is too generic to exposing select word at point in
WebView.
> > Can we handle force touch as other touch event handling in EventHandler?
> > "selecting word" for
> > force touch seems to be like a default action. Web authors may want to
action
> > for force touch
> > like other events.
> 
> "Force touch" is a Mac-specific concept that we don't plan to expose to the
web in any way (Safari has some webkit-prefixed APIs but there's no plans for
anything standard).  So, IMHO, from blink's perspective this has nothing to do
with "force touch" or necessarily even "mouse" - it's just that the UI wants to
trigger it's "lookup word" feature at a specific point.
> 
> We could define this as a new type of mouse event (spqchan@ started with
something more like that), but it would be weird for blink because it would be a
type of mouse event that exists only for Mac.  I thought it was better to leave
all that at the UI layer and just have the UI layer tell blink what it wants
done.  But if you feel strongly that "select word as point" is too low-level to
expose from blink, then I'd also be OK with using a new type of mouse event and
having EventHandler.cpp implement that directly by calling into selection code.
> 
> WDYT?

I understand "Force Touch" is Mac-only and won't be exposed to web. I agree that
we don't want to have an Mac-specific event.

[yosin_UTC9, 2016-01-07 08:47:02]

https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebViewImpl.h (right):

https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebViewImpl.h:236: bool selectWordIfAnyAt(const
WebPoint&) override;
Can we use WebFrame::moveCaretSelection(const WebPoint&) and
WebFrame::selectWordAroundCaret() to select a word at a point?

[Rick Byers, 2016-01-07 14:33:16]

On 2016/01/07 08:47:02, Yosi_UTC9 wrote:
>
https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/web/WebViewImpl.h (right):
> 
>
https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/web/WebViewImpl.h:236: bool selectWordIfAnyAt(const
> WebPoint&) override;
> Can we use WebFrame::moveCaretSelection(const WebPoint&) and
> WebFrame::selectWordAroundCaret() to select a word at a point?

Oh, interesting - I didn't realize those existed (I wasn't looking on WebFrame).
 The UI layer really doesn't want to need to reason about individual frames (I
suspect esprehn@ would say this is one of those details that blink should be
completely abstracting away from contents).  Eg. how it content supposed to know
WHICH frame to change?

It looks like RenderViewImpl::OnMoveCaret and OnSelectWordAroundCaret rely on
the notion of the "currently focused frame" (used for touch text selection, and
contextual search). Perhaps this works because touchstart will set the focused
frame.  Regardless I'm fairly sure that mousedown sets the focused frame (and
doesn't mousedown also typically move the caret?  Though probably not if it's
canceled by JavaScript).

So Sarah could possibly use the existing RenderWidgetHostImpl::MoveCaret and
RenderWidgetHost::SelectWordAroundCaret methods for her use case rather than add
ANYTHING new on the renderer side.  Using more of the same logic as contextual
search makes a lot of sense, since it's a similar feature in some ways.  Sarah,
can you please give this a try?  Sorry I didn't see this option sooner - I
looked around for some existing functions, but missed these ones.  Again, if
this works it should make your CL even simpler, so I think it's worth the effort
:-)

[spqchan, 2016-01-07 17:40:29]

On 2016/01/07 14:33:16, Rick Byers wrote:
> On 2016/01/07 08:47:02, Yosi_UTC9 wrote:
> >
>
https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/web/WebViewImpl.h (right):
> > 
> >
>
https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/web/WebViewImpl.h:236: bool
selectWordIfAnyAt(const
> > WebPoint&) override;
> > Can we use WebFrame::moveCaretSelection(const WebPoint&) and
> > WebFrame::selectWordAroundCaret() to select a word at a point?
> 
> Oh, interesting - I didn't realize those existed (I wasn't looking on
WebFrame).
>  The UI layer really doesn't want to need to reason about individual frames (I
> suspect esprehn@ would say this is one of those details that blink should be
> completely abstracting away from contents).  Eg. how it content supposed to
know
> WHICH frame to change?
> 
> It looks like RenderViewImpl::OnMoveCaret and OnSelectWordAroundCaret rely on
> the notion of the "currently focused frame" (used for touch text selection,
and
> contextual search). Perhaps this works because touchstart will set the focused
> frame.  Regardless I'm fairly sure that mousedown sets the focused frame (and
> doesn't mousedown also typically move the caret?  Though probably not if it's
> canceled by JavaScript).
> 
> So Sarah could possibly use the existing RenderWidgetHostImpl::MoveCaret and
> RenderWidgetHost::SelectWordAroundCaret methods for her use case rather than
add
> ANYTHING new on the renderer side.  Using more of the same logic as contextual
> search makes a lot of sense, since it's a similar feature in some ways. 
Sarah,
> can you please give this a try?  Sorry I didn't see this option sooner - I
> looked around for some existing functions, but missed these ones.  Again, if
> this works it should make your CL even simpler, so I think it's worth the
effort
> :-)

Sure thing! I'll give it a go and will update on this

[spqchan, 2016-01-08 00:51:32]

Description was changed from

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content, the word should 
be selected and the Look Up dialog should pop up.

Quick rundown of the implementation
- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordIfAnyAtCoordinates" message to the 
  renderer to select a word, if there's any.
- Renderer calls on blink's API "selectWordIfAnyAtCoordinates" to
   select the text. If text is selected, it'll send a  
   DidSelectWordAtCoordinate message which will cause the browser to
   show the LookUp dialog


BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content, the word should 
be selected and the Look Up dialog should pop up.

Quick rundown:
- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordAroundCaret" message to the 
  renderer.
- When it receives an ACK for the message from the renderer,
  RenderWidgetHostView will then show the lookup dialog if a word was
  selected


BUG=481890
==========

[spqchan, 2016-01-08 01:08:29]

Description was changed from

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content, the word should 
be selected and the Look Up dialog should pop up.

Quick rundown:
- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordAroundCaret" message to the 
  renderer.
- When it receives an ACK for the message from the renderer,
  RenderWidgetHostView will then show the lookup dialog if a word was
  selected


BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content, the word should 
be selected and the Look Up dialog should pop up.

- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordAroundCaret" message to the 
  renderer.
- When it receives an ACK for the message from the renderer,
  RenderWidgetHostView will then show the lookup dialog if a word was
  selected

BUG=481890
==========

[spqchan, 2016-01-08 01:11:12]

On 2016/01/07 17:40:29, spqchan (out until Jan 5) wrote:
> On 2016/01/07 14:33:16, Rick Byers wrote:
> > On 2016/01/07 08:47:02, Yosi_UTC9 wrote:
> > >
> >
>
https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/web/WebViewImpl.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/web/WebViewImpl.h:236: bool
> selectWordIfAnyAt(const
> > > WebPoint&) override;
> > > Can we use WebFrame::moveCaretSelection(const WebPoint&) and
> > > WebFrame::selectWordAroundCaret() to select a word at a point?
> > 
> > Oh, interesting - I didn't realize those existed (I wasn't looking on
> WebFrame).
> >  The UI layer really doesn't want to need to reason about individual frames
(I
> > suspect esprehn@ would say this is one of those details that blink should be
> > completely abstracting away from contents).  Eg. how it content supposed to
> know
> > WHICH frame to change?
> > 
> > It looks like RenderViewImpl::OnMoveCaret and OnSelectWordAroundCaret rely
on
> > the notion of the "currently focused frame" (used for touch text selection,
> and
> > contextual search). Perhaps this works because touchstart will set the
focused
> > frame.  Regardless I'm fairly sure that mousedown sets the focused frame
(and
> > doesn't mousedown also typically move the caret?  Though probably not if
it's
> > canceled by JavaScript).
> > 
> > So Sarah could possibly use the existing RenderWidgetHostImpl::MoveCaret and
> > RenderWidgetHost::SelectWordAroundCaret methods for her use case rather than
> add
> > ANYTHING new on the renderer side.  Using more of the same logic as
contextual
> > search makes a lot of sense, since it's a similar feature in some ways. 
> Sarah,
> > can you please give this a try?  Sorry I didn't see this option sooner - I
> > looked around for some existing functions, but missed these ones.  Again, if
> > this works it should make your CL even simpler, so I think it's worth the
> effort
> > :-)
> 
> Sure thing! I'll give it a go and will update on this

I finished making the changes in the renderer to use SelectWordAroundCaret and
updated the description. With this, the code is much more simplified and changes
to blink are no longer needed. Anyway, without blink changes, feel free to
remove yourselves from the CC'ed list. Thanks a ton for your help :)!

[yosin_UTC9, 2016-01-08 04:16:52]

On 2016/01/08 at 01:11:12, spqchan wrote:
> On 2016/01/07 17:40:29, spqchan (out until Jan 5) wrote:
> > On 2016/01/07 14:33:16, Rick Byers wrote:
> > > On 2016/01/07 08:47:02, Yosi_UTC9 wrote:
> > > >
> > >
> >
https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
> > > > File third_party/WebKit/Source/web/WebViewImpl.h (right):
> > > > 
> > > >
> > >
> >
https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
> > > > third_party/WebKit/Source/web/WebViewImpl.h:236: bool
> > selectWordIfAnyAt(const
> > > > WebPoint&) override;
> > > > Can we use WebFrame::moveCaretSelection(const WebPoint&) and
> > > > WebFrame::selectWordAroundCaret() to select a word at a point?
> > > 
> > > Oh, interesting - I didn't realize those existed (I wasn't looking on
> > WebFrame).
> > >  The UI layer really doesn't want to need to reason about individual
frames (I
> > > suspect esprehn@ would say this is one of those details that blink should
be
> > > completely abstracting away from contents).  Eg. how it content supposed
to
> > know
> > > WHICH frame to change?
> > > 
> > > It looks like RenderViewImpl::OnMoveCaret and OnSelectWordAroundCaret rely
on
> > > the notion of the "currently focused frame" (used for touch text
selection,
> > and
> > > contextual search). Perhaps this works because touchstart will set the
focused
> > > frame.  Regardless I'm fairly sure that mousedown sets the focused frame
(and
> > > doesn't mousedown also typically move the caret?  Though probably not if
it's
> > > canceled by JavaScript).
> > > 
> > > So Sarah could possibly use the existing RenderWidgetHostImpl::MoveCaret
and
> > > RenderWidgetHost::SelectWordAroundCaret methods for her use case rather
than
> > add
> > > ANYTHING new on the renderer side.  Using more of the same logic as
contextual
> > > search makes a lot of sense, since it's a similar feature in some ways. 
> > Sarah,
> > > can you please give this a try?  Sorry I didn't see this option sooner - I
> > > looked around for some existing functions, but missed these ones.  Again,
if
> > > this works it should make your CL even simpler, so I think it's worth the
> > effort
> > > :-)
> > 
> > Sure thing! I'll give it a go and will update on this
> 
> I finished making the changes in the renderer to use SelectWordAroundCaret and
updated the description. With this, the code is much more simplified and changes
to blink are no longer needed. Anyway, without blink changes, feel free to
remove yourselves from the CC'ed list. Thanks a ton for your help :)!

Cool! It's so simple. (^_^)b
Thanks for utilizing Blink API.

[Rick Byers, 2016-01-08 17:07:35]

On 2016/01/08 01:11:12, spqchan (out until Jan 5) wrote:
> On 2016/01/07 17:40:29, spqchan (out until Jan 5) wrote:
> > On 2016/01/07 14:33:16, Rick Byers wrote:
> > > On 2016/01/07 08:47:02, Yosi_UTC9 wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
> > > > File third_party/WebKit/Source/web/WebViewImpl.h (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1528153004/diff/20001/third_party/WebKit/Sour...
> > > > third_party/WebKit/Source/web/WebViewImpl.h:236: bool
> > selectWordIfAnyAt(const
> > > > WebPoint&) override;
> > > > Can we use WebFrame::moveCaretSelection(const WebPoint&) and
> > > > WebFrame::selectWordAroundCaret() to select a word at a point?
> > > 
> > > Oh, interesting - I didn't realize those existed (I wasn't looking on
> > WebFrame).
> > >  The UI layer really doesn't want to need to reason about individual
frames
> (I
> > > suspect esprehn@ would say this is one of those details that blink should
be
> > > completely abstracting away from contents).  Eg. how it content supposed
to
> > know
> > > WHICH frame to change?
> > > 
> > > It looks like RenderViewImpl::OnMoveCaret and OnSelectWordAroundCaret rely
> on
> > > the notion of the "currently focused frame" (used for touch text
selection,
> > and
> > > contextual search). Perhaps this works because touchstart will set the
> focused
> > > frame.  Regardless I'm fairly sure that mousedown sets the focused frame
> (and
> > > doesn't mousedown also typically move the caret?  Though probably not if
> it's
> > > canceled by JavaScript).
> > > 
> > > So Sarah could possibly use the existing RenderWidgetHostImpl::MoveCaret
and
> > > RenderWidgetHost::SelectWordAroundCaret methods for her use case rather
than
> > add
> > > ANYTHING new on the renderer side.  Using more of the same logic as
> contextual
> > > search makes a lot of sense, since it's a similar feature in some ways. 
> > Sarah,
> > > can you please give this a try?  Sorry I didn't see this option sooner - I
> > > looked around for some existing functions, but missed these ones.  Again,
if
> > > this works it should make your CL even simpler, so I think it's worth the
> > effort
> > > :-)
> > 
> > Sure thing! I'll give it a go and will update on this
> 
> I finished making the changes in the renderer to use SelectWordAroundCaret and
> updated the description. With this, the code is much more simplified and
changes
> to blink are no longer needed. Anyway, without blink changes, feel free to
> remove yourselves from the CC'ed list. Thanks a ton for your help :)!

Awesome!  Sorry it took awhile to get to this point.  I'll move myself to cc.

[Rick Byers, 2016-01-08 17:07:48]

rbyers@chromium.org changed reviewers:
- ccameron@chromium.org, rbyers@chromium.org, yosin@chromium.org

[spqchan, 2016-01-08 18:21:25]

spqchan@chromium.org changed reviewers:
+ ccameron@chromium.org

[spqchan, 2016-01-08 22:03:00]

PTAL

[spqchan, 2016-01-08 22:29:51]

Description was changed from

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content, the word should 
be selected and the Look Up dialog should pop up.

- Browser UI handles NSPressure events and converts them into Force 
 Touch events in the base_view. 
- RenderWidgetHostView will then handle the Force Touch event by 
  sending a IPC "selectWordAroundCaret" message to the 
  renderer.
- When it receives an ACK for the message from the renderer,
  RenderWidgetHostView will then show the lookup dialog if a word was
  selected

BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content, the word should 
be selected and the Look Up dialog should pop up.

BUG=481890
==========

[ccameron, 2016-01-08 22:48:19]

On 2016/01/08 at 22:03:00, spqchan wrote:
> PTAL

lgtm % the suggestion to limit the change to sdk_forward_declarations.h and
render_widget_host_view_mac.mm

[spqchan, 2016-01-08 22:51:00]

The CQ bit was checked by spqchan@chromium.org

[spqchan, 2016-01-08 22:51:00]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org
Link to the patchset: https://codereview.chromium.org/1528153004/#ps100001
(title: "nit")

[spqchan, 2016-01-08 22:52:12]

Description was changed from

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content, the word should 
be selected and the Look Up dialog should pop up.

BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content the Look Up dialog should
pop up for the word.

BUG=481890
==========

[spqchan, 2016-01-08 22:52:33]

Description was changed from

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content the Look Up dialog should
pop up for the word.

BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content the Look Up dialog
should pop up for the word.

BUG=481890
==========

[commit-bot: I haz the power, 2016-01-08 22:53:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1528153004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1528153004/100001

[commit-bot: I haz the power, 2016-01-09 00:43:32]

Description was changed from

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content the Look Up dialog
should pop up for the word.

BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content the Look Up dialog
should pop up for the word.

BUG=481890
==========

[commit-bot: I haz the power, 2016-01-09 00:43:34]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-01-09 00:44:58]

Description was changed from

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content the Look Up dialog
should pop up for the word.

BUG=481890
==========

to

==========
Implementation for Look Up on Force Touch.
If the user force touched a word in the web content the Look Up dialog
should pop up for the word.

BUG=481890

Committed: https://crrev.com/927f27305d77ff600a505aa33d586ca8246f1614
Cr-Commit-Position: refs/heads/master@{#368481}
==========

[commit-bot: I haz the power, 2016-01-09 00:45:00]

Patchset 6 (id:??) landed as
https://crrev.com/927f27305d77ff600a505aa33d586ca8246f1614
Cr-Commit-Position: refs/heads/master@{#368481}