Showing virtual keyboard when input type changed

Showing virtual keyboard when input type changed

This CL tries showing virtual keyboard everytime ARC reports that text
input type is changed, even when the type isn't actually changed.

BUG=581282
BUG=b/27747791
BUG=b/28677278

Committed: https://crrev.com/d9457f17224b5eec57c5d974213ed58591595837
Cr-Commit-Position: refs/heads/master@{#399672}

[takaoka, 2016-06-07 08:50:59]

Description was changed from

==========
Showing virtual keyboard when input type changed

This CL tries showing virtual keyboard everytime ARC reports that text
input type is changed, even when the type isn't actually changed.

BUG=581282
BUG=b/27747791
BUG=b/28677278

Reports virtual keyboard bounds change to ARC

BUG=581282
BUG=b/27747791
BUG=b/28677278
==========

to

==========
Showing virtual keyboard when input type changed

This CL tries showing virtual keyboard everytime ARC reports that text
input type is changed, even when the type isn't actually changed.

BUG=581282
BUG=b/27747791
BUG=b/28677278
==========

[takaoka, 2016-06-07 08:51:13]

Patchset #1 (id:1) has been deleted

[takaoka, 2016-06-14 04:05:48]

Patchset #1 (id:20001) has been deleted

[takaoka, 2016-06-14 04:12:21]

takaoka@chromium.org changed reviewers:
+ dcheng@chromium.org, hidehiko@chromium.org, kinaba@chromium.org,
rickyz@chromium.org

[takaoka, 2016-06-14 04:12:22]

PTAL?

[kinaba, 2016-06-14 04:22:08]

https://codereview.chromium.org/2039233003/diff/40001/components/arc/ime/arc_...
File components/arc/ime/arc_ime_service.cc (left):

https://codereview.chromium.org/2039233003/diff/40001/components/arc/ime/arc_...
components/arc/ime/arc_ime_service.cc:129: if (ime_type_ == type)
Although it's a matter of taste, can we keep this early exit logic as-is?

https://codereview.chromium.org/2039233003/diff/40001/components/arc/ime/arc_...
components/arc/ime/arc_ime_service.cc:143: input_method->ShowImeIfNeeded();
Removing this code at once causes a regression in New-Chrome + Old-ARC
combination.

Is it possible to leave this code as is for a while (I hope it's harmless),
at least until we get new ARC image rolled to the Chrome OS build.

[takaoka, 2016-06-14 05:55:51]

PTAL at Patch Set 2?

https://codereview.chromium.org/2039233003/diff/40001/components/arc/ime/arc_...
File components/arc/ime/arc_ime_service.cc (left):

https://codereview.chromium.org/2039233003/diff/40001/components/arc/ime/arc_...
components/arc/ime/arc_ime_service.cc:129: if (ime_type_ == type)
On 2016/06/14 04:22:08, kinaba wrote:
> Although it's a matter of taste, can we keep this early exit logic as-is?

Done.

https://codereview.chromium.org/2039233003/diff/40001/components/arc/ime/arc_...
components/arc/ime/arc_ime_service.cc:143: input_method->ShowImeIfNeeded();
On 2016/06/14 04:22:08, kinaba wrote:
> Removing this code at once causes a regression in New-Chrome + Old-ARC
> combination.
> 
> Is it possible to leave this code as is for a while (I hope it's harmless),
> at least until we get new ARC image rolled to the Chrome OS build.
> 

Done.

[kinaba, 2016-06-14 05:56:58]

lgtm

[hidehiko, 2016-06-14 06:06:34]

lgtm

[dcheng, 2016-06-14 08:39:52]

mojom lgtm with a question to help confirm I understand what's going on =)

https://codereview.chromium.org/2039233003/diff/60001/components/arc/ime/arc_...
File components/arc/ime/arc_ime_service.cc (right):

https://codereview.chromium.org/2039233003/diff/60001/components/arc/ime/arc_...
components/arc/ime/arc_ime_service.cc:139: ime_type_ !=
ui::TEXT_INPUT_TYPE_NONE) {
So the final intent is to remove this once all the IME state is synced, right?

[takaoka, 2016-06-14 09:04:25]

https://codereview.chromium.org/2039233003/diff/60001/components/arc/ime/arc_...
File components/arc/ime/arc_ime_service.cc (right):

https://codereview.chromium.org/2039233003/diff/60001/components/arc/ime/arc_...
components/arc/ime/arc_ime_service.cc:139: ime_type_ !=
ui::TEXT_INPUT_TYPE_NONE) {
On 2016/06/14 08:39:52, dcheng wrote:
> So the final intent is to remove this once all the IME state is synced, right?

Yes.
This can be removed once the corresponding CL on Android side (ag/1113933) has
been merged, new ShowImeIfNeeded RPC will be called from Android side as needed.

[takaoka, 2016-06-14 09:04:40]

The CQ bit was checked by takaoka@chromium.org

[commit-bot: I haz the power, 2016-06-14 09:04:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2039233003/60001

[commit-bot: I haz the power, 2016-06-14 09:08:58]

Description was changed from

==========
Showing virtual keyboard when input type changed

This CL tries showing virtual keyboard everytime ARC reports that text
input type is changed, even when the type isn't actually changed.

BUG=581282
BUG=b/27747791
BUG=b/28677278
==========

to

==========
Showing virtual keyboard when input type changed

This CL tries showing virtual keyboard everytime ARC reports that text
input type is changed, even when the type isn't actually changed.

BUG=581282
BUG=b/27747791
BUG=b/28677278
==========

[commit-bot: I haz the power, 2016-06-14 09:08:59]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-06-14 09:10:54]

Description was changed from

==========
Showing virtual keyboard when input type changed

This CL tries showing virtual keyboard everytime ARC reports that text
input type is changed, even when the type isn't actually changed.

BUG=581282
BUG=b/27747791
BUG=b/28677278
==========

to

==========
Showing virtual keyboard when input type changed

This CL tries showing virtual keyboard everytime ARC reports that text
input type is changed, even when the type isn't actually changed.

BUG=581282
BUG=b/27747791
BUG=b/28677278

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

[commit-bot: I haz the power, 2016-06-14 09:10:56]

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