Closed Bug 1352446 Opened 5 years ago Closed 5 years ago
Crash in libovrrt32
59 bytes, text/x-review-board-request
4.93 KB, patch
|Details | Diff | Splinter Review|
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
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.
Assignee: nobody → kgilbert
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.
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 on attachment 8878719 [details] Bug 1352446 - Fix 32-bit build crash with Oculus VR https://reviewboard.mozilla.org/r/150030/#review154754 LGTM! Good Job!
Attachment #8878719 - Flags: review?(dmu) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/9867d77814f0 Fix 32-bit build crash with Oculus VR r=daoshengmu
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
Attachment #8879282 - Flags: approval-mozilla-beta?
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 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.
Attachment #8879282 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Marking as qe-verify+ to make sure this case is covered when testing WebVR.
QA Contact: cornel.ionce
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.
You need to log in before you can comment on or make changes to this bug.