1375060 - webvr - Nightly 56 crashes with oculus rift when entering VR

Status: VERIFIED FIXED

(Core :: WebVR, defect)

Description

[Rémy Bouquet]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36

Steps to reproduce:

On window 10
- Installed latest Nightly (56.0a1)
- updated oculus software to latest version (1.16.0.407085)
- plugged the rift headset (oculus software reports everything is ok)
- launched any demo site from https://webvr.rocks/firefox


Actual results:

Instant browser crash with no information.


Expected results:

The browser should not crash and the demo should work

Comment 1

[:kip (Kearwood Gilbert)]

I have reproduced this and believe I have found the cause of the crash.
Bug 1352446 fixed 32-bit builds, but introduced a crash when enumerating the touch controllers.  (Affecting both 64-bit and 32-bit builds).

This is due to bumping up the version number of the Oculus SDK.

The Oculus SDK updated a struct with new members, increasing its length, while keeping the same function signature.  This happens within the same dll we are binding, but with a different requested version number.  This struct had no padding appended to it to reserve space for expansion, causing a stack corruption when our version of the struct was not also expanded.

Comment 2

[:kip (Kearwood Gilbert)]

Attachment: Bug 1375060 - Update Oculus headers to match requested API version.

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

Comment 3

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8880178 [details]
Bug 1375060 - Update Oculus headers to match requested API version.

https://reviewboard.mozilla.org/r/151544/#review156658

LGTM. I am curious if we should add all the function pointers from Oculus library, or we only need to add the functions we are using?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8a170f2d331
Update Oculus headers to match requested API version. r=daoshengmu

Comment 5

[:kip (Kearwood Gilbert)]

(In reply to Daosheng Mu[:daoshengmu] from comment #3)
> Comment on attachment 8880178 [details]
> Bug 1375060 - Update Oculus headers to match requested API version.
> 
> https://reviewboard.mozilla.org/r/151544/#review156658
> 
> LGTM. I am curious if we should add all the function pointers from Oculus
> library, or we only need to add the functions we are using?

I added the remaining functions that we aren't using yet when I just updated it, so that if we use them in the future (ie, for stage bounds in Bug 1311830), we will not accidentally use a newer version of a struct or function against the older API version.

Comment 6

[:kip (Kearwood Gilbert)]

Ideally, there won't need to be any custom header for Oculus SDK.  I will query Oculus to see what options are possible.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e8a170f2d331

Comment 8

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

Marking this affected for 55 as we want to uplift this patch along with the issue in bug 1352446.

Comment 9

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

Comment on attachment 8880178 [details]
Bug 1375060 - Update Oculus headers to match requested API version.

Approval request is in bug 1352446. This should help prevent a crash for Oculus users.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d36fe4881abb

Comment 11

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

I verified this issue using Fx 55.0b10 and the latest nightly. The browser no longer crashes while entering VR.

Cheers!