New issue
Advanced search Search tips

Issue 762724 link

Starred by 2 users

Issue metadata

Status: ExternalDependency
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-XR

Blocking:
issue 734611



Sign in to add a comment

VR entry flaky on swarmed Pixel XLs

Project Member Reported by bsheedy@chromium.org, Sep 6 2017

Issue description

Now that the FYI bot is finally green again, I'm occasionally seeing tests fail on the swarmed Pixel XLs due to waitForVrEntry() timing out (10 second timeout).

Example failures:
https://chromium-swarm.appspot.com/task?id=3871571f2c31be10&refresh=10&show_raw=1
https://chromium-swarm.appspot.com/task?id=3872031944fb9c10&refresh=10&show_raw=1
 
It looks like this is somehow being caused by the temporary fix for  Issue 754342 . When VR entry fails, enterVrInternal() is returning early due to mWaitingForVrTimeout being set.

This would imply that Chrome is somehow getting paused, as otherwise mWaitingForVrTimeout should never get set.

I was able to reproduce locally on a Pixel w/ N using the following:

out-gn/bot/bin/run_chrome_public_test_vr_apk --shared-prefs-file chrome/android/shared_preference_files/test/vr_ddview_skipdon_setupcomplete.json --test-filter VrFeedbackInfoBarTest#*

It doesn't happen very frequently, though, so you'll need to either use the --repeat <#> argument or a bash loop. Note that I was having issues with logcat output for the failing test run showing up if several other (3-5?) test runs were run between the failure and when I checked the logs, so you may want to keep the number of iterations low.
Owner: ----
Status: Available (was: Started)
Update on this:

In both the case where a test passes and fails, Chrome is getting paused due to DonPrepareActivity getting launched, which causes mProbablyInDon to be set, so that's not inherently the issue.

The problem is that when the test passes, mDonSucceeded is set to true, so onResume calls into handleDonFlowSuccess. When the test fails, mDonSucceeded is false, but mProbablyInDon is true, so VrShellDelegate thinks the user backed out of the DON flow.
Another update:

This appears to be due to a race condition with receiving the broadcast from DonPrepareActivity. We apparently sometimes get resumed before receiving the broadcast, causing us to think that the DON flow didn't succeed.
b/65681875 has been filed to see if this can be fixed on VrCore's end. In the meantime, I'll set the tests that hit this to be retried on failure.
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 14 2017

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

commit ef5b46eeccc2e562be574a771b5635897aa91798
Author: bsheedy <bsheedy@chromium.org>
Date: Thu Sep 14 21:03:11 2017

Make VrFeedbackInfoBarTests retry on failure

Sets the tests in VrFeedbackInfoBarTest.java to retry on failure since
their rapid entering and exiting of VR makes them prone to hitting the
VR entry race condition that was recently discovered.

Bug: 762724
Change-Id: I89de45489be25f5bc15155341c90c9f9b614da1a
Reviewed-on: https://chromium-review.googlesource.com/667874
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502040}
[modify] https://crrev.com/ef5b46eeccc2e562be574a771b5635897aa91798/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrFeedbackInfoBarTest.java

Status: Assigned (was: Available)
Brian, could you close this bug off it's no longer relevant?  If it is, just return to Available state.  Thanks!
Status: Fixed (was: Assigned)
Labels: Test-Complete
Status: ExternalDependency (was: Fixed)
Re-opening this since we've started running into issues with receiving the broadcast after being resumed again.
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 25 2018

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

commit ffff67f6f055730e24c384f92215f2326fecb3ce
Author: bsheedy <bsheedy@chromium.org>
Date: Mon Jun 25 17:34:18 2018

Add VR workarounds for early entry

Adds two workarounds for Chrome being resumed or started too early when
entering VR. These can be removed if/when the platform-side issue that
causes these issues is fixed in VrCore.

Bug:  854327 , 762724
Change-Id: If9645c58e9d208a734582992aad5c85d378f5f40
Reviewed-on: https://chromium-review.googlesource.com/1112761
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570084}
[modify] https://crrev.com/ffff67f6f055730e24c384f92215f2326fecb3ce/chrome/android/java/src/org/chromium/chrome/browser/vr/VrMainActivity.java
[modify] https://crrev.com/ffff67f6f055730e24c384f92215f2326fecb3ce/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrIntentUtils.java
[modify] https://crrev.com/ffff67f6f055730e24c384f92215f2326fecb3ce/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/TransitionUtils.java

Flakiness should be fixed now, but I'll keep open until the root cause is fixed.
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 25 2018

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

commit 1b3df93c5adbf3768d6e3896a0431314708c973b
Author: Theresa <twellington@chromium.org>
Date: Mon Jun 25 23:21:34 2018

Revert "Add VR workarounds for early entry"

This reverts commit ffff67f6f055730e24c384f92215f2326fecb3ce.

Reason for revert: Suspected to be causing test failures on Nougat bot

Bug:  854327 , 762724, 856373

Original change's description:
> Add VR workarounds for early entry
> 
> Adds two workarounds for Chrome being resumed or started too early when
> entering VR. These can be removed if/when the platform-side issue that
> causes these issues is fixed in VrCore.
> 
> Bug:  854327 , 762724
> Change-Id: If9645c58e9d208a734582992aad5c85d378f5f40
> Reviewed-on: https://chromium-review.googlesource.com/1112761
> Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570084}

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

Change-Id: I714637798cc2944a93b33240360fb27fc6dc0ee4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  854327 , 762724
Reviewed-on: https://chromium-review.googlesource.com/1113846
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570231}
[modify] https://crrev.com/1b3df93c5adbf3768d6e3896a0431314708c973b/chrome/android/java/src/org/chromium/chrome/browser/vr/VrMainActivity.java
[modify] https://crrev.com/1b3df93c5adbf3768d6e3896a0431314708c973b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrIntentUtils.java
[modify] https://crrev.com/1b3df93c5adbf3768d6e3896a0431314708c973b/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/TransitionUtils.java

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 26 2018

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

commit 82ba5f3a55697933e46bc391022b0f425db0152f
Author: bsheedy <bsheedy@chromium.org>
Date: Tue Jun 26 16:40:28 2018

Reland "Add VR workarounds for early entry"

This is a reland of ffff67f6f055730e24c384f92215f2326fecb3ce

Original change's description:
> Add VR workarounds for early entry
>
> Adds two workarounds for Chrome being resumed or started too early when
> entering VR. These can be removed if/when the platform-side issue that
> causes these issues is fixed in VrCore.
>
> Bug:  854327 , 762724
> Change-Id: If9645c58e9d208a734582992aad5c85d378f5f40
> Reviewed-on: https://chromium-review.googlesource.com/1112761
> Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570084}

Bug:  854327 , 762724, 856373
Change-Id: Ifc7bd0d0a460fe1defa05a13ce20593b2662babd
Reviewed-on: https://chromium-review.googlesource.com/1114046
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570426}
[modify] https://crrev.com/82ba5f3a55697933e46bc391022b0f425db0152f/chrome/android/java/src/org/chromium/chrome/browser/vr/VrMainActivity.java
[modify] https://crrev.com/82ba5f3a55697933e46bc391022b0f425db0152f/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrIntentUtils.java
[modify] https://crrev.com/82ba5f3a55697933e46bc391022b0f425db0152f/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/TransitionUtils.java

Cc: bsheedy@chromium.org
 Issue 767944  has been merged into this issue.
Components: Internals>XR
Removing Internals>VR component and remapping to Internals>XR
Components: -Internals>VR

Sign in to add a comment