I can confirm this issue. The problem recently appeared (last few days), for at least 1 customer, using Samsung S6 with default Samsung keyboard (with all default settings). 

In my case also present in Cordova webview.

Im able to reproduce on samsung galaxy s7 / MMB29M with latest M56

I did bisect, here's a broken point: 
54.0.2840.35 - broken
54.0.2840.34 - good 

https://chromium.googlesource.com/chromium/src/+log/54.0.2840.34..54.0.2840.35?pretty=fuller&n=10000

I don't have a Samsung device right now, I'll have to go to the office and try to repro.

Does this happen on Chrome as well?

Could you provide a minimum code snippet that leads to this problem? Or a sample app in case this only happens on WebView?

I suspect that https://codereview.chromium.org/2309983002/ was the culprit.
I'm thinking of reverting the change.

amineer@, do we have room for a respin within M54?

Yes, but just barely - we need to settle the matter by Sunday.

This is pretty impactful, correct?  Unfortunately that is a lot of change to take right now - is it safe to revert all that, and then ship it right to stable?  What's the risk?

I expect the impact will be small except for fixing this and issue 660546 because it was fixing a theoretical problem that was not found in the wild. The risk would be that Google keyboard may not work correctly when we revert this change (in these scenarios and potentially others). If it's due Sunday I'll have to manually test lots of scenarios myself. I still need to do a bit more investigation before reverting the change, though.

Yeah, it seems high-impact, I can also repro on Samsung Keyboard on Galaxy S7 in a Chrome textbox.  Affects any punctuation.

I'm not that comfortable with the revert because it doesn't return us to a proven good state.  In http://crbug.com/643473 you said the old behavior doesn't match ReplicaInputConnection.  The current behavior apparently doesn't match ReplicaInputConnection behavior either.  What would a fix look like that actually returned us to the previous order of operations presented to the IME?

Give me a bit more time, I'll investigate a bit more about that. One thing I found so far is that ThreadedInputConnection#updateStateOnUiThread()'s comment does not match what it does.

To reduce risk, I think we should restrict the behavior change on the branch to Samsung keyboard.  I think we can probably do that by examining InputMethodManager.getCurrentInputMethodSubtype().getDisplayName()

Issue 660546 has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72381b3189537b4e3c7e2dbff8a2ca398bd3b447

commit 72381b3189537b4e3c7e2dbff8a2ca398bd3b447
Author: changwan <changwan@chromium.org>
Date: Sat Oct 29 05:08:25 2016

Fix endBatchEdit() state update order issue

If we call endBatchEdit() and then call getExtractedText() immediately,
then the order gets reversed.

The reason is that getExtractedText() is already block-waiting for a new
state, and endBatchEdit() was piggy-backing on FROM_IME to enforce a
state update.

The more natural change source for a state update at the end of a batch
edit is actually FROM_NON_IME, which does not interfere with the
block-wait mechanism of get* methods. The state update order in the above
scenario is fixed because blockAndGetStateUpdate() handles it correctly
when it's FROM_NON_IME.

Also, now we bounce state update in RenderWidget instead of doing so in
ThreadedInputConnection. This is another way to make sure that we get
state update at the end of batch edit: the intermediate state update was
already bounced off.

Now there is no need to check isInBatchEdit() inside
ThreadedInputConnection, and ThreadedInputConnectionTest needs to be
updated, but let me handle them in a separate CL.

BUG= 659934 

Review-Url: https://codereview.chromium.org/2462583004
Cr-Commit-Position: refs/heads/master@{#428602}

[modify] https://crrev.com/72381b3189537b4e3c7e2dbff8a2ca398bd3b447/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnection.java
[modify] https://crrev.com/72381b3189537b4e3c7e2dbff8a2ca398bd3b447/content/renderer/render_widget.cc

After seeing the fix, I no longer think we should make conditional on Samsung IME.  It's not relating to a Samsung quirk but was straight-up race condition in our own code, that just happened to have a predictable repro with a Samsung call pattern.  The bug almost certainly affects some other keyboards too with varying symptoms, and the fix is small and well-understood.

[Automated comment] Request affecting a post-stable build (M54), manual review required.

Your change meets the bar and is auto-approved for M55 (branch: 2883)

[Automated comment] Request affecting a post-stable build (M54), manual review required.

I take that back, I found a bug in the fix.  It causes suppression of show_ime changes coming during a batch edit.  On a Chromium build with r428602 applied and Google Keyboard 5.1:

1. Create a test account on try.discourse.org
2. Select "+ New Topic"
3. Tap in the textbox and observe the soft keyboard appears.
4. Type a few characters of text.
5. Tap outside the textbox (e.g. in the gray empty area at the bottom) and observe soft keyboard hides.
6. Tap inside the textbox again.  Now the soft keyboard does not appear (and can never be made to appear from now on).

I've changed my mind and I'm now favoring revert of culprit https://codereview.chromium.org/2309983002/ on 54 after all.  We do have some evidence to trust the old way given that it survived 50% Finch trial for all of 53 beta.  And after seeing the reordering and side effect dropping pitfalls we're running into now, I think the pitfall of the old way (undernotifying selection) is probably inherently safer (more prone to cause cosmetic bugs than ship blockers).  And the substantive lines of code changed (non test, non plumbing) are small.

Re 23, probably it can be fixed if we replace UpdateTextInputState by ImeEventGuard... But I agree that reverting is simpler.

Please disregard #25. Let me revert the two changes from ToT.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/872c3c42beb58bbf5b6a2995db0be4fa14f67000

commit 872c3c42beb58bbf5b6a2995db0be4fa14f67000
Author: changwan <changwan@chromium.org>
Date: Sat Oct 29 16:32:30 2016

Revert of Fix endBatchEdit() state update order issue (patchset #1 id:1 of https://codereview.chromium.org/2462583004/ )

Reason for revert:
As pointed out in https://bugs.chromium.org/p/chromium/issues/detail?id=659934#c23, keyboard may not show up when it has to with this patch.

Original issue's description:
> Fix endBatchEdit() state update order issue
>
> If we call endBatchEdit() and then call getExtractedText() immediately,
> then the order gets reversed.
>
> The reason is that getExtractedText() is already block-waiting for a new
> state, and endBatchEdit() was piggy-backing on FROM_IME to enforce a
> state update.
>
> The more natural change source for a state update at the end of a batch
> edit is actually FROM_NON_IME, which does not interfere with the
> block-wait mechanism of get* methods. The state update order in the above
> scenario is fixed because blockAndGetStateUpdate() handles it correctly
> when it's FROM_NON_IME.
>
> Also, now we bounce state update in RenderWidget instead of doing so in
> ThreadedInputConnection. This is another way to make sure that we get
> state update at the end of batch edit: the intermediate state update was
> already bounced off.
>
> Now there is no need to check isInBatchEdit() inside
> ThreadedInputConnection, and ThreadedInputConnectionTest needs to be
> updated, but let me handle them in a separate CL.
>
> BUG= 659934 
>
> Committed: https://crrev.com/72381b3189537b4e3c7e2dbff8a2ca398bd3b447
> Cr-Commit-Position: refs/heads/master@{#428602}

TBR=aelias@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 659934 

Review-Url: https://codereview.chromium.org/2463583002
Cr-Commit-Position: refs/heads/master@{#428612}

[modify] https://crrev.com/872c3c42beb58bbf5b6a2995db0be4fa14f67000/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnection.java
[modify] https://crrev.com/872c3c42beb58bbf5b6a2995db0be4fa14f67000/content/renderer/render_widget.cc

Revert requested in c#24 approved for M54 branch 2840. 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a680ae83d852ecf8f24b1236c67c8df855bb241

commit 3a680ae83d852ecf8f24b1236c67c8df855bb241
Author: Alexandre Elias <aelias@chromium.org>
Date: Sat Oct 29 19:08:11 2016

Revert "Allow selection change update before beginBatchEdit" on M54

This reverts commit 4662007a92a92b016ef3f05fb4cbbd6904825cc0.

BUG= 659934 

Review URL: https://codereview.chromium.org/2460173003 .

Cr-Commit-Position: refs/branch-heads/2840@{#802}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/browser/renderer_host/ime_adapter_android.cc
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/browser/renderer_host/ime_adapter_android.h
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/common/input_messages.h
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/common/text_input_state.cc
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/common/text_input_state.h
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/common/view_messages.h
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/public/android/java/src/org/chromium/content/browser/input/ReplicaInputConnection.java
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/public/android/java/src/org/chromium/content/browser/input/TextInputState.java
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnection.java
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/public/android/junit/src/org/chromium/content/browser/input/TextInputStateTest.java
[add] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionFactoryTest.java
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionTest.java
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/renderer/render_widget.cc
[modify] https://crrev.com/3a680ae83d852ecf8f24b1236c67c8df855bb241/content/renderer/render_widget.h

Fixed builds at https://pantheon.corp.google.com/storage/browser/chrome-signed/android-B0urB0N/54.0.2840.85/ .  They seem to work fine for me with a few minutes of smoke testing with Samsung Keyboard for S7 and Google Keyboard 5.1.

An interesting finding: I realized today this issue is specific to WebView, and does not affect Chrome Stable.  That's surprising because it affects Chrome Beta/Dev/Canary.  The exact same version 54.0.2840.68 reproes bug on Chrome.apk but not on ChromeBeta.apk.  I infer the Samsung keyboard has an APK package name-guarded workaround for Chrome Stable that avoids the racy call pattern.  That's likely due to some older IME bug (our old IME architecture was not great at handling this call pattern either).

Fix verified on webview : 54.0.2840.85 on the devices below, where issue was reproducible on the below samsung devices with older webview: 54.0.2840.68
Samsung Galaxy S7 SM-ZG930F/MMB29K
Samsung Galaxy S7 Edge/MMB29M -Samsung Keyboard version: 1.2.48
LG G5/MMB29M
HTC one M9/MRA58K 
Samsung S6/ MMB29K- keyboard version: 1.0.89, 
Samsung Note 5/MMB29K- samsung keyboard version: 1.2.14, 

Issue was not reproducible on Samsung Galaxy S7(SM-930P)/MMB29M -samsung keyboard version- 1.3.50, Samsung S5 Mini (SM-G800F)-samsung keyboard version:4.0,  with older webview: 54.0.2840.68. Issue is also not reproducible on these devices with webview: 54.0.2840.85

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The fix for this issue in M55 is actually still under review @ https://codereview.chromium.org/2464023002/, so removing merge approval for that milestone for now.

Still seeing this issue in 56.0.2906.0 on Samsung Galaxy S6 Edge SM-G925T.

Master (therefore 56) doesn't have this revert in comment #29: 3a680ae83d852ecf8f24b1236c67c8df855bb241

Actually, we'd like to revert the original patch (https://codereview.chromium.org/2309983002) on M55 and having the new patch (https://codereview.chromium.org/2464023002/) go through the full cycle. Let me just add back the approval label assuming it is still valid.

Sorry for the confusion.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c760d28de560a9c989b0c5b7cb6fef1930d6181d

commit c760d28de560a9c989b0c5b7cb6fef1930d6181d
Author: Changwan Ryu <changwan@google.com>
Date: Wed Nov 02 02:26:27 2016

Revert "Allow selection change update before beginBatchEdit" on M55

This reverts commit 4662007a92a92b016ef3f05fb4cbbd6904825cc0.

BUG= 659934 

Review URL: https://codereview.chromium.org/2465173004 .

Cr-Commit-Position: refs/branch-heads/2883@{#415}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/browser/renderer_host/ime_adapter_android.cc
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/browser/renderer_host/ime_adapter_android.h
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/common/input_messages.h
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/common/text_input_state.cc
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/common/text_input_state.h
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/common/view_messages.h
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/public/android/java/src/org/chromium/content/browser/input/ReplicaInputConnection.java
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/public/android/java/src/org/chromium/content/browser/input/TextInputState.java
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnection.java
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/public/android/junit/src/org/chromium/content/browser/input/TextInputStateTest.java
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionFactoryTest.java
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/public/android/junit/src/org/chromium/content/browser/input/ThreadedInputConnectionTest.java
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/renderer/render_widget.cc
[modify] https://crrev.com/c760d28de560a9c989b0c5b7cb6fef1930d6181d/content/renderer/render_widget.h

Issue verified on latest M55
Tested Devices:
Samsung Galaxy Grand Prime 4G(SM-G531F) / LMY48B
LG G/5MMB29M
HTC One M9/MRA58K
Thanks

Issue verified on latest M55
Galaxy S7 edge AT&T / MMB29M 

Ok, the reason why try.discourse.org was not working was:

In some cases, beginBatchEdit() is not matched with endBatchEdit() because of sudden reactivation of input.

The first approach (https://codereview.chromium.org/2462583004) did not work because it could not handle the following sequence:

restartInput(type=TEXT)
beginBatchEdit()
restartInput(type=NONE)

The second restartInput did not send endBatchEdit() in any of InputConnection#resetOnUiThread(), onRestartInputOnUiThread(), unblockOnUiThread().

However, the marking mechanism in the new patchset (https://codereview.chromium.org/2464023002/#ps20001) worked because ThreadedInputConnection’s state got updated after reset (mNumNestedBatchEdits = 0).

If we are to improve the first approach, I'll have to try to polish up InputConnection#resetOnUiThread(), onRestartInputOnUiThread(), unblockOnUiThread() and figure out which one should call endBatchEdit() in a consistent manner.

One more thing I'm concerned about is update from external event. Intermediate updates caused by IME's batch edit should not update selection - this is crystal clear. But what about updates from JS events or user touch - should it still update selection to InputMethodManager? If that is the case, we may still need to distinguish FROM_IME from FROM_NON_IME.

aelias@, what do you think?

OK.  There are bound to be more bugs in the future from failing to send endBatchEdit() to renderer on restart, so it's something that we should clean up rather than work around.

> But what about updates from JS events or user touch - should it still update selection to InputMethodManager?

What does EditText do when it receives a touch during batch edit?

-cc amineer and removing milestone labels as this only affects ToT now.

The original patch reverted from ToT, punting to m57.

Ok, actually we don't need to keep this bug. I'll reopen the original bug instead.