VR process is not shutdown after exiting WebVR pages
Categories
(Core :: WebVR, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox77 | --- | wontfix |
firefox78 | --- | fixed |
firefox79 | --- | fixed |
People
(Reporter: daoshengmu, Assigned: daoshengmu)
References
(Regression)
Details
(Keywords: regression)
Attachments
(2 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
STR:
- Open a tab and navigate to https://webvr.info/samples/04-simple-mirroring.html
- Close this tab. Open another tab, go to https://www.wikipedia.org/.
- Check if vr process is still alive after 5 seconds in Process Explorer.
However, if navigate to a WebXR page like https://immersive-web.github.io/webxr-samples/immersive-vr-session.html, and follow the step 2 - 3. We will see VR process is shutdown properly.
VR process takes 12% CPU on my laptop, so I think we should make sure its lifetime be managed correctly.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
Assignee | ||
Comment 2•4 years ago
|
||
Depends on D78436
Assignee | ||
Comment 3•4 years ago
|
||
It causes by [1]. Navigator::OnXRPermissionRequestAllow will be called a couple of times when entering a WebXR/WebVR page, then if the permission is granted, it will ask VRManager::EnumerateDevices
. Once it starts to enumeration, mEnumerationRequested
[2] will be false.
However, Due to [1], !usingWebXR
makes VRManager continuously ask for enumeration, and mEnumerationRequested
will be true then. Therefore, when we ask for shutdown VR process, it will be returned before Shutdown() [3].
It seems to be a regression and affect Gecko from FF 77, I will use moz-regression to check and give a uplift.
[1] https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/dom/base/Navigator.cpp#1570
[2] https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/gfx/vr/VRManager.cpp#695
[3] https://searchfox.org/mozilla-central/rev/7cadba1d8b8feaec4615f5bb98aac4b8a719793c/gfx/vr/VRManager.cpp#488
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
I have confirmed FF 77 - 79 are affected, we need to uplift it then.
Comment 6•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0cea8390afe9
https://hg.mozilla.org/mozilla-central/rev/5f7fbd09153c
Updated•4 years ago
|
Assignee | ||
Comment 7•4 years ago
|
||
Comment on attachment 9154435 [details]
Bug 1643514 - Avoid requesting VR display when it already completed.
Beta/Release Uplift Approval Request
- User impact if declined: VR process will continue to take very high percent CPU usage even though there is no WebVR pages exist in Firefox. Obviously, that VR process should be shutdown.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): It only adds a flag in order to avoid our logic to block us shutdown VR process. We did that for WebXR already, we just need to follow the same way for WebVR.
- String changes made/needed:
Updated•4 years ago
|
Comment 8•4 years ago
|
||
Do we need both patches on beta or just the second one?
Assignee | ||
Comment 9•4 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #8)
Do we need both patches on beta or just the second one?
I think the second one is enough. You can do both if it is more convenient for you because the first one just has some code refinement.
Comment 10•4 years ago
|
||
Comment on attachment 9154435 [details]
Bug 1643514 - Avoid requesting VR display when it already completed.
thanks for the clarification; approved for 78.0b7.
Comment 11•4 years ago
|
||
bugherder uplift |
Description
•