Bisect points to https://chromium-review.googlesource.com/1126244 as the culprit CL.

CL has been reverted. Assigning to the author to track the fix.

It seems that the problem is that onCloseAccessorySheet (called from KeyboardAccessoryMediator) is opening the keyboard, and it is not using the InputMethodManagerWrapper set for the ImeAdapter.

I want to disable our feature in VR mode.

Is it sufficient to check for the #enable-webvr feature?
Or is there a run-time check that is more reliable?


Is there a bug about providing generalized and VR-friendly keyboard controls?
As soon as those exist, reenabling should be fine. I am thinking of something like org.chromium.ui.UiUtils which should be marked deprecated or fixed for VR. Otherwise, you will frequently run into this issue.

Please consider reaching with the ImeAdapter and/or UiUtils owners first.

We rather avoid disabling things for VR, but in case it's the only way, we have the VrTabHelper (native).

Hi, can someone explain why we're trying to control show/hide of keyboard from on{Open,Close}{AccessorySheet,KeyboardAccessory}?

As you can see from ImeAdapterImpl logic, it is somewhat complex to get it right on every bit of it. I had a glimpse of memory of what keyboard accessory was trying to achieve, but do you have any design doc around this? (Especially around show/hide logic).

UiUtils usage should be limited to controlling non-web part such as omnibox.

The design doc is go/android-accessory-for-manual-ui for the accessory and go/android-password-fallback for the bottom sheet but the show/hide logic isn't really described - both docs could use an update and some implementation details. 

Could you elaborate on what you mean with non-web part?
The keyboard accessory is a bottom element of the ChromeActivity and only parts of its contents are related to the shown site (e.g. password suggestions or generation).

Oh, about the on{Open,Close}{AccessorySheet,KeyboardAccessory}:
One main use case why keyboard control is necessary is the following (and I will put this in a nicer form into the DD as well).

1. The keyboard pops open
2. The accessory is put on top of it to provide Chrome-specific suggestions that might save the user some typing (e.g. password generation).
3. The user triggers a bottom sheet (e.g. by pressing the key icon)
  --> onOpenAccessorySheet() is called, which closes the keyboard and opens the sheet.
  a. The user selects a suggestion from the bottom sheet.
    --> The suggestion is filled into the focused field
    --> OnCloseKeyboardAccessory() is called, which closes the sheet (no interaction with the keyboard) [1].
  b. The user doesn't find a good suggestion, so they close the bottom sheet again (pressing the key icon again)
    --> OnCloseAccessorySheet() is called, which closes the sheet _and_ brings up the keyboard again - because the user isn't done filling the field yet.


[1] This is the change that the CL introduced which was reverted above. Without this CL, users must close the sheet themselves. Some other edge cases (like closing the sheet when another field is focused) are handled as well, but this shouldn't be the main concern here because there would probably be another way to do this.

Thanks for the pointers.
Normally I wouldn't recommend using UiUtils for web contents because of the following reasons:

1) There is some complexity in determining whether or not to show/hide software keyboard.
2) There are other keyboard types such as floating and physical keyboards, which may not work with UiUtils.isKeyboardShowing().
3) InputConnection status in ImeAdapterImpl is determined separately.

For keyboard accessory, checking the window inset might be the only viable option to check whether keyboard accessory is applicable (what UiUtils.isKeyboardShowing() is already doing). The rationale is that this applies only to the case where soft keyboard is showing at the bottom window.

In this sense, I'm ok with keyboard accessory using UiUtils in general, and disabling it for VR case, as long as you can check the current show/hide status using UiUtils.isKeyboardShowing() at the right time.

For example, does the following case work?

1. Keyboard dismissal / physical keyboard attached.
a. Focus on an element. Keyboard shows up.
b. Trigger keyboard accessory
c. Dismiss keyboard
d. Trigger bottom sheet
e. Close bottom sheet
--> Should not show keyboard after e). Or should dismiss keyboard access after c).

AAMOF, the VR keyboard doesn't take content space. So maybe it is worth trying to use UiUtils.isKeyboardShowing without disabling anything for VR.

Hmm, it looks like this will be very similar to our open task about supporting Hardware keyboards (which we haven't completely figured out yet - we might even need different UI). We should get VR for free once physical works. So, thanks for the thoughts and pointers so far!

So for 69 (because the original patch is definitely needed), I'd not enable the feature in VR, but I'll link this and related bugs to our TB so we keep it in mind for 70+.

As mentioned in a chat, our PM and I definitely want the accessory to work in VR but it's also in a very, very early stage and we first have to focus on getting to a stable version that we can experiment with.

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

commit f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Thu Jul 26 17:01:22 2018

[Android] Close accessory when suggestions were filled

This is a reland of https://chromium-review.googlesource.com/1126244
which was reverted because it promoted a legacy bug which triggered
a 2D keyboard in VR mode.

The keyboard accessory is not enabled in VR mode.
There is a test to ensure that resetting that the sheet doesn't
trigger the keyboard unnecessarily anymore.

Original change's description:
> This CL causes the accessory sheet to close when a user selected a valid
> filling suggestions.
> There is no indicator of the negative case yet (i.e. error during
> filling).

TBR=rouslan@chromium.org

Bug:  856026 , 857592,  865749 
Change-Id: I71abca9366cfa96789ffe7538204f5435332e4ac
Reviewed-on: https://chromium-review.googlesource.com/1146361
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Reviewed-by: Aldo Culquicondor <acondor@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578343}
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryCoordinator.java
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryMediator.java
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryModel.java
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingCoordinator.java
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingMediator.java
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryBridge.java
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillKeyboardAccessoryIntegrationTest.java
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryIntegrationTest.java
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryControllerTest.java
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/browser/android/password_manager/password_accessory_view_android.cc
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/browser/android/password_manager/password_accessory_view_android.h
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a/chrome/browser/password_manager/password_accessory_view_interface.h

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

commit f2c85815f804966792a33e00f5e9ff0fb59ce333
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Thu Aug 02 17:11:01 2018

[Android] Close accessory when suggestions were filled

This is a reland of https://chromium-review.googlesource.com/1126244
which was reverted because it promoted a legacy bug which triggered
a 2D keyboard in VR mode.

The keyboard accessory is not enabled in VR mode.
There is a test to ensure that resetting that the sheet doesn't
trigger the keyboard unnecessarily anymore.

Original change's description:
> This CL causes the accessory sheet to close when a user selected a valid
> filling suggestions.
> There is no indicator of the negative case yet (i.e. error during
> filling).

TBR=rouslan@chromium.org

Bug:  856026 , 857592,  865749 
Change-Id: I71abca9366cfa96789ffe7538204f5435332e4ac
Reviewed-on: https://chromium-review.googlesource.com/1146361
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Reviewed-by: Aldo Culquicondor <acondor@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#578343}(cherry picked from commit f12d4ba6e1e497efa70fbb8d6a76fd36dadf553a)
Reviewed-on: https://chromium-review.googlesource.com/1160981
Cr-Commit-Position: refs/branch-heads/3497@{#340}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryCoordinator.java
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryMediator.java
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryModel.java
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingCoordinator.java
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingMediator.java
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryBridge.java
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillKeyboardAccessoryIntegrationTest.java
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryIntegrationTest.java
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryControllerTest.java
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/browser/android/password_manager/password_accessory_view_android.cc
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/browser/android/password_manager/password_accessory_view_android.h
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/f2c85815f804966792a33e00f5e9ff0fb59ce333/chrome/browser/password_manager/password_accessory_view_interface.h

Reenabling requires more work and will be tracked in issue 902305.