Ignore reentrancy for ImeThread triggering

Ignore reentrancy for ImeThread triggering

In the delayed activation of input connection, we call requestFocus()
on proxy view, and this triggers HwSecImmHelper.isPasswordInputType()
on Huawei phone, which in turn calls View#onCreateInputConnection() so
there is an infinite loop, which eventually causes stackoverflow.

Presumably isPasswordInputType() is only concerned about whether
onCreateInputConnection()'s outAttrs.inputType is password type or not,
so we fill out outAttrs.inputType and early out when there is a reentrance.

BUG=636197
Committed: https://crrev.com/f5feceb259130400bc3f05826c8bc625f3399794
Cr-Commit-Position: refs/heads/master@{#415220}

[Changwan Ryu, 2016-08-15 07:17:03]

The CQ bit was checked by changwan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-15 07:17:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Changwan Ryu, 2016-08-15 07:17:41]

changwan@chromium.org changed reviewers:
+ aelias@chromium.org

[Changwan Ryu, 2016-08-15 07:17:42]

PTAL

[commit-bot: I haz the power, 2016-08-15 07:49:10]

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

[commit-bot: I haz the power, 2016-08-15 07:49:10]

Dry run: This issue passed the CQ dry run.

[aelias_OOO_until_Jul13, 2016-08-15 20:07:49]

I'd like us to try some more to find a local repro before we land.  For all we
know there will be a second crasher right after this one and we actually need to
workaround two things.

[Changwan Ryu, 2016-08-29 07:16:45]

The CQ bit was checked by changwan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-29 07:16:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Changwan Ryu, 2016-08-29 07:19:02]

Description was changed from

==========
Presumptive fix for stackoverflow on Huawei phones

In the delayed activation of input connection, we call requestFocus()
on proxy view, and this triggers HwSecImmHelper.isPasswordInputType()
which in turn calls View#onCreateInputConnection() so there is an infinite
loop.

Presumably isPasswordInputType() is only concerned about whether
onCreateInputConnection()'s outAttrs.inputType is password type or not,
so we fill out outAttrs.inputType and early out in this case.

BUG=636197
==========

to

==========
Ignore reentrancy for ImeThread triggering

In the delayed activation of input connection, we call requestFocus()
on proxy view, and this triggers HwSecImmHelper.isPasswordInputType()
on Huawei phone, which in turn calls View#onCreateInputConnection() so
there is an infinite loop, which eventually causes stackoverflow.

Presumably isPasswordInputType() is only concerned about whether
onCreateInputConnection()'s outAttrs.inputType is password type or not,
so we fill out outAttrs.inputType and early out when there is a reentrance.

BUG=636197
==========

[Changwan Ryu, 2016-08-29 07:22:34]

I've generalized it a bit. Could you take another look?

[commit-bot: I haz the power, 2016-08-29 08:25:19]

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

[commit-bot: I haz the power, 2016-08-29 08:25:20]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[aelias_OOO_until_Jul13, 2016-08-29 22:49:02]

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

https://codereview.chromium.org/2246833002/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java:117:
ImeUtils.computeEditorInfo(
Can we move this up a few levels and run it unconditionally, and delete the
corresponding line in initializeOutAttrsOnUiThread (which would also mean
renaming that method to something like reset())?  Then
triggerDelayedOnCreateInputConnection can continue to return void.  And add a
TODO to move it all the way up to ImeAdapter.onCreateInputConnection after
ReplicaInputConnection is deleted.

(This would result in behavior change for all users, but it's probably OK.)

https://codereview.chromium.org/2246833002/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java:145:
mReentrantTriggering = false;
Since we cannot use RAII pattern in Java, could you move the
"mReentrantTriggering = true;" line right above requestFocus() instead?  I think
this would also clarify that we only ever expect to encounter reentrancy there.

[Changwan Ryu, 2016-08-30 01:12:45]

The CQ bit was checked by changwan@chromium.org to run a CQ dry run

[Changwan Ryu, 2016-08-30 01:13:03]

PTAL

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

https://codereview.chromium.org/2246833002/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java:117:
ImeUtils.computeEditorInfo(
On 2016/08/29 22:49:02, aelias wrote:
> Can we move this up a few levels and run it unconditionally, and delete the
> corresponding line in initializeOutAttrsOnUiThread (which would also mean
> renaming that method to something like reset())?  Then
> triggerDelayedOnCreateInputConnection can continue to return void.  And add a
> TODO to move it all the way up to ImeAdapter.onCreateInputConnection after
> ReplicaInputConnection is deleted.
> 
> (This would result in behavior change for all users, but it's probably OK.)

Done. Maybe I was too worried about the change. I've tested several cases and
this seems to work.

https://codereview.chromium.org/2246833002/diff/20001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java:145:
mReentrantTriggering = false;
On 2016/08/29 22:49:02, aelias wrote:
> Since we cannot use RAII pattern in Java, could you move the
> "mReentrantTriggering = true;" line right above requestFocus() instead?  I
think
> this would also clarify that we only ever expect to encounter reentrancy
there.

Done.

[commit-bot: I haz the power, 2016-08-30 01:51:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 03:49:44]

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

[commit-bot: I haz the power, 2016-08-30 03:49:45]

Dry run: This issue passed the CQ dry run.

[aelias_OOO_until_Jul13, 2016-08-30 05:41:28]

lgtm

[Changwan Ryu, 2016-08-30 05:46:11]

The CQ bit was checked by changwan@chromium.org

[commit-bot: I haz the power, 2016-08-30 05:47:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 06:25:58]

Description was changed from

==========
Ignore reentrancy for ImeThread triggering

In the delayed activation of input connection, we call requestFocus()
on proxy view, and this triggers HwSecImmHelper.isPasswordInputType()
on Huawei phone, which in turn calls View#onCreateInputConnection() so
there is an infinite loop, which eventually causes stackoverflow.

Presumably isPasswordInputType() is only concerned about whether
onCreateInputConnection()'s outAttrs.inputType is password type or not,
so we fill out outAttrs.inputType and early out when there is a reentrance.

BUG=636197
==========

to

==========
Ignore reentrancy for ImeThread triggering

In the delayed activation of input connection, we call requestFocus()
on proxy view, and this triggers HwSecImmHelper.isPasswordInputType()
on Huawei phone, which in turn calls View#onCreateInputConnection() so
there is an infinite loop, which eventually causes stackoverflow.

Presumably isPasswordInputType() is only concerned about whether
onCreateInputConnection()'s outAttrs.inputType is password type or not,
so we fill out outAttrs.inputType and early out when there is a reentrance.

BUG=636197
==========

[commit-bot: I haz the power, 2016-08-30 06:25:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-30 06:29:10]

Description was changed from

==========
Ignore reentrancy for ImeThread triggering

In the delayed activation of input connection, we call requestFocus()
on proxy view, and this triggers HwSecImmHelper.isPasswordInputType()
on Huawei phone, which in turn calls View#onCreateInputConnection() so
there is an infinite loop, which eventually causes stackoverflow.

Presumably isPasswordInputType() is only concerned about whether
onCreateInputConnection()'s outAttrs.inputType is password type or not,
so we fill out outAttrs.inputType and early out when there is a reentrance.

BUG=636197
==========

to

==========
Ignore reentrancy for ImeThread triggering

In the delayed activation of input connection, we call requestFocus()
on proxy view, and this triggers HwSecImmHelper.isPasswordInputType()
on Huawei phone, which in turn calls View#onCreateInputConnection() so
there is an infinite loop, which eventually causes stackoverflow.

Presumably isPasswordInputType() is only concerned about whether
onCreateInputConnection()'s outAttrs.inputType is password type or not,
so we fill out outAttrs.inputType and early out when there is a reentrance.

BUG=636197

Committed: https://crrev.com/f5feceb259130400bc3f05826c8bc625f3399794
Cr-Commit-Position: refs/heads/master@{#415220}
==========

[commit-bot: I haz the power, 2016-08-30 06:29:12]

Patchset 3 (id:??) landed as
https://crrev.com/f5feceb259130400bc3f05826c8bc625f3399794
Cr-Commit-Position: refs/heads/master@{#415220}