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

Issue 799584 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
(currently inactive on Chromium)
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR

Blocking:
issue 641470



Sign in to add a comment

VR: Prompt the user if the keyboard APK is not installed or out of date

Project Member Reported by ymalik@chromium.org, Jan 5 2018

Issue description

This happens when the user manually uninstalls the keyboard APK.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 6 2018

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

commit 35d4a2da8c8e559bfc20cc633d0e8f3533edc0dc
Author: Yash Malik <ymalik@google.com>
Date: Sat Jan 06 18:27:18 2018

VR: Add strings for the keyboard not installed prompt

TBR: rockot
Bug:  799584 
Change-Id: I4759e01e75070c0d4fe4f56506b8a48497696f1b
Reviewed-on: https://chromium-review.googlesource.com/853072
Reviewed-by: Yash Malik <ymalik@chromium.org>
Commit-Queue: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527522}
[modify] https://crrev.com/35d4a2da8c8e559bfc20cc633d0e8f3533edc0dc/chrome/app/generated_resources.grd

Comment 2 by ymalik@chromium.org, Jan 24 2018

Summary: VR: Prompt the user if the keyboard APK is not installed or out of date (was: VR: Prompt the user if the keyboard APK is not installed)

Comment 3 by ymalik@chromium.org, Jan 24 2018

We should also show some UI if the dynamic loading of the keyboard fails for any other reason.
From discussion with Gordon, we should also exit VR and prompt the update, or whatever needs to be done. We can use a pattern similar to permission dialogs.
Labels: -M-65 M-66 OS-Android

Comment 6 Deleted

Based on discussion, the only feasible technical solution is to force VR keyboard update iff there is a breaking change or feature required in a new keyboard version.

UX recommendation for how to handle this:

1. (preferred approach) Allow user to go through DON and enter Chrome VR. Clicking any element that would ordinarily trigger keyboard (Omnibox or web input) will instead trigger a modal infobar prompting user to update the keyboard. Essentially, the Bare Bones Browser flow is still supported if the required keyboard is not present (you can view webpages, but you cannot navigate).

2. (alternative approach) User goes through DON. Upon entering Chrome VR, we immediately prompt user to install keyboard (no option to cancel). User is then DOFFed and in 2D, we intent them to Play Store to install keyboard.

We think approach #1 will result in a marginally better user experience. During discussion, #1 was thought to be the same effort as #2. In either case, we should use the same modal infobar component that we use for prompting voice input audio access.

Assumptions / constraints / givens:

- We can force keyboard update when we need to, but not all keyboard releases require a force-update. 
- Keyboard update is only possible via a DOFF and intent to Google Play Store.
- We can only trigger a check once the user has gone through the DON flow and entered VR. This precludes us from taking the approach of prompting while still in 2D, the way VRCore does when keyboard is missing.

(Recap of discussion with ymalik, mthiesse, ericde)
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 2 2018

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

commit d68a05a9d5eb7dbe2280b5838d379f8807cae22d
Author: Yash Malik <ymalik@google.com>
Date: Fri Mar 02 01:06:54 2018

VR: Add java-side plumbing for updating GVR Keyboard

Note that this path is not exercised yet.

Bug:  799584 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I45f34d54eabcbd539c17d3d87ba8c029489a0f28
Reviewed-on: https://chromium-review.googlesource.com/944750
Commit-Queue: Yash Malik <ymalik@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540391}
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/browser/android/vr/gvr_keyboard_delegate.cc
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/browser/android/vr/gvr_keyboard_shim.cc
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/browser/android/vr/vr_gl_thread.cc
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/browser/android/vr/vr_shell.cc
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/browser/vr/model/modal_prompt_type.cc
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/browser/vr/model/modal_prompt_type.h
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/browser/vr/model/model.h
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/browser/vr/ui.cc
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/browser/vr/ui.h
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/chrome/browser/vr/ui_unsupported_mode.h
[modify] https://crrev.com/d68a05a9d5eb7dbe2280b5838d379f8807cae22d/tools/metrics/histograms/enums.xml

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 2 2018

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

commit dd8e1f56da5ef6df38c8d29905f840703f8b7b1d
Author: Yash Malik <ymalik@google.com>
Date: Fri Mar 02 18:59:58 2018

VR: Prompt user to update their keyboard version if needed

This CL renames AudioPermissionPrompt and its corresponding texture to
Prompt and uses the same texture for the exit prompt.

Bug:  799584 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: If435488f3e10a2f704d7e4df2177756e4d7aaa15
Reviewed-on: https://chromium-review.googlesource.com/945110
Commit-Queue: Yash Malik <ymalik@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540573}
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/BUILD.gn
[delete] https://crrev.com/b187fe86f9c11aa4c3f2bcffa51a6eb9b0cd9875/chrome/browser/vr/elements/audio_permission_prompt.cc
[delete] https://crrev.com/b187fe86f9c11aa4c3f2bcffa51a6eb9b0cd9875/chrome/browser/vr/elements/audio_permission_prompt.h
[delete] https://crrev.com/b187fe86f9c11aa4c3f2bcffa51a6eb9b0cd9875/chrome/browser/vr/elements/audio_permission_prompt_texture.h
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/elements/exit_prompt_texture.h
[add] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/elements/prompt.cc
[add] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/elements/prompt.h
[rename] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/elements/prompt_texture.cc
[add] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/elements/prompt_texture.h
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/elements/ui_element_name.cc
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/elements/ui_element_name.h
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/elements/ui_element_type.cc
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/elements/ui_element_type.h
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/model/color_scheme.cc
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/model/color_scheme.h
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/ui.cc
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/ui_input_manager_unittest.cc
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/ui_scene_constants.h
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/ui_scene_creator.cc
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/ui_scene_creator.h
[modify] https://crrev.com/dd8e1f56da5ef6df38c8d29905f840703f8b7b1d/chrome/browser/vr/ui_unittest.cc

Labels: Merge-Request-66
Project Member

Comment 11 by sheriffbot@chromium.org, Mar 5 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: There is .grd file changes and we are only 42 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Note that the string was added well before the feature freeze deadline for m66 (Feb 16).
Rationale for merge request:

Background: Chrome has an external dependency on the GVR Keyboard apk which renders the keyboard in VR mode. We found a bug in their code where selection was completely broken (b/72506018). A fix was rolled into the latest keyboard APK. This bug tracks forcing the user to upgrade their keyboard version if necessary. 

Note that this bug was originally intended to only prompt when no keyboard apk was found, which was an edge case. The priority is significantly bumped after the bug was found.

Now as for the change itself, although it look a bit lengthy, its mostly just trivial plumbing and renaming. The logic changes are pretty trivial where we add code to show our already implemented UI dialog when the keyboard is out of date. The java side changes are again boilerplate plumbing to intent to the play store to install the apk. All the changes are contained within VR, and should have no effect outside of VR mode and rest of Chrome on Android. That is, the benefit to risk ratio is pretty high, hence the merge request.


Cc: cma...@chromium.org

Comment 15 by cmasso@google.com, Mar 6 2018

Labels: -Hotlist-Merge-Review -Merge-Review-66 Merge-Approved-66
Project Member

Comment 16 by bugdroid1@chromium.org, Mar 7 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/869f88ff770f5122cded208071c6e7a321bdde24

commit 869f88ff770f5122cded208071c6e7a321bdde24
Author: Yash Malik <ymalik@google.com>
Date: Wed Mar 07 04:05:39 2018

VR: Add java-side plumbing for updating GVR Keyboard

Note that this path is not exercised yet.

Bug:  799584 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I45f34d54eabcbd539c17d3d87ba8c029489a0f28
Reviewed-on: https://chromium-review.googlesource.com/944750
Commit-Queue: Yash Malik <ymalik@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#540391}(cherry picked from commit d68a05a9d5eb7dbe2280b5838d379f8807cae22d)
Reviewed-on: https://chromium-review.googlesource.com/952644
Reviewed-by: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#52}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/android/vr/gvr_keyboard_delegate.cc
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/android/vr/gvr_keyboard_shim.cc
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/android/vr/vr_gl_thread.cc
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/android/vr/vr_shell.cc
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/vr/model/modal_prompt_type.cc
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/vr/model/modal_prompt_type.h
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/vr/model/model.h
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/vr/ui.cc
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/vr/ui.h
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/vr/ui_unsupported_mode.h
[modify] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/tools/metrics/histograms/enums.xml

Project Member

Comment 17 by bugdroid1@chromium.org, Mar 7 2018

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

commit af670aae40df6b874feb214760d460d3cfcb9875
Author: Yash Malik <ymalik@google.com>
Date: Wed Mar 07 04:06:35 2018

VR: Prompt user to update their keyboard version if needed

This CL renames AudioPermissionPrompt and its corresponding texture to
Prompt and uses the same texture for the exit prompt.

Bug:  799584 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: If435488f3e10a2f704d7e4df2177756e4d7aaa15
Reviewed-on: https://chromium-review.googlesource.com/945110
Commit-Queue: Yash Malik <ymalik@chromium.org>
Reviewed-by: Christopher Grant <cjgrant@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#540573}(cherry picked from commit dd8e1f56da5ef6df38c8d29905f840703f8b7b1d)
Reviewed-on: https://chromium-review.googlesource.com/952745
Reviewed-by: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#53}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/BUILD.gn
[delete] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/vr/elements/audio_permission_prompt.cc
[delete] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/vr/elements/audio_permission_prompt.h
[delete] https://crrev.com/869f88ff770f5122cded208071c6e7a321bdde24/chrome/browser/vr/elements/audio_permission_prompt_texture.h
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/elements/exit_prompt_texture.h
[add] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/elements/prompt.cc
[add] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/elements/prompt.h
[rename] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/elements/prompt_texture.cc
[add] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/elements/prompt_texture.h
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/elements/ui_element_name.cc
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/elements/ui_element_name.h
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/elements/ui_element_type.cc
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/elements/ui_element_type.h
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/model/color_scheme.cc
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/model/color_scheme.h
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/ui.cc
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/ui_input_manager_unittest.cc
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/ui_scene_constants.h
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/ui_scene_creator.cc
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/ui_scene_creator.h
[modify] https://crrev.com/af670aae40df6b874feb214760d460d3cfcb9875/chrome/browser/vr/ui_unittest.cc

Status: Fixed (was: Started)
Labels: Test-Complete
Added test case "Update Keyboard Driver prompt" to "2D UI Handling Test Plan"

Sign in to add a comment