1352446 - Crash in libovrrt32_1.dll@0x209b9

Status: VERIFIED FIXED

(Core :: WebVR, defect)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is 
report bp-3a8e882d-9686-47f4-ab62-acb182170331.
=============================================================

New crash spotted during nightly crash review - crashes started using 20170330030213: http://bit.ly/2oGReaU

Possible regression range based on Build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=272ce6c2572164f5f6a9fba2a980ba9ccf50770c&tochange=60d7a0496a3673450ddbc37ec387525148c32604

Comment 1

[:kip (Kearwood Gilbert)]

I have reproduced this error while debugging from source.

In my case at least, GetShaderResourceView() returned null within VRDisplayOculus::SubmitFrame.

I will add some error handling to this to avoid the crash.  Hopefully this solves the other cases as well.

Comment 2

[:kip (Kearwood Gilbert)]

Adding error handling to the GetShaderResourceView result was not sufficient to prevent the crash.  It crashed elsewhere with that adjustment.

This appears to be affecting 32-bit Firefox Nightly builds, but works fine with the 64-bit builds.

I'll continue investigating deeper.

Comment 3

[:kip (Kearwood Gilbert)]

Attachment: Bug 1352446 - Fix 32-bit build crash with Oculus VR

- Validate GetSharedResourceView in VRDisplayOculus::SubmitFrame and VRDisplayPuppet::SubmitFrame
- Correct size of frameIndex to be "long long" rather than "int" for
  ovr_GetPredictedDisplayTime
- Updated to Oculus 1.15.0 SDK
- Now sending absolute timestamp of the predicted frame time
  to ovr_GetTrackingState rather than a delta

Review commit: https://reviewboard.mozilla.org/r/150030/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150030/

Comment 4

[:kip (Kearwood Gilbert)]

Comment on attachment 8878719 [details]
Bug 1352446 - Fix 32-bit build crash with Oculus VR

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150030/diff/1-2/

Comment 5

[:kip (Kearwood Gilbert)]

The cause of the crash was due to a mismatch of the ovr_SubmitFrame declaration with its implementation in the Oculus dll.

The frameIndex was declared as an "int" in our modified headers, while the latest Oculus SDK declares it as a "long long".  This resulted in a mismatch for 32-bit builds.

I have also updated the requested Oculus Runtime to 1.15.0, which I have tested this fix on.

This is a low-risk patch, which I recommend be uplifted to beta.

Comment 6

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8878719 [details]
Bug 1352446 - Fix 32-bit build crash with Oculus VR

https://reviewboard.mozilla.org/r/150030/#review154754

LGTM! Good Job!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9867d77814f0
Fix 32-bit build crash with Oculus VR r=daoshengmu

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/9867d77814f0

Comment 9

[:kip (Kearwood Gilbert)]

Attachment: Bug 1352446 - Fix 32-bit build crash with Oculus VR - Beta Uplift

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1260530 - Add support for Oculus 1.3 runtime.
[User impact if declined]:
32-bit Windows Firefox will crash if Oculus hardware is attached and the user attempts to enter VR on a WebVR site.
[Is this code covered by automated tests?]:
Fault can only occur with physical hardware attached, can not automate test currently.
[Has the fix been verified in Nightly?]:
Yes
[Needs manual test from QE? If yes, steps to reproduce]: 
Test plan for Softvision will cover this use case.  We must be certain to include 32-bit builds in the test plan.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
Very low risk.
[Why is the change risky/not risky?]:
Change was minimal and only affects Oculus WebVR users.
[String changes made/needed]:
No string changes

Comment 10

[:kip (Kearwood Gilbert)]

This fix causes a regression, resulting in a crash when enumerating Oculus Touch controllers in WebVR - Bug 1375060.

The fix for Bug 1375060 should be uplifted simultaneously.

Comment 11

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8879282 [details] [diff] [review]
Bug 1352446 - Fix 32-bit build crash with Oculus VR - Beta Uplift

Prevents a crash for Oculus users, let's uplift to beta.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/15f369b66378

Comment 13

[Florin Mezei, QA (:FlorinMezei)]

Marking as qe-verify+ to make sure this case is covered when testing WebVR.

Comment 14

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

Since the webVR was disabled on x86 builds, this signature will no longer reproducible. Putting this issue on my list and will monitor after webVR will be re-enabled in x86 build.
Closing this issue.