New issue
Advanced search Search tips

Issue 838358 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

VR: Lock Chrome to the primary display on Standalones VR devices

Project Member Reported by mthiesse@chromium.org, Apr 30 2018

Issue description

Full context is in b/78108624.

Basically, Chrome crashes when launched on Standalones because we end up getting moved from the virtual display to the real display when toggling VR mode.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 1 2018

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

commit 1abb7599fff54f64f4a7b09ee948c18fb0cb0222
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Tue May 01 15:05:35 2018

VR: Ensure Chrome launches onto the primary display for VR Standlones

There's a platform bug with Standalone VR devices where transitioning
between the virtual and primary display causes crashes in the
framework. This means that currently (M67) Chrome crashes 100% of the
time on launch, regardless of how Chrome was launched, on Standalones.

I'm not 100% clear on how moving between the displays works on
standalones as it's complicated. Here are the things I know:
1. 2D apps by default launch to the virtual display.
2. Toggling VR mode can cause your Activity to be moved to the primary,
real, display.
3. Moving between displays is causing platform crashes due to the
display being cached in the View Root, and probably elsewhere.

The recommended workaround by the platform folks is to just make sure
Chrome is never launched onto the Virtual Display. This has some
implications. The main implication is that the Chrome window when using
the 2D-in-VR path is super tall as they have to keep text readable with
the app rendering at real display size. When we ship browsing on
standalones, you'll never see the tall window anyways, so it won't be
an issue in the future.

I would provide screenshots, but that's impossible :)

Bug:  838358 
Change-Id: I618a01e9e020dec2abcc8f1e65ce7a24842c2113
Reviewed-on: https://chromium-review.googlesource.com/1036117
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555040}
[modify] https://crrev.com/1abb7599fff54f64f4a7b09ee948c18fb0cb0222/base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java
[modify] https://crrev.com/1abb7599fff54f64f4a7b09ee948c18fb0cb0222/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/1abb7599fff54f64f4a7b09ee948c18fb0cb0222/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrIntentUtils.java

Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, May 2 2018

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

commit 36ab219a5c0160c5b85b25191856ad588fd7920a
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed May 02 21:25:21 2018

VR: Fix NPE on x86 devices.

Oversight where I forgot to check if the VrClassesWrapper was null.

TBR=tedchoc@chromium.org

Bug: 838840,  838358 
Change-Id: I87e562a50a34fe755572eb7176eea2ce4e2530ed
Reviewed-on: https://chromium-review.googlesource.com/1040427
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555556}
[modify] https://crrev.com/36ab219a5c0160c5b85b25191856ad588fd7920a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/36ab219a5c0160c5b85b25191856ad588fd7920a/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java

Project Member

Comment 4 by bugdroid1@chromium.org, May 2 2018

Labels: merge-merged-3417
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18055b01394bf7ba1602305fd31a8ce67c89842e

commit 18055b01394bf7ba1602305fd31a8ce67c89842e
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed May 02 21:53:16 2018

VR: Fix NPE on x86 devices.

Oversight where I forgot to check if the VrClassesWrapper was null.

TBR=mthiesse@chromium.org, tedchoc@chromium.org

(cherry picked from commit 36ab219a5c0160c5b85b25191856ad588fd7920a)

Bug: 838840,  838358 
Change-Id: I87e562a50a34fe755572eb7176eea2ce4e2530ed
Reviewed-on: https://chromium-review.googlesource.com/1040427
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555556}
Reviewed-on: https://chromium-review.googlesource.com/1040805
Cr-Commit-Position: refs/branch-heads/3417@{#4}
Cr-Branched-From: dd2d3effe08392fcd5277bf052c2d3ebbeaf8cd1-refs/heads/master@{#555217}
[modify] https://crrev.com/18055b01394bf7ba1602305fd31a8ce67c89842e/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/18055b01394bf7ba1602305fd31a8ce67c89842e/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java

Labels: Merge-Request-67
Verified on Canary.
Project Member

Comment 6 by sheriffbot@chromium.org, May 7 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 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), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-67 Merge-Approved-67
Project Member

Comment 8 by bugdroid1@chromium.org, May 8 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fd889939a105428bed46377ab1c685fa58923e6a

commit fd889939a105428bed46377ab1c685fa58923e6a
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Tue May 08 18:24:26 2018

VR: Ensure Chrome launches onto the primary display for VR Standlones

There's a platform bug with Standalone VR devices where transitioning
between the virtual and primary display causes crashes in the
framework. This means that currently (M67) Chrome crashes 100% of the
time on launch, regardless of how Chrome was launched, on Standalones.

I'm not 100% clear on how moving between the displays works on
standalones as it's complicated. Here are the things I know:
1. 2D apps by default launch to the virtual display.
2. Toggling VR mode can cause your Activity to be moved to the primary,
real, display.
3. Moving between displays is causing platform crashes due to the
display being cached in the View Root, and probably elsewhere.

The recommended workaround by the platform folks is to just make sure
Chrome is never launched onto the Virtual Display. This has some
implications. The main implication is that the Chrome window when using
the 2D-in-VR path is super tall as they have to keep text readable with
the app rendering at real display size. When we ship browsing on
standalones, you'll never see the tall window anyways, so it won't be
an issue in the future.

I would provide screenshots, but that's impossible :)

TBR=mthiesse@chromium.org

(cherry picked from commit 1abb7599fff54f64f4a7b09ee948c18fb0cb0222)

Bug:  838358 
Change-Id: I618a01e9e020dec2abcc8f1e65ce7a24842c2113
Reviewed-on: https://chromium-review.googlesource.com/1036117
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555040}
Reviewed-on: https://chromium-review.googlesource.com/1050471
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#523}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/fd889939a105428bed46377ab1c685fa58923e6a/base/android/java/src/org/chromium/base/ApiCompatibilityUtils.java
[modify] https://crrev.com/fd889939a105428bed46377ab1c685fa58923e6a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/fd889939a105428bed46377ab1c685fa58923e6a/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrIntentUtils.java

Project Member

Comment 9 by bugdroid1@chromium.org, May 8 2018

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

commit 32784c411ec1b41328c71bafb05edf5462ed7d71
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Tue May 08 18:25:40 2018

VR: Fix NPE on x86 devices.

Oversight where I forgot to check if the VrClassesWrapper was null.

TBR=mthiesse@chromium.org, tedchoc@chromium.org

(cherry picked from commit 36ab219a5c0160c5b85b25191856ad588fd7920a)

Bug: 838840,  838358 
Change-Id: I87e562a50a34fe755572eb7176eea2ce4e2530ed
Reviewed-on: https://chromium-review.googlesource.com/1040427
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555556}
Reviewed-on: https://chromium-review.googlesource.com/1050571
Cr-Commit-Position: refs/branch-heads/3396@{#524}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/32784c411ec1b41328c71bafb05edf5462ed7d71/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/32784c411ec1b41328c71bafb05edf5462ed7d71/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java

Status: Fixed (was: Started)
Labels: Test-Complete
Status: Verified (was: Fixed)
Ok.  it's working.  Both Chrome 2D for 67 and VR Browsing for 68 launch without crashing on build 67.0.3396.41 and 68.0.3425.0 respectively.

Components: Internals>XR

Sign in to add a comment