Switch to sending IME selection updates early.

Switch to sending IME selection updates early.

- Switching to send out UpdateSelection updates as soon as the change is
  applied in AdapterInputConnection (instead of waiting for Blink)
- Start ignoring the IME updates from Blink if it was caused by an IME event
  sent from AdapterInputConnection by checking requiredAck flag.

BUG=235704, 145521
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=226301

[aurimas (slooooooooow), 2013-09-24 02:59:21]

Hey Tommy,

This is the continuation of the IME refactoring. Please take a look at this CL.

Thanks,
Aurimas

[aurimas (slooooooooow), 2013-09-25 21:18:21]

On 2013/09/24 02:59:21, aurimas wrote:
> Hey Tommy,
> 
> This is the continuation of the IME refactoring. Please take a look at this
CL.
> 
> Thanks,
> Aurimas

ping?

[nyquist, 2013-09-27 18:02:36]

nice work!

https://codereview.chromium.org/24195023/diff/6001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
(right):

https://codereview.chromium.org/24195023/diff/6001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:121:
int compositionStart, int compositionEnd, boolean requireAck) {
Add javadoc for requireAck

https://codereview.chromium.org/24195023/diff/6001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:154:
private void updateSelection() {
How about updateSelectionIfNotNested? Or do you feel it is clear enough from the
current name?

https://codereview.chromium.org/24195023/diff/6001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:284:
if (mNumNestedBatchEdits == 0) return true;
return false here?

https://codereview.chromium.org/24195023/diff/6001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:288:
return true;
return mNumNestedBatchEdits != 0; ?

[aurimas (slooooooooow), 2013-09-27 21:06:04]

https://chromiumcodereview.appspot.com/24195023/diff/6001/content/public/andr...
File
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
(right):

https://chromiumcodereview.appspot.com/24195023/diff/6001/content/public/andr...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:121:
int compositionStart, int compositionEnd, boolean requireAck) {
On 2013/09/27 18:02:36, nyquist wrote:
> Add javadoc for requireAck

Done.

https://chromiumcodereview.appspot.com/24195023/diff/6001/content/public/andr...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:154:
private void updateSelection() {
On 2013/09/27 18:02:36, nyquist wrote:
> How about updateSelectionIfNotNested? Or do you feel it is clear enough from
the
> current name?

Done.

https://chromiumcodereview.appspot.com/24195023/diff/6001/content/public/andr...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:284:
if (mNumNestedBatchEdits == 0) return true;
On 2013/09/27 18:02:36, nyquist wrote:
> return false here?

Done.

https://chromiumcodereview.appspot.com/24195023/diff/6001/content/public/andr...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:288:
return true;
On 2013/09/27 18:02:36, nyquist wrote:
> return mNumNestedBatchEdits != 0; ?

Done.

[aurimas (slooooooooow), 2013-09-27 23:46:33]

+tedchoc: please take a look at this CL.

Aurimas

[nyquist, 2013-09-30 21:28:24]

lgtm

[Ted C, 2013-09-30 21:59:46]

lgtm w/ a couple comments

https://codereview.chromium.org/24195023/diff/12001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
(right):

https://codereview.chromium.org/24195023/diff/12001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:429:
protected boolean isIgnoringTextInputStateUpdates() {
can we just delete this method entirely?

https://codereview.chromium.org/24195023/diff/12001/content/public/android/ja...
File
content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
(left):

https://codereview.chromium.org/24195023/diff/12001/content/public/android/ja...
content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:199:
public void testUpdatesGetIgnoredDuringBatchEdits() throws Throwable {
Can we keep some kind of test that ensures updateSelectionIfRequired isn't
called if beginBatchEdit is called (verifying the nesting functionality)

[aurimas (slooooooooow), 2013-09-30 23:04:50]

https://codereview.chromium.org/24195023/diff/12001/content/public/android/ja...
File
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java
(right):

https://codereview.chromium.org/24195023/diff/12001/content/public/android/ja...
content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java:429:
protected boolean isIgnoringTextInputStateUpdates() {
On 2013/09/30 21:59:46, Ted C wrote:
> can we just delete this method entirely?

Done.

https://codereview.chromium.org/24195023/diff/12001/content/public/android/ja...
File
content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
(left):

https://codereview.chromium.org/24195023/diff/12001/content/public/android/ja...
content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:199:
public void testUpdatesGetIgnoredDuringBatchEdits() throws Throwable {
On 2013/09/30 21:59:46, Ted C wrote:
> Can we keep some kind of test that ensures updateSelectionIfRequired isn't
> called if beginBatchEdit is called (verifying the nesting functionality)

Done.

[aurimas (slooooooooow), 2013-09-30 23:08:59]

cdn@chromium.org: Please review changes in content/common/view_messages.h

jamesr@chromium.org: Please review changes in content/renderer/render_widget.*

sievers@chromium.org: Please review changes in content/browser/renderer_host/*

[no sievers, 2013-09-30 23:18:23]

On 2013/09/30 23:08:59, aurimas wrote:
> cdn@chromium.org: Please review changes in content/common/view_messages.h
> 
> jamesr@chromium.org: Please review changes in content/renderer/render_widget.*
> 
> sievers@chromium.org: Please review changes in content/browser/renderer_host/*

content/browser/renderer_host lgtm

[jamesr, 2013-10-01 00:28:00]

lgtm

[Cris Neckar, 2013-10-01 18:19:05]

IPC changes LGTM

[commit-bot: I haz the power, 2013-10-01 18:21:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aurimas@chromium.org/24195023/35001

[commit-bot: I haz the power, 2013-10-01 19:07:57]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[commit-bot: I haz the power, 2013-10-01 19:43:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aurimas@chromium.org/24195023/35001

[commit-bot: I haz the power, 2013-10-01 21:14:38]

Change committed as 226301