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

Issue 829513 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR



Sign in to add a comment

VR: WebVR spinner showed up for non-WebVR page

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

Issue description

On ToT, I entered VR through headset insertion with the DON flow skipped on chrome://history and encountered the WebVR spinner.

I had 3 tabs, the first was on a webVR page, the last was the history page.

 
Just saw this again. Happened when I was looking at the NTP. WebVR page was tab 3 of 5...
Owner: mthiesse@chromium.org
Status: Started (was: Available)
I've got a speculative fix.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 11 2018

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

commit f2ccbfd1e44fdbe033d1432a2d78f3da5d8e6fbc
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Apr 11 02:28:02 2018

VR: Clean up WebVR headset insertion.

This CL makes it explicitly clear when we care about whether the page
was listening for activate before pause, and when we should autopresent
on entering VR (for headset insertion).

This also removes the workaround in vr_display_impl that allowed pages
without focus to handle displayActivate, favoring handling this case
more correctly in VrShellDelegate.

Bug:  829513 
Change-Id: Ie19c55305d0c59ec3f576010ded0f61ae906a084
Reviewed-on: https://chromium-review.googlesource.com/1004927
Reviewed-by: Yash Malik <ymalik@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549736}
[modify] https://crrev.com/f2ccbfd1e44fdbe033d1432a2d78f3da5d8e6fbc/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/f2ccbfd1e44fdbe033d1432a2d78f3da5d8e6fbc/device/vr/vr_device_base.cc
[modify] https://crrev.com/f2ccbfd1e44fdbe033d1432a2d78f3da5d8e6fbc/device/vr/vr_device_base.h
[modify] https://crrev.com/f2ccbfd1e44fdbe033d1432a2d78f3da5d8e6fbc/device/vr/vr_device_base_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 11 2018

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

commit 30b94a8ae4ef23e7ce0e8709db08c04e855d3d59
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Apr 11 15:09:35 2018

Revert "VR: Clean up WebVR headset insertion."

This reverts commit f2ccbfd1e44fdbe033d1432a2d78f3da5d8e6fbc.

Reason for revert: Speculative revert for crbug.com/831589

Original change's description:
> VR: Clean up WebVR headset insertion.
> 
> This CL makes it explicitly clear when we care about whether the page
> was listening for activate before pause, and when we should autopresent
> on entering VR (for headset insertion).
> 
> This also removes the workaround in vr_display_impl that allowed pages
> without focus to handle displayActivate, favoring handling this case
> more correctly in VrShellDelegate.
> 
> Bug:  829513 
> Change-Id: Ie19c55305d0c59ec3f576010ded0f61ae906a084
> Reviewed-on: https://chromium-review.googlesource.com/1004927
> Reviewed-by: Yash Malik <ymalik@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#549736}

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

Change-Id: I92a56254e933381770a56495553facfbcb2e5c59
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  829513 
Reviewed-on: https://chromium-review.googlesource.com/1007374
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549886}
[modify] https://crrev.com/30b94a8ae4ef23e7ce0e8709db08c04e855d3d59/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/30b94a8ae4ef23e7ce0e8709db08c04e855d3d59/device/vr/vr_device_base.cc
[modify] https://crrev.com/30b94a8ae4ef23e7ce0e8709db08c04e855d3d59/device/vr/vr_device_base.h
[modify] https://crrev.com/30b94a8ae4ef23e7ce0e8709db08c04e855d3d59/device/vr/vr_device_base_unittest.cc

Status: Started (was: Fixed)
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 12 2018

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

commit 40a37bce55637d5d1162f2f663e70c990a2750d2
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Thu Apr 12 06:02:10 2018

Reland "VR: Clean up WebVR headset insertion."

This reverts commit 30b94a8ae4ef23e7ce0e8709db08c04e855d3d59.

Original change's description:
> Revert "VR: Clean up WebVR headset insertion."
>
> This reverts commit f2ccbfd1e44fdbe033d1432a2d78f3da5d8e6fbc.
>
> Reason for revert: Speculative revert for crbug.com/831589
>
> Original change's description:
> > VR: Clean up WebVR headset insertion.
> >
> > This CL makes it explicitly clear when we care about whether the page
> > was listening for activate before pause, and when we should autopresent
> > on entering VR (for headset insertion).
> >
> > This also removes the workaround in vr_display_impl that allowed pages
> > without focus to handle displayActivate, favoring handling this case
> > more correctly in VrShellDelegate.
> >
> > Bug:  829513 
> > Change-Id: Ie19c55305d0c59ec3f576010ded0f61ae906a084
> > Reviewed-on: https://chromium-review.googlesource.com/1004927
> > Reviewed-by: Yash Malik <ymalik@chromium.org>
> > Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#549736}
>
> TBR=mthiesse@chromium.org,ymalik@chromium.org
>
> Change-Id: I92a56254e933381770a56495553facfbcb2e5c59
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  829513 
> Reviewed-on: https://chromium-review.googlesource.com/1007374
> Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#549886}

TBR=bajones@chromium.org

Change-Id: I84fa5bd2f187859770166c1a3c79387492b50df1
Bug:  829513 
Reviewed-on: https://chromium-review.googlesource.com/1007922
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Yash Malik <ymalik@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550039}
[modify] https://crrev.com/40a37bce55637d5d1162f2f663e70c990a2750d2/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/40a37bce55637d5d1162f2f663e70c990a2750d2/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/TestVrShellDelegate.java
[modify] https://crrev.com/40a37bce55637d5d1162f2f663e70c990a2750d2/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/WebVrInputTest.java
[modify] https://crrev.com/40a37bce55637d5d1162f2f663e70c990a2750d2/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/NfcSimUtils.java
[modify] https://crrev.com/40a37bce55637d5d1162f2f663e70c990a2750d2/chrome/browser/android/vr/vr_shell_delegate.cc
[modify] https://crrev.com/40a37bce55637d5d1162f2f663e70c990a2750d2/chrome/browser/android/vr/vr_shell_delegate.h
[modify] https://crrev.com/40a37bce55637d5d1162f2f663e70c990a2750d2/device/vr/vr_device_base.cc
[modify] https://crrev.com/40a37bce55637d5d1162f2f663e70c990a2750d2/device/vr/vr_device_base.h
[modify] https://crrev.com/40a37bce55637d5d1162f2f663e70c990a2750d2/device/vr/vr_device_base_unittest.cc
[modify] https://crrev.com/40a37bce55637d5d1162f2f663e70c990a2750d2/third_party/blink/renderer/modules/vr/vr_display.cc

Labels: Merge-Request-67
Project Member

Comment 9 by sheriffbot@chromium.org, Apr 16 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
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
Revert mentioned in comments is a red herring... it's a reland.
Labels: -Hotlist-Merge-Review -Merge-Review-67 Merge-Approved-67
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 18 2018

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

commit 310c5deb116649e9a599dd7f069e51e2c9e1d0f1
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Apr 18 14:16:18 2018

Reland "VR: Clean up WebVR headset insertion."

This reverts commit 30b94a8ae4ef23e7ce0e8709db08c04e855d3d59.

Original change's description:
> Revert "VR: Clean up WebVR headset insertion."
>
> This reverts commit f2ccbfd1e44fdbe033d1432a2d78f3da5d8e6fbc.
>
> Reason for revert: Speculative revert for crbug.com/831589
>
> Original change's description:
> > VR: Clean up WebVR headset insertion.
> >
> > This CL makes it explicitly clear when we care about whether the page
> > was listening for activate before pause, and when we should autopresent
> > on entering VR (for headset insertion).
> >
> > This also removes the workaround in vr_display_impl that allowed pages
> > without focus to handle displayActivate, favoring handling this case
> > more correctly in VrShellDelegate.
> >
> > Bug:  829513 
> > Change-Id: Ie19c55305d0c59ec3f576010ded0f61ae906a084
> > Reviewed-on: https://chromium-review.googlesource.com/1004927
> > Reviewed-by: Yash Malik <ymalik@chromium.org>
> > Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#549736}
>
> TBR=mthiesse@chromium.org,ymalik@chromium.org
>
> Change-Id: I92a56254e933381770a56495553facfbcb2e5c59
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  829513 
> Reviewed-on: https://chromium-review.googlesource.com/1007374
> Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#549886}

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

(cherry picked from commit 40a37bce55637d5d1162f2f663e70c990a2750d2)

Change-Id: I84fa5bd2f187859770166c1a3c79387492b50df1
Bug:  829513 
Reviewed-on: https://chromium-review.googlesource.com/1007922
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Yash Malik <ymalik@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550039}
Reviewed-on: https://chromium-review.googlesource.com/1016686
Cr-Commit-Position: refs/branch-heads/3396@{#77}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/310c5deb116649e9a599dd7f069e51e2c9e1d0f1/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/TestVrShellDelegate.java

Status: Fixed (was: Started)
Labels: Test-Complete

Sign in to add a comment