[Remoting Android] Hide soft keyboard button when physical keyboard is connected

[Remoting Android] Hide soft keyboard button when physical keyboard is connected

Currently the soft keyboard button on the desktop actionbar is visible when a
physical keyboard is present, in which case the button has unexpected behavior:

* Soft keyboard doesn't show up since physical keyboard is present.
* Actionbar and status bar won't go away since triggering the IME blocks
  requests to hide the status bar but won't show any soft keyboard.

Neither can be easily fixed due to limitation of available API. So this CL
hides the soft keyboard button when a physical keyboard is present to prevent
unexpected behaviors.

BUG=640426
Committed: https://crrev.com/f0925da1a559aa0d3d7e02423ad22e45a626f0fb
Cr-Commit-Position: refs/heads/master@{#423401}

[Yuwei, 2016-10-05 18:33:09]

yuweih@chromium.org changed reviewers:
+ joedow@chromium.org

[Yuwei, 2016-10-05 18:33:09]

PTAL. Thanks!

[joedow, 2016-10-05 22:50:59]

lgtm after comments addressed and approach validated.

https://codereview.chromium.org/2391993004/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/Desktop.java (right):

https://codereview.chromium.org/2391993004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:83: private
boolean mHasPhysicalKeyboard;
Can you add a comment on how you detect a physical keyboard and any gotchas you
can think of for future engineers?  We are trying to deduce whether a keyboard
exists or not and that might prove wrong for a subset of Android hardware.

https://codereview.chromium.org/2391993004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:230: //
visibility of the keyboard button.
Shorten this to one line if possible:
"Keyboard state must be set after the keyboard icon has been added to the menu."

https://codereview.chromium.org/2391993004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:408: * present.
nit: Shorten to one line if possible...

[Yuwei, 2016-10-06 00:26:50]

It looks like the suggestion we discussed is not very practical. Overriding
InputMethodService is in fact introducing a new input method, which will become
complicated and may have more unknown issues.

Discussed with Jamie. We should still want to check this in since this is the
best we can get before the branch point O_o

https://codereview.chromium.org/2391993004/diff/1/remoting/android/java/src/o...
File remoting/android/java/src/org/chromium/chromoting/Desktop.java (right):

https://codereview.chromium.org/2391993004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:83: private
boolean mHasPhysicalKeyboard;
On 2016/10/05 22:50:59, joedow wrote:
> Can you add a comment on how you detect a physical keyboard and any gotchas
you
> can think of for future engineers?  We are trying to deduce whether a keyboard
> exists or not and that might prove wrong for a subset of Android hardware.

Done.

https://codereview.chromium.org/2391993004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:230: //
visibility of the keyboard button.
On 2016/10/05 22:50:59, joedow wrote:
> Shorten this to one line if possible:
> "Keyboard state must be set after the keyboard icon has been added to the
menu."

Done.

https://codereview.chromium.org/2391993004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:243:
mHasPhysicalKeyboard = (keyboardState & Configuration.KEYBOARD_NOKEYS) == 0;
Oops. Looks like keyboardState is just an integer enum, not bit flags...

https://codereview.chromium.org/2391993004/diff/1/remoting/android/java/src/o...
remoting/android/java/src/org/chromium/chromoting/Desktop.java:408: * present.
On 2016/10/05 22:50:59, joedow wrote:
> nit: Shorten to one line if possible...

Done.

[joedow, 2016-10-06 00:50:43]

Thanks for looking into the override mechanism, at first it seemed reasonable
but it was definitely more involved than I had expected.  This approach is
reasonable as I don't think people will be switching between physical and
software keyboards often.

[Yuwei, 2016-10-06 02:14:07]

The CQ bit was checked by yuweih@chromium.org

[Yuwei, 2016-10-06 02:14:07]

The patchset sent to the CQ was uploaded after l-g-t-m from joedow@chromium.org
Link to the patchset: https://codereview.chromium.org/2391993004/#ps60001
(title: "Only hide the button for QWERTY keyboard")

[commit-bot: I haz the power, 2016-10-06 02:14:50]

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-10-06 02:25:14]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-06 02:28:09]

Description was changed from

==========
[Remoting Android] Hide soft keyboard button when physical keyboard is connected

Currently the soft keyboard button on the desktop actionbar is visible when a
physical keyboard is present, in which case the button has unexpected behavior:

* Soft keyboard doesn't show up since physical keyboard is present.
* Actionbar and status bar won't go away since triggering the IME blocks
  requests to hide the status bar but won't show any soft keyboard.

Neither can be easily fixed due to limitation of available API. So this CL
hides the soft keyboard button when a physical keyboard is present to prevent
unexpected behaviors.

BUG=640426
==========

to

==========
[Remoting Android] Hide soft keyboard button when physical keyboard is connected

Currently the soft keyboard button on the desktop actionbar is visible when a
physical keyboard is present, in which case the button has unexpected behavior:

* Soft keyboard doesn't show up since physical keyboard is present.
* Actionbar and status bar won't go away since triggering the IME blocks
  requests to hide the status bar but won't show any soft keyboard.

Neither can be easily fixed due to limitation of available API. So this CL
hides the soft keyboard button when a physical keyboard is present to prevent
unexpected behaviors.

BUG=640426

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

[commit-bot: I haz the power, 2016-10-06 02:28:10]

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