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

Issue 745899 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-XR



Sign in to add a comment

vr/getFrameData_samewithinframe.html is flaky on Linux

Project Member Reported by alph@chromium.org, Jul 18 2017

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Jul 18 2017

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

commit 4a6917c66d843b2aa3978ebbd59b2750c4b62895
Author: Alexei Filippov <alph@chromium.org>
Date: Tue Jul 18 19:42:31 2017

Mark vr/getFrameData_samewithinframe.html as flaky

TBR=rogerta
NOTRY=true

Bug:  745899 
Change-Id: I04c8e24149b278a16e2cf14762ca965c91b8cec3
Reviewed-on: https://chromium-review.googlesource.com/576381
Commit-Queue: Alexei Filippov <alph@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487562}
[modify] https://crrev.com/4a6917c66d843b2aa3978ebbd59b2750c4b62895/third_party/WebKit/LayoutTests/TestExpectations

Cc: bsheedy@chromium.org leilei@chromium.org
Labels: Proj-VR VR-Test
Owner: bsheedy@chromium.org
Status: Assigned (was: Untriaged)
Brian and Lei, could you comment on this bug?  Is it a test issue that we can fix?

Please return to "Available" state, if appropriate, after assessing.  Thanks!
Cc: bajones@chromium.org
Sorry I missed this.

Taking a look at the test, it looks like this assert is failing https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/vr/getFrameData_samewithinframe.html?l=24

We're assuming that if a VRDisplay triggers its requestAnimationFrame, then valid frame data will always be available, and thus getFrameData should always return true as long as:
1. We're passing it a valid VRFrameData object (we are)
2. We're calling it in response to VRDisplay.requestAnimationFrame (we are)

If this assumption is correct, then that means there's a bug somewhere in our code where requestAnimationFrame is getting fired without frame data being available.

If the assumption is not correct, we can change the test slightly to accommodate.

Brandon, do you know whether this assumption is correct?
Spec-wise there's not a strong guarantee that rAF being called always means you have frame data. It's possible on some systems that you've lost tracking, for example. That may not be the case for Chrome's implementation, I'm not sure, but I'd argue that it's not something that should be relied on.

Of course, a test environment where we're synthesizing frame data is a different matter altogether, and we should be able to tightly control if the frame data is available or not, so if it's flaky I'd consider that a bug.
I've started looking into this, and this seems to be a race condition. We are correctly setting the mock pose, and as far as the JavaScript mocking is concerned, the pose has been set before we start doing anything with requestAnimationFrame/getFrameData. However, on the C++ side, we get the following events in chronological order:

RequestVSync
  Asynchronously get the next magic window pose, calling OnMagicWindowPose when done
  request a document animation frame, providing a VRDisplayFrameRequestCallback
VRDisplayFrameRequestCallback::handleRequest triggered
  call OnMagicWindowVSync
  JavaScript gets notified of the animation frame
  getFrameData is called
  getFrameData returns false because there is no pose data
OnMagicWindowPose is called
  pose data is set

A simple solution to this would be to just let one VRDisplay.requestAnimationFrame trigger before expecting any frame data, assuming this is allowed behavior and doesn't need to be fixed.
"A simple solution to this would be to just let one VRDisplay.requestAnimationFrame trigger before expecting any frame data, assuming this is allowed behavior and doesn't need to be fixed." <-- This doesn't sound unreasonable to me.
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 13 2017

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

commit c8a7f48916575d60fa46a0ac97f0afedde77d405
Author: bsheedy <bsheedy@chromium.org>
Date: Wed Sep 13 20:30:20 2017

Fix getFrameData_samewithinframe flakiness

Fixes flakiness with the vr/getFrameData_samewithinframe layout test
that got it disabled. This seems to have been due to a race condition
where the JavaScript mocking properly set the frame data, but it
wouldn't show up in C++ in time due to other JavaScript code being run.

Bug:  745899 
Change-Id: I5d0228a18b09ebf6b97d0d8fc499f71845cc3f56
Reviewed-on: https://chromium-review.googlesource.com/662961
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501738}
[modify] https://crrev.com/c8a7f48916575d60fa46a0ac97f0afedde77d405/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/c8a7f48916575d60fa46a0ac97f0afedde77d405/third_party/WebKit/LayoutTests/vr/getFrameData_samewithinframe.html

Status: Fixed (was: Assigned)
The flakiness should now be fixed, and the test re-enabled. No failures in any of the VR tests on Linux after 100 runs, so should be good to go.
Components: Internals>XR

Sign in to add a comment