Closed Bug 1362578 Opened 3 years ago Closed 8 months ago

[meta] Move VRService to its own process

Categories

(Core :: WebVR, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: kip, Assigned: kip)

References

(Blocks 5 open bugs)

Details

(Keywords: meta)

The Oculus and OpenVR runtime libraries require specific sandboxing rules.  We are working with external parties to determine what would needed to be whitelisted to allow access to VR hardware in a sandboxed process.  In order to allow the GPU process to have tighter sandboxing rules, we will move the VR hardware interfacing code (VRManager) to its own process.

VRManager communicates with the PVRManager protocol, which is a top-level protocol.  This protocol will remain as the mechanism to communicate with the other Firefox processes.

Additionally, a shared memory structure will be implemented to enable efficient broadcasting of sensor state to multiple processes:

Bug 1346927 - Replace PVRManager::GetSensorState with lock-free shared memory structure (Eliminating sync IPC)

To reduce overhead, the VR process should be launched only after the user accesses a WebVR site.  If no VR hardware runtimes are detected, the process should terminate and set a flag to prevent unnecessary repeated attempts.  If VR hardware runtimes are detected but VR hardware is not turned on / connected, the VR process should remain running and respond to VR hardware hotswap events.
The initial release with WebVR enabled by default will ride the trains in Firefox 55.

This work to shift VR functionality to its own process should commence with Firefox 56.
Do you have an updated timeline for this? I assume it's not going into Firefox 56 anymore since that's already on beta and this doesn't seem to have patches yet.
Flags: needinfo?(kgilbert)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
> Do you have an updated timeline for this? I assume it's not going into
> Firefox 56 anymore since that's already on beta and this doesn't seem to
> have patches yet.

This will likely be broken into two pieces...
In FF57, we aim to get WebVR/PVRManager off the compositor thread, but will still be in the GPU process.
In FF58, we will be launching a new process on-demand when you hit a WebVR site that tries to access the VR hardware.

Essentially, the perf advantages will come with FF57, while we will be unblocking the GPU sandboxing in FF58.

Does this align with the plans you have in mind?
Flags: needinfo?(kgilbert)
That sounds reasonable, thanks!
We have filed a meta bug, Bug 1392215 for tracking the progress of VR thread.
See Also: → 1392215
Depends on: 1400457
Depends on: 1392215
Depends on: 1404534
Hey Kip,

It looks like we have resolved all blockers for VR process work. We can start to work on separating it to an individual process. Because it is too late to take FF58 and FF59 trains, I am interested if you will continue to work on this in FF 60? Thanks!
Flags: needinfo?(kgilbert)
I have converted this bug to a meta bug.  We will hang individual tasks under this one.

@daosheng: Please feel free to start on any work here if you get to it before I do and add any pieces as bugs blocked by this one.

A couple of points to be careful of:

- We should continue to support a single process environment, for some platforms, such as Android.
- We should ensure that the VR process starts only when needed, after accessing a WebVR site.  If no VR devices are found, we should shut down the process after detection and avoid launching it again until a timer expires.  (We may be able to use the existing logic behind dom.vr.display.enumerate.interval for this).
- We should ensure that the VR process stops when VR presentation has ended.  The VR presentation may continue after content has been stopped if the browser Chrome is displaying UI in the headset.
- We will not be allowing positional tracking unless the VR presentation is started, which should help with security and reduce the number of states to consider when starting and stopping the VR process.  (See Bug 1392806)
Depends on: 1392806
Flags: needinfo?(kgilbert)
Summary: Move VRManager to its own process → [meta] Move VRManager to its own process
Hey Kip, checking on on this project. What's your team's current release target? Anything we can do to help?
Flags: needinfo?(kgilbert)
Flags: needinfo?(kgilbert)
(In reply to Jim Mathies [:jimm] from comment #8)
> Hey Kip, checking on on this project. What's your team's current release
> target? Anything we can do to help?

Hi Jim,

@daoshengmu and I will be pushing forward with this once again, with the aim to enable in the F63-64 timeframe.

We could use some feedback on our approach...

- We will want to have a separate set of environment variables for the VR process on MacOS (re-enabling the "nano allocator" for the VR process, while leaving it disabled for the rest)
- We have a simple IPC mechanism built upon a SHMEM (currently behind a pref), which allows an external process to submit sensory input data to Firefox and receive back shared GPU surface handles for the VR output.  This interface is likely all we need for communication with the VR process.  We would only be interested in the latest state, as opposed to transactions in a queue.  See gfx/vr/gfxVRExternal.cpp/h and gfx/vr/external_api/moz_external_vr.h
- Perhaps it may be safer to build a separate .exe to avoid any code running in the process that is untested with the variant of the run-time environment flags set.  We would still need to integrate telemetry and crash reporting; however.  Alternately, we would choose to activate the main() function for the vr process via a command-line switch to firefox.exe.
- We would only spin up the VR process once WebVR content is detected.  As the hardware detection code will execute in the VR process, the VR process would need to be running while visiting a WebVR capable page, even if hardware may not be present.  We would terminate the process once the user hasn't been viewing WebVR content for a particular duration of time.
- It's possible that in the future an app submitted to the Oculus store and/or SteamVR may contain a VR browser front-end which communicates with the user's existing Firefox processes through this SHMEM interface.  In this case, the app's process would pre-empt the need for the browser to spawn it's own VR process.
- The GPU process, which currently runs the VR interface code, would solely connect to this GPU process, polling the SHMEM for sensor changes and populate a ring buffer of data to return to the headset.
- We would no longer need to load 3rd party runtime dll's into any process other than the VR process, which should simplify the sandboxing efforts for the GPU process.

The first decision that I think we will need to settle on, is if we should add a command-line argument to firefox.exe to act as the VR process, or if we should generate a separate .exe for the VR process.  Would you have any preference either way, or know who would be most opinionated on the matter?
Flags: needinfo?(jmathies)
The WIP patch for Bug 1466699 includes much of the functionality needed for the VR process.  In particular, gfx/vr/service/VRService.cpp.

I expect to land this next week along with service implementations for OpenVR, Oculus, and OSVR (Jun 18-22)
(In reply to :kip (Kearwood Gilbert) from comment #9)
> (In reply to Jim Mathies [:jimm] from comment #8)
> > Hey Kip, checking on on this project. What's your team's current release
> > target? Anything we can do to help?
> 
> Hi Jim,
> 
> @daoshengmu and I will be pushing forward with this once again, with the aim
> to enable in the F63-64 timeframe.
> 
> We could use some feedback on our approach...
> 
> - We will want to have a separate set of environment variables for the VR
> process on MacOS (re-enabling the "nano allocator" for the VR process, while
> leaving it disabled for the rest)

no opinion here. I'm not familiar with this feature or why we need it.

> - We have a simple IPC mechanism built upon a SHMEM (currently behind a
> pref), which allows an external process to submit sensory input data to
> Firefox and receive back shared GPU surface handles for the VR output.  This
> interface is likely all we need for communication with the VR process.  We
> would only be interested in the latest state, as opposed to transactions in
> a queue.  See gfx/vr/gfxVRExternal.cpp/h and
> gfx/vr/external_api/moz_external_vr.h

On Windows we're communicating from the GPU process which is isolated from untrusted content. Not sure about OSX and Linux though, I think for the Mac the GPU process is the content process and so your VR process would represent a direct attack vector. So I'd be careful about the interface you expose, and run anything you decide on through a sec-audit.
 
> - Perhaps it may be safer to build a separate .exe to avoid any code running
> in the process that is untested with the variant of the run-time environment
> flags set.  We would still need to integrate telemetry and crash reporting;
> however.  Alternately, we would choose to activate the main() function for
> the vr process via a command-line switch to firefox.exe.

I would suggest using firefox.exe since that's what we're doing in other cases. Why add another variant.

> - We would only spin up the VR process once WebVR content is detected.  As
> the hardware detection code will execute in the VR process, the VR process
> would need to be running while visiting a WebVR capable page, even if
> hardware may not be present.  We would terminate the process once the user
> hasn't been viewing WebVR content for a particular duration of time.

seems resonable.

> - It's possible that in the future an app submitted to the Oculus store
> and/or SteamVR may contain a VR browser front-end which communicates with
> the user's existing Firefox processes through this SHMEM interface.  In this
> case, the app's process would pre-empt the need for the browser to spawn
> it's own VR process.

Don't understand this at all, can you elaborate? Which processes would it need to communicate with and why?

> - The GPU process, which currently runs the VR interface code, would solely
> connect to this GPU process, polling the SHMEM for sensor changes and
> populate a ring buffer of data to return to the headset.

s/GPU/VR ?

> - We would no longer need to load 3rd party runtime dll's into any process
> other than the VR process, which should simplify the sandboxing efforts for
> the GPU process.

yep!
Flags: needinfo?(jmathies)
Depends on: 1473399
I have updated the title to reflect that the VRManager will be simplified and act as the client interface to communicate with the VRService thread in the VR process.
Summary: [meta] Move VRManager to its own process → [meta] Move VRService to its own process
Depends on: 1473402
Depends on: 1481327
No longer depends on: 1430046
Blocks: 1430043
No longer depends on: 1430043
No longer depends on: 1392806
Blocks: 1473402
No longer depends on: 1473402
Depends on: 1498625
No longer depends on: 1430038
Depends on: 1484076
Depends on: 1498724
Depends on: 1516554

Removed dependency on gfxVRPuppet refactoring, as we can enable the VR process without it (Tests will still run in the GPU process for now)

No longer depends on: 1466702
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.