Allow browser process to request selection of word around caret

Allow browser process to request selection of word around caret.

This adds an IPC message for the browser process to be able to
request the renderer to select the word around the caret.
If the selection happens, the browser process will received a
SelectionChanged message back.

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

[pedro (no code reviews), 2014-04-30 22:07:08]

Mounir, Thanks for working on this!

https://codereview.chromium.org/267563003/diff/1/content/common/view_messages.h
File content/common/view_messages.h (right):

https://codereview.chromium.org/267563003/diff/1/content/common/view_messages...
content/common/view_messages.h:1001:
IPC_MESSAGE_ROUTED0(ViewMsg_SelectForSearch)
Nit: I would call this TapToSelect, since from this particular feature
perspective the search is non-existent.

[mlamouri (slow - plz ping), 2014-05-06 15:23:12]

jochen@, could you review the content/* bits?
cdn@, could you review the ipc changes?

Thanks :)

[jochen (gone - plz use gerrit), 2014-05-07 06:49:54]

can you explain why this is needed?

Maybe also add the code that invokes this method in this CL?

[mlamouri (slow - plz ping), 2014-05-07 09:08:42]

On 2014/05/07 06:49:54, jochen wrote:
> can you explain why this is needed?
> 
> Maybe also add the code that invokes this method in this CL?

This would be used for the contextual search experiment in Chrome Android:
unhandled tap would trigger a word selection. Patch set 1 which was a WIP has
some code using it for testing purposes.

[jochen (gone - plz use gerrit), 2014-05-08 07:44:33]

ok

can you please file a tracking bug for this feature and reference it from the CL
description?

Can you also add a browser test? Something like move the caret to a word, invoke
the method, check what the current selection is?

[mlamouri (slow - plz ping), 2014-05-08 09:24:09]

On 2014/05/08 07:44:33, jochen wrote:
> ok
> 
> can you please file a tracking bug for this feature and reference it from the
CL
> description?

I'm not owning the feature. Just giving a hand with some code plumbing. Will try
to find out where is the bug (thus the BUG=XXX).

> Can you also add a browser test? Something like move the caret to a word,
invoke
> the method, check what the current selection is?

I could but this is a Blink feature. The code I'm adding is merely plumbing
calling WebFrame::selectWordAroundCaret(). I'm not sure if testing this method
in Chrome is a good idea because it might fail if Blink changes. I thought that
it would make more sense to add integration tests later instead. WDYT?

[Cris Neckar, 2014-05-08 17:19:34]

On 2014/05/08 09:24:09, Mounir Lamouri wrote:
> On 2014/05/08 07:44:33, jochen wrote:
> > ok
> > 
> > can you please file a tracking bug for this feature and reference it from
the
> CL
> > description?
> 
> I'm not owning the feature. Just giving a hand with some code plumbing. Will
try
> to find out where is the bug (thus the BUG=XXX).
> 
> > Can you also add a browser test? Something like move the caret to a word,
> invoke
> > the method, check what the current selection is?
> 
> I could but this is a Blink feature. The code I'm adding is merely plumbing
> calling WebFrame::selectWordAroundCaret(). I'm not sure if testing this method
> in Chrome is a good idea because it might fail if Blink changes. I thought
that
> it would make more sense to add integration tests later instead. WDYT?

IPC changes LGTM

[donnd, 2014-05-08 23:16:12]

On 2014/05/08 17:19:34, Cris Neckar wrote:
> On 2014/05/08 09:24:09, Mounir Lamouri wrote:
> > On 2014/05/08 07:44:33, jochen wrote:
> > > ok
> > > 
> > > can you please file a tracking bug for this feature and reference it from
> the
> > CL
> > > description?
> > 
> > I'm not owning the feature. Just giving a hand with some code plumbing. Will
> try
> > to find out where is the bug (thus the BUG=XXX).
> > 
> > > Can you also add a browser test? Something like move the caret to a word,
> > invoke
> > > the method, check what the current selection is?
> > 
> > I could but this is a Blink feature. The code I'm adding is merely plumbing
> > calling WebFrame::selectWordAroundCaret(). I'm not sure if testing this
method
> > in Chrome is a good idea because it might fail if Blink changes. I thought
> that
> > it would make more sense to add integration tests later instead. WDYT?
> 
> IPC changes LGTM

Mounir, I patched this in and it works pretty well!  Yea!!! However, I noticed
that it sometimes selects a space even though there's text on both sides.  I
don't think this has anything to do with your CL here, but wanted to make note
of that in case you have ideas about what might be going on or what we might do.

[jochen (gone - plz use gerrit), 2014-05-15 10:53:41]

On 2014/05/08 09:24:09, Mounir Lamouri wrote:
> On 2014/05/08 07:44:33, jochen wrote:
> > ok
> > 
> > can you please file a tracking bug for this feature and reference it from
the
> CL
> > description?
> 
> I'm not owning the feature. Just giving a hand with some code plumbing. Will
try
> to find out where is the bug (thus the BUG=XXX).
> 
> > Can you also add a browser test? Something like move the caret to a word,
> invoke
> > the method, check what the current selection is?
> 
> I could but this is a Blink feature. The code I'm adding is merely plumbing
> calling WebFrame::selectWordAroundCaret(). I'm not sure if testing this method
> in Chrome is a good idea because it might fail if Blink changes. I thought
that
> it would make more sense to add integration tests later instead. WDYT?

as long as there is some test that's fine by me

lgtm

[mlamouri (slow - plz ping), 2014-05-16 14:05:32]

On 2014/05/15 10:53:41, jochen wrote:
> On 2014/05/08 09:24:09, Mounir Lamouri wrote:
> > On 2014/05/08 07:44:33, jochen wrote:
> > > ok
> > > 
> > > can you please file a tracking bug for this feature and reference it from
> the
> > CL
> > > description?
> > 
> > I'm not owning the feature. Just giving a hand with some code plumbing. Will
> try
> > to find out where is the bug (thus the BUG=XXX).
> > 
> > > Can you also add a browser test? Something like move the caret to a word,
> > invoke
> > > the method, check what the current selection is?
> > 
> > I could but this is a Blink feature. The code I'm adding is merely plumbing
> > calling WebFrame::selectWordAroundCaret(). I'm not sure if testing this
method
> > in Chrome is a good idea because it might fail if Blink changes. I thought
> that
> > it would make more sense to add integration tests later instead. WDYT?
> 
> as long as there is some test that's fine by me
> 
> lgtm

Blink tests are here: https://codereview.chromium.org/286203012/

[mlamouri (slow - plz ping), 2014-05-16 15:41:15]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-05-16 15:41:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mlamouri@chromium.org/267563003/20001

[commit-bot: I haz the power, 2014-05-16 18:32:48]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-05-16 19:13:19]

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

[commit-bot: I haz the power, 2014-05-16 19:13:20]

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

[mlamouri (slow - plz ping), 2014-05-17 13:50:44]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-05-17 13:52:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mlamouri@chromium.org/267563003/20001

[commit-bot: I haz the power, 2014-05-17 15:35:07]

Change committed as 271206