Closed
Bug 1384459
Opened 7 years ago
Closed 7 years ago
Only enable WebVR on 64-bit builds
Categories
(Core :: WebVR, enhancement)
Tracking
()
VERIFIED
FIXED
mozilla56
People
(Reporter: kip, Assigned: kip)
Details
Attachments
(1 file, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
jgilbert
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
We wish to enable WebVR only for 64-bit builds before it rides the trains with 55 in release. This will serve a few purposes: - Reduction of test requirements by reduction of configuration matrices. - Ensuring that the optimal 64-bit builds are used for WebVR sites, which are often likely to hit 32-bit address space fragmentation limitations resulting in OOMs. - Act as a rudimentary soft-launch in 55. 56 is expected to bring a larger set of users to 64-bit builds. We believe that desktop PC's with supported VR hardware are likely to be running a 64-bit capable OS, so this impact should be minimal for users that can benefit from our Desktop WebVR implementation. We will keep the 32-bit specific Oculus and OpenVR code paths for a while in case we hear any feedback contrary to these assumptions. This will be implemented by changing the default value of the dom.vr.enabled pref to "false" for 32-bit builds.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8890244 -
Flags: review?(dmu)
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8890244 [details] Bug 1384459 - Disable WebVR by default for 32-bit builds https://reviewboard.mozilla.org/r/161368/#review166662 LGTM.
Attachment #8890244 -
Flags: review?(dmu) → review+
Assignee | ||
Comment 3•7 years ago
|
||
The try run unfortunately shows that the test-interfaces.html mochitest fails. This test does not have provisions for expecting interfaces to be enabled for 64-bit but not 32-bit. I am preparing a new patch, which takes a simpler approach -- WebVR interfaces are still visible, but the device enumeration will be #ifdef'ed out for 32-bit builds. The puppet device and chrome-only API for detecting WebVR pages will survive for use in later UX and to avoid breaking tests.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8890244 -
Attachment is obsolete: true
Assignee | ||
Comment 5•7 years ago
|
||
The updated patch applies this is the lowest risk fashion -- we simply disable dom.vr.openvr.enabled and dom.vr.oculus.enabled for 32-bit builds by default. dom.vr.enabled is left enabled in both builds to satisfy the tests and give consistency with the exposed DOM interfaces.
Assignee | ||
Updated•7 years ago
|
Attachment #8890483 -
Flags: review?(jgilbert)
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8890483 [details] Bug 1384459 - Only enable WebVR device enumeration for 64-bit builds https://reviewboard.mozilla.org/r/161624/#review166942
Attachment #8890483 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 7•7 years ago
|
||
Once the try push completes, I will land this in central and request beta uplift.
Pushed by kgilbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/813927e0ce55 Only enable WebVR device enumeration for 64-bit builds r=jgilbert
Assignee | ||
Comment 9•7 years ago
|
||
Comment on attachment 8890483 [details] Bug 1384459 - Only enable WebVR device enumeration for 64-bit builds Approval Request Comment We wish to enable WebVR only for 64-bit builds before it rides the trains with 55 in release. This will serve a few purposes: - Reduction of test requirements by reduction of configuration matrices. - Ensuring that the optimal 64-bit builds are used for WebVR sites, which are often likely to hit 32-bit address space fragmentation limitations resulting in OOMs. - Act as a rudimentary soft-launch in 55. 56 is expected to bring a larger set of users to 64-bit builds. We believe that desktop PC's with supported VR hardware are likely to be running a 64-bit capable OS, so this impact should be minimal for users that can benefit from our Desktop WebVR implementation. [Feature/Bug causing the regression]: N/A - New Feature [User impact if declined]: WebVR will be enabled by default for both 32-bit and 64-bit builds. [Is this code covered by automated tests?]: [Has the fix been verified in Nightly?]: [Needs manual test from QE? If yes, steps to reproduce]: Yes: - Access a webvr page with a 32-bit and 64-bit version of Firefox with a support VR headset attached. - Access a webvr site such as https://webvr.info/samples/03-vr-presentation.html - The "Enter VR" button on the page should present VR content to the headset for the 64-bit version - The "Enter VR" button will not be visible for the 32-bit version and you may see a message such as "WebVR supported, but no VR displays found" [List of other uplifts needed for the feature/fix]: N/A [Is the change risky?]: Not risky. Reduces risk by implementing a rudimentary "soft launch" of WebVR in 55 by only launching on 64-bit Firefox. [Why is the change risky/not risky?]: This is a very simple #ifdef around prefs to change their default value. [String changes made/needed]: N/A
Attachment #8890483 -
Flags: approval-mozilla-beta?
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/813927e0ce55
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 11•7 years ago
|
||
Comment on attachment 8890483 [details] Bug 1384459 - Only enable WebVR device enumeration for 64-bit builds disable webvr on 32bit builds, beta55+, for 55.0b13
Attachment #8890483 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 12•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/79c0cef8a343
status-firefox55:
--- → fixed
Comment 13•7 years ago
|
||
I verified this issue using Fx 55.0b13(x32) and the Fx 56.0a1 (build ID: 20170728100358, x32). The webVR is disabled on both builds. Cheers!
Comment 14•7 years ago
|
||
https://hg.mozilla.org/projects/jamun/rev/79c0cef8a343d80be70242b31284fd3316d4fec1 Bug 1384459 - Only enable WebVR device enumeration for 64-bit builds. r=jgilbert, a=jcristau
You need to log in
before you can comment on or make changes to this bug.
Description
•