New issue
Advanced search Search tips

Issue 821990 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-XR
Proj-XR-VR



Sign in to add a comment

VR: Chrome crashes when selecting text in input field

Project Member Reported by vsupruniuk@google.com, Mar 14 2018

Issue description

Chrome Version: 66.0.3359.30 Beta
Device: Pixel XL
OS: Android N

Reproduction steps are not obvious:
1. Enable flags:
	#vr-browsing-native-android-ui
	#vr-web-input-editing
2. Access www.google.com on the test device
3. Enter Chrome VR
4. Click Search input field using Daydream controller
5. Type ‘qwerty’ in the search field using keyboard click "Enter" to start search
=> Search results are loaded
6. Put cursor in the input field
=> Make sure suggestions dropdown appears
7. Select one of the items in the suggestions dropdown
8. Immediately put the cursor back in the search field and click multiple times using controller selecting/deselecting search phrase

What is the expected result?
- Search text is selected or deselected based on it's current state

What happens instead?
Chrome crashes

Reproducibility is ~50%. Repro steps might seen confusing, basically what you need to do:
1. Type some text in the search field
2. Select one of the search suggestions
3. Start clicking in search field to select/deselect search phrase
4. Repeat steps 2-3 in case crash doesn't repro

Issue is also reproducible in 66.0.3359.28

Crash Report ID 4b094de8697a7e70
 
Labels: -M-66
Labels: M-66
Owner: cjgrant@chromium.org
Status: Assigned (was: Untriaged)
Owner: ----
Status: Available (was: Assigned)
Oh, wait this isn't omnibox. Not sure who should take this while Yash is out...
Owner: billorr@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 15 2018

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

commit e6ba0e6c1ad28cd08047fbb5242f3a602185558a
Author: Bill Orr <billorr@chromium.org>
Date: Thu Mar 15 01:04:22 2018

VR: Chrome crashes when selecting text in input field

VrInputConnection stores a callback to call when a request completes. If
another request comes in, the callback is overwritten, and the new
callback will be called.  The second request will not have a callback to
call, breaking assumptions and crashing.

The fix is to queue callbacks to handle multiple outstanding requests.

Bug:  821990 
Change-Id: I937136bc5291805b5a421f162c4ec175d511867b
Reviewed-on: https://chromium-review.googlesource.com/963695
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Bill Orr <billorr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543269}
[modify] https://crrev.com/e6ba0e6c1ad28cd08047fbb5242f3a602185558a/chrome/browser/android/vr/vr_input_connection.cc
[modify] https://crrev.com/e6ba0e6c1ad28cd08047fbb5242f3a602185558a/chrome/browser/android/vr/vr_input_connection.h
[modify] https://crrev.com/e6ba0e6c1ad28cd08047fbb5242f3a602185558a/chrome/browser/vr/content_input_delegate.cc
[modify] https://crrev.com/e6ba0e6c1ad28cd08047fbb5242f3a602185558a/chrome/browser/vr/content_input_delegate.h

Comment 6 by ymalik@chromium.org, Mar 15 2018

Thanks for taking care of this. This is correctly marked M66 and should be merged back in.
Labels: Merge-Request-66
Project Member

Comment 8 by sheriffbot@chromium.org, Mar 15 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: M66 has already been promoted to the beta branch, so this requires manual review
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
Justification for merge:
This bug fixes a crash in VR input editing, which is shipping in M66.

Why is this low risk:
The fix is specific to VR input editing, and specific to a specific situation so regressions risk is localized to a specific feature area.  The worst case if this introduces a regression is that we are no worse off than taking the fix.  The fix has baked in Canary for a couple days with no noticed regressions.

Comment 10 by cmasso@google.com, Mar 19 2018

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

Comment 11 by bugdroid1@chromium.org, Mar 19 2018

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

commit 1ccc5eb8223f42181464c95083c713e421524c65
Author: Bill Orr <billorr@chromium.org>
Date: Mon Mar 19 17:19:12 2018

VR: Chrome crashes when selecting text in input field

VrInputConnection stores a callback to call when a request completes. If
another request comes in, the callback is overwritten, and the new
callback will be called.  The second request will not have a callback to
call, breaking assumptions and crashing.

The fix is to queue callbacks to handle multiple outstanding requests.

Bug:  821990 
Change-Id: I937136bc5291805b5a421f162c4ec175d511867b
Reviewed-on: https://chromium-review.googlesource.com/963695
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Bill Orr <billorr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#543269}(cherry picked from commit e6ba0e6c1ad28cd08047fbb5242f3a602185558a)
Reviewed-on: https://chromium-review.googlesource.com/969182
Reviewed-by: Bill Orr <billorr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#309}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/1ccc5eb8223f42181464c95083c713e421524c65/chrome/browser/android/vr/vr_input_connection.cc
[modify] https://crrev.com/1ccc5eb8223f42181464c95083c713e421524c65/chrome/browser/android/vr/vr_input_connection.h
[modify] https://crrev.com/1ccc5eb8223f42181464c95083c713e421524c65/chrome/browser/vr/content_input_delegate.cc
[modify] https://crrev.com/1ccc5eb8223f42181464c95083c713e421524c65/chrome/browser/vr/content_input_delegate.h

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
Verified in 66.0.3359.43
Labels: Test-Complete

Sign in to add a comment