Handler shown in editable field should be removed on any key press on keypad.

Hide insertion handles when deleting text

Insertion text handles should be hidden when the underlying text changes. As
such, add a hook to notify the AdapterInputConnection embedder of an IME event 
when text is deleted, in turn hiding the insertion handle.

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

[ankit, 2014-06-27 07:14:41]

PTAL

[jdduke (slow), 2014-06-27 15:25:01]

https://codereview.chromium.org/351403002/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/351403002/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:536:
getContentViewClient().onImeEvent();
So, is this function not getting called when we delete a character? 

What if we move the hideHandles() call to |ContentViewCore.dispatchKeyEvent()|?

https://codereview.chromium.org/351403002/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:1978:
protected InsertionHandleController getInsertionHandleController() {
Hmm, exposing the internals of handle manipulation to ContentView or ImeAdapter
is  probably the wrong direction, particularly as these handles will be removed
in the next several weeks (see https://codereview.chromium.org/335943002/).

[aurimas (slooooooooow), 2014-06-27 15:55:39]

https://codereview.chromium.org/351403002/diff/1/content/public/android/java/...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/351403002/diff/1/content/public/android/java/...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:536:
getContentViewClient().onImeEvent();
On 2014/06/27 15:25:01, jdduke wrote:
> So, is this function not getting called when we delete a character? 
> 
> What if we move the hideHandles() call to
|ContentViewCore.dispatchKeyEvent()|?

We can simply start calling mViewEmbedder.onImeEvent(false) in
ImeAdapter#deleteSurroundingText(..)

[jdduke (slow), 2014-06-27 16:07:54]

On 2014/06/27 15:55:39, aurimas wrote:
> We can simply start calling mViewEmbedder.onImeEvent(false) in
> ImeAdapter#deleteSurroundingText(..)

Works for me.

[ankit, 2014-06-30 05:16:50]

PTAL new patch set

[ankit, 2014-07-01 12:21:38]

@aurimas
PTAL new patch set

[jdduke (slow), 2014-07-01 19:01:15]

On 2014/07/01 12:21:38, ankit2.kumar wrote:
> @aurimas
> PTAL new patch set

LGTM but please wait for aurimas@ to approve.  I don't see any existing tests
for |deleteSurroundingText()|, so I'm not sure if it's worth adding one
explicitly for this addition.

[aurimas (slooooooooow), 2014-07-01 19:31:14]

https://codereview.chromium.org/351403002/diff/20001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
(right):

https://codereview.chromium.org/351403002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java:364:
if (mNativeImeAdapterAndroid == 0) return;
Should we add a call to onImeEvent here as well?

https://codereview.chromium.org/351403002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java:430:
if (mNativeImeAdapterAndroid == 0) return false;
Should we add a call to onImeEvent here as well?

https://codereview.chromium.org/351403002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java:442:
if (mNativeImeAdapterAndroid == 0) return false;
Should we add a call to onImeEvent here as well?

[jdduke (slow), 2014-07-01 19:36:37]

https://codereview.chromium.org/351403002/diff/20001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
(right):

https://codereview.chromium.org/351403002/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java:364:
if (mNativeImeAdapterAndroid == 0) return;
On 2014/07/01 19:31:14, aurimas wrote:
> Should we add a call to onImeEvent here as well?

I don't think so, and neither in the other functions you referenced.  Ideally
the |onImeEvent()| would correspond to synthetic/real key events being sent to
the renderer.  I think adding the call in |deleteSurroundingText()| should be
sufficient for now (the other cases are already handled by the response from the
renderer when the selection changes).

[ankit, 2014-07-03 11:23:18]

@aurimas
PTAL

[ankit, 2014-07-06 15:08:20]

PTAL

[aurimas (slooooooooow), 2014-07-07 20:56:28]

lgtm

[ankit, 2014-07-08 03:36:59]

The CQ bit was checked by ankit2.kumar@samsung.com

[commit-bot: I haz the power, 2014-07-08 03:38:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ankit2.kumar@samsung.com/351403002/20001

[commit-bot: I haz the power, 2014-07-08 07:28:25]

Change committed as 281686