New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 630937 link

Starred by 2 users
Status: Verified
Owner:
changwan@chromium.org
Closed: Aug 2016
Cc:
█ yabinh@chromium.org
█ aelias@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

ImeThread: Play store login hangs when 'next' is pressed

Project Member Reported by changwan@chromium.org, Jul 25 2016 
Chrome Version       : development version (53+)

What steps will reproduce the problem?
1. Override WebView and enable ImeThread feature on S7.
2. Choose to use Samsung keyboard.
3. Remove Google account if already set up.
4. Go to Play Store and try to sign in your account by typing your email.
5. Press NEXT.

What is the expected result?
Proceed to the password section.

What happens instead of that?
'NEXT' button is pressed, but hangs forever.

50% repro rate. This only occurs on WebView.


A deadlock occurs because UI thread waits for a lock and another thread waits for UI thread while holding the lock.

1) The selection change causes 'cancelComposition' and eventually restarts input on the main thread. This waits for InputMethodManager$H to become lockable on UI thread.
2) Almost at the same time, Java Bridge thread runs InputMethodManager#hideSoftInputFromWindow(), which holds InputMethodManager$H lock and calls ThreadedInputConnectionProxyView#getWindowToken(). Now getWindowToken() in turn waits for UI thread to become available.

Comment 1 by changwan@chromium.org, Jul 25 2016 

JavaBridge is the name of the background thread to run JavaScript-triggered tasks on WebView:
https://developer.android.com/reference/android/webkit/WebView.html#addJavascriptInterface(java.lang.Object, java.lang.String)

In ThreadedInputConnectionProxyView, we'll have to run on UI thread only those that were originally running on IME thread.

Comment 2 by changwan@chromium.org, Jul 27 2016

Description: Show this description

Comment 3 by changwan@chromium.org, Jul 27 2016

Description: Show this description
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1 2016 

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

commit 43ceb11f9abf67fea18e8a731a9ef61f5cfe22fc
Author: changwan <changwan@chromium.org>
Date: Mon Aug 01 07:41:33 2016

Cache proxy view return value to avoid deadlock

If WebView has an active IME but the Android app calls
InputMethodManager#hideSoftInputFromWindow() on a non-UI thread,
then a deadlock may happen.

One example case is when JavaScript triggers it through JavascriptInterface
(therefore, on JavaBridge thread.)

The deadlock scenario is as follows:

1) InputMethodManager#hideSoftFromWindow() calls
   ThreadedInputConnectionProxyView#getWindowToken() on JavaBridge thread
   while holding InputMethodManager#mH.
   Then getWindowToken() waits for UI thread to become available.
2) At almost same time, InputMethodManager#restartInput() was waiting for
   InputMethodManager#mH on UI thread

This deadlock can be avoided by caching return values as atomic objects.

Alternative approach I've tried: check the current thread and block
it only when it's IME thread - this may still leave room for deadlock
between IME thread and UI thread.

BUG= 630937 

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

[modify] https://crrev.com/43ceb11f9abf67fea18e8a731a9ef61f5cfe22fc/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/43ceb11f9abf67fea18e8a731a9ef61f5cfe22fc/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java
[modify] https://crrev.com/43ceb11f9abf67fea18e8a731a9ef61f5cfe22fc/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/43ceb11f9abf67fea18e8a731a9ef61f5cfe22fc/content/public/android/java/src/org/chromium/content/browser/input/ReplicaInputConnection.java
[modify] https://crrev.com/43ceb11f9abf67fea18e8a731a9ef61f5cfe22fc/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java
[modify] https://crrev.com/43ceb11f9abf67fea18e8a731a9ef61f5cfe22fc/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionProxyView.java
[modify] https://crrev.com/43ceb11f9abf67fea18e8a731a9ef61f5cfe22fc/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java

Comment 5 by changwan@chromium.org, Aug 1 2016

Labels: Merge-Request-53
requesting merge of #4 to m53

Comment 6 by dimu@chromium.org, Aug 1 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 1 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28ceae2df36aa9368b2a04179ce23a03d44ea328

commit 28ceae2df36aa9368b2a04179ce23a03d44ea328
Author: Changwan Ryu <changwan@google.com>
Date: Mon Aug 01 08:14:04 2016

Cache proxy view return value to avoid deadlock

If WebView has an active IME but the Android app calls
InputMethodManager#hideSoftInputFromWindow() on a non-UI thread,
then a deadlock may happen.

One example case is when JavaScript triggers it through JavascriptInterface
(therefore, on JavaBridge thread.)

The deadlock scenario is as follows:

1) InputMethodManager#hideSoftFromWindow() calls
   ThreadedInputConnectionProxyView#getWindowToken() on JavaBridge thread
   while holding InputMethodManager#mH.
   Then getWindowToken() waits for UI thread to become available.
2) At almost same time, InputMethodManager#restartInput() was waiting for
   InputMethodManager#mH on UI thread

This deadlock can be avoided by caching return values as atomic objects.

Alternative approach I've tried: check the current thread and block
it only when it's IME thread - this may still leave room for deadlock
between IME thread and UI thread.

BUG= 630937 

Review-Url: https://codereview.chromium.org/2175263002
Cr-Commit-Position: refs/heads/master@{#408925}
(cherry picked from commit 43ceb11f9abf67fea18e8a731a9ef61f5cfe22fc)

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

Cr-Commit-Position: refs/branch-heads/2785@{#427}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/28ceae2df36aa9368b2a04179ce23a03d44ea328/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/28ceae2df36aa9368b2a04179ce23a03d44ea328/content/public/android/java/src/org/chromium/content/browser/input/ChromiumBaseInputConnection.java
[modify] https://crrev.com/28ceae2df36aa9368b2a04179ce23a03d44ea328/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/28ceae2df36aa9368b2a04179ce23a03d44ea328/content/public/android/java/src/org/chromium/content/browser/input/ReplicaInputConnection.java
[modify] https://crrev.com/28ceae2df36aa9368b2a04179ce23a03d44ea328/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java
[modify] https://crrev.com/28ceae2df36aa9368b2a04179ce23a03d44ea328/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionProxyView.java
[modify] https://crrev.com/28ceae2df36aa9368b2a04179ce23a03d44ea328/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java

Comment 8 by changwan@chromium.org, Aug 1 2016

Status: Fixed (was: Started)

Comment 9 by changwan@chromium.org, Aug 1 2016

Labels: M-53

Comment 10 by alek...@chromium.org, Aug 2 2016

Status: Verified (was: Fixed)

Comment 11 by alek...@chromium.org, Aug 2 2016 

Verified on: Samsung S7 MMB29K with latest M53
►

Sign in to add a comment