Closed Bug 1218482 Opened 8 years ago Closed 8 years ago

[webvr] Enable WebVR By Default on all platforms

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla46
Tracking Flags:
Tracking Status
firefox46 --- disabled
relnote-firefox --- 55+

People

(Reporter: kip, Assigned: kip)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, devrel-needed, Whiteboard: [webvr])

Attachments

(1 file, 2 obsolete files)

Bug 1218482 - Enable WebVR By Default,r=bz
5.95 KB, patch
Details | Diff | Splinter Review 
WebVR has been developed behind the dom.vr.enabled preference, but has not yet been enabled by default due to incompatibility with e10s.

Bug 1182048 enables compatibility with e10s.

Once Bug 1182048 lands, we wish to enable WebVR by default.
See Also: → 1215147
The patch changes two preferences:

dom.vr.enabled is now "true" by default.

dom.vr.add-test-devices is now "0" by default, so that content may detect VR devices accurately.
Bug 1218482 - Enable WebVR By Default
- dom.vr.enabled is now "true" by default.
- dom.vr.add-test-devices is now "0" by default, so that content may detect
  VR devices accurately.
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Bug 1218482 - Enable WebVR By Default
- dom.vr.enabled is now "true" by default.
- dom.vr.add-test-devices is now "0" by default, so that content may detect
  VR devices accurately.
Attachment #8679023 - Flags: review?(vladimir)
Release Note Request (optional, but appreciated)
[Why is this notable]: WebVR enabled by default
[Suggested wording]: Use virtual reality devices with the web with the introduction of WebVR
[Links (documentation, blog post, etc)]:
http://mozvr.com/
http://webvr.info/
relnote-firefox: --- → ?
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Bug 1218482 - Enable WebVR By Default
- dom.vr.enabled is now "true" by default.
- dom.vr.add-test-devices is now "0" by default, so that content may detect
  VR devices accurately.
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Bug 1218482 - Enable WebVR By Default
- dom.vr.enabled is now "true" by default.
- dom.vr.add-test-devices is now "0" by default, so that content may detect
  VR devices accurately.
Attachment #8679023 - Flags: review?(bzbarsky)
As this functionality is not intended to land in Firefox 44, we will delay committing until after the Firefox 44 branch date and let it ride the trains to Firefox 45.
I have added bzbarsky to the reviewers for the patch, as it now includes an update to test_interfaces.html to confirm that there has been DOM peer review.
Whiteboard: [webvr]
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

https://reviewboard.mozilla.org/r/23343/#review21111

I would really like to see answers to Ehsan's questions in the intent to ship thread before we ship this.
Attachment #8679023 - Flags: review?(bzbarsky)
Hmm, does "all platforms" include Android and b2g?  It seems like those platforms don't need this enabled, so you should probably make the pref change in firefox.js.
(In reply to Ehsan Akhgari (don't ask for review please) [Away Nov 3-19] from comment #13)
> Hmm, does "all platforms" include Android and b2g?  It seems like those
> platforms don't need this enabled, so you should probably make the pref
> change in firefox.js.
This does indeed include Android and B2G.  Android can utilize VR through the Google "Cardboard VR" holders.  I have not yet tested this with B2G; however, it should theoretically work as well.  (I'll give it a try and report back)
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23343/diff/2-3/
Attachment #8679023 - Flags: review?(bugs)
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23343/diff/2-3/
Attachment #8679023 - Flags: review?(bugs) → review?(bzbarsky)
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23343/diff/2-3/
I have updated the (In reply to :Ehsan Akhgari from comment #13)
> Hmm, does "all platforms" include Android and b2g?  It seems like those
> platforms don't need this enabled, so you should probably make the pref
> change in firefox.js.

I have updated the patches to enable the dom.vr.enable preference and the DOM interfaces in test_interfaces.html to enable WebVR by default for non-release builds only.

Does this look good to you?  Are there any other changes required?
Flags: needinfo?(ehsan)
Yes, this looks good!  But your test_interfaces.html changes look wrong, you shouldn't need to set |android: true|.
Flags: needinfo?(ehsan)
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

https://reviewboard.mozilla.org/r/23343/#review23461

r=me
Attachment #8679023 - Flags: review?(bzbarsky) → review+
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23343/diff/3-4/
Attachment #8679023 - Flags: review?(vladimir)
Blocks: 1229464
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/46da19b55672 for "Assertion failure: mCompositorThreadHolder, at /builds/slave/b2g_m-in_emu-d_dep-00000000000/build/gecko/gfx/vr/ipc/VRManagerParent.cpp:109" in a wide variety of b2g emulator mochitests.
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23343/diff/3-4/
Attachment #8679023 - Attachment description: MozReview Request: Bug 1218482 - Enable WebVR By Default → MozReview Request: Bug 1218482 - Remove unnecessary assertion
Attachment #8679023 - Flags: review+
Running B2G Emulator Mochitests on Try to ensure that this is the extent of the issues.

        Attachment #8679023 -
        Attachment is obsolete: true

    
    

    
  


  
Removed an unnecessary assertion that was erroneously triggering
on shutdown in B2G Mochitests.

    
  
Depends on: 1235803

    
    

    
  
Comment on attachment 8703008 [details] [diff] [review]
Bug 1218482 - Remove unnecessary assertion

This patch has been moved to Bug 1235803

        Attachment #8703008 -
        Attachment is obsolete: true

    
    

    
  
As Bug 1235803 and Bug 1235740 have landed, now re-landing this one.

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/591f1064b73c
Status: NEW → RESOLVED
Closed: 8 years ago

          status-firefox46:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46

    
    

    
  
Confirming that WebVR is now enabled by default across platforms (Win 10 x64, Ubuntu 14.04 x86 and Mac OS X 10.9.5), using latest 46 Nightly, build ID: 20160105030211.
Status: RESOLVED → VERIFIED

          status-firefox46:
          fixed → verified

    
    

    
  
Kip, is there a blog post or something I can link to for the aurora release notes?
Flags: needinfo?(kgilbert)
Depends on: 1239055

    
    

    
  
Browser compat data now updated on all the WebVR API ref pages:

https://developer.mozilla.org/en-US/docs/Web/API/WebVR_API

Also, I want to confirm that this is noted on the developer release notes:

https://developer.mozilla.org/en-US/Firefox/Releases/46#New_APIs

On a separate note, I've noticed that the MDN ref docs are now out-of-date with the spec; this will be updated once we have spare cycles to do so, probably in Q2.
Keywords: dev-doc-needed → dev-doc-complete

    
    

    
  
I have a question here: is the implementation out-of-date with the MDN ref docs? We have not seen requests for changes in our weekly doc request triage meetings.

    
    

    
  
(In reply to Jean-Yves Perrier [:teoli] from comment #56)
> I have a question here: is the implementation out-of-date with the MDN ref
> docs? We have not seen requests for changes in our weekly doc request triage
> meetings.

This is a good point. I am assuming we'd be up to date with the latest spec, which is not necessarily going to be the case ;-)

    
    

    
  
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #57)
> (In reply to Jean-Yves Perrier [:teoli] from comment #56)
> > I have a question here: is the implementation out-of-date with the MDN ref
> > docs? We have not seen requests for changes in our weekly doc request triage
> > meetings.
> 
> This is a good point. I am assuming we'd be up to date with the latest spec,
> which is not necessarily going to be the case ;-)

A brief scan through the current MDN docs appears to match the implementation.

It should be noted; however, that the current API that is implemented will be deprecated soon in favor of the compatibility-breaking "WebVR 1.0" api:

https://mozvr.github.io/webvr-spec/

The dust is still settling on "WebVR 1.0" as it has just been released publicly for feedback.  Our implementation of this API is tracked in Bug 1250244.  This is expected to land within one quarter.  Please NI me if you would like more details.
Flags: needinfo?(kgilbert)

    
    

    
  
(In reply to :kip (Kearwood Gilbert) from comment #58)
> (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #57)
> > (In reply to Jean-Yves Perrier [:teoli] from comment #56)
> > > I have a question here: is the implementation out-of-date with the MDN ref
> > > docs? We have not seen requests for changes in our weekly doc request triage
> > > meetings.
> > 
> > This is a good point. I am assuming we'd be up to date with the latest spec,
> > which is not necessarily going to be the case ;-)
> 
> A brief scan through the current MDN docs appears to match the
> implementation.
> 
> It should be noted; however, that the current API that is implemented will
> be deprecated soon in favor of the compatibility-breaking "WebVR 1.0" api:
> 
> https://mozvr.github.io/webvr-spec/
> 
> The dust is still settling on "WebVR 1.0" as it has just been released
> publicly for feedback.  Our implementation of this API is tracked in Bug
> 1250244.  This is expected to land within one quarter.  Please NI me if you
> would like more details.

Thanks for the info kip — I think we should be ok at this point; I've put WebVR docs update into our publishing pipeline for Q2, and we've got the dev-doc-needed keyword set on the implementation bug, so it will come up in our triage list when the work is complete. We might want to still touch base nearer the time to discuss a schedule for getting the doc updates published.

    
    

    
  
We should update the release notes as we disabled it because of bug 1239055

          relnote-firefox:
          46+ → ?

    
    

    
  
dev-doc-needed so that we undo what has been done in comment 55 :-(
Keywords: dev-doc-complete → dev-doc-needed

    
    

    
  
Compat information reverted.
Keywords: dev-doc-needed → dev-doc-complete

    
    

    
  
Kip: which bug should the writers follow to be notified when WebVR will be re-enabled by default?
Flags: needinfo?(kgilbert)

    
    

    
  
(In reply to Jean-Yves Perrier [:teoli] from comment #63)
> Kip: which bug should the writers follow to be notified when WebVR will be
> re-enabled by default?

FYI - WebVR is still enabled by default in non-release builds.  Once stability is improved, we still still want to wait for changes in the WebVR API to stabilize with WebVR 1.0.

I have added Bug 1256444 to track the dependencies and enabling WebVR in release.
Flags: needinfo?(kgilbert)

    
    

    
  
Thanks a lot, :kip

    
    

    
  
Marking this as disabled for 46 since it will not be on by default on release next week.

          status-firefox46:
          verified → disabled

    
  
Depends on: 1271157

    
    

    
  
adding to the 55beta release notes

          relnote-firefox:
          ? → 55+

          You need to log in
          before you can comment on or make changes to this bug.