Closed
Bug 1386308
Opened 7 years ago
Closed 7 years ago
Stop trying to alter display management (sleep) code in content processes
Categories
(Core :: Security: Process Sandboxing, enhancement, P1)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: Alex_Gaynor, Assigned: Alex_Gaynor)
References
Details
(Whiteboard: sbmc3)
Attachments
(1 file)
Currently when we want to affect power management (e.g. disable display sleeping because you're watching a video), this is done in both the content and parent process, as far as I can tell.
This happens here: https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsAppShell.mm#74-78
We should stop trying to do this in the content process, as it triggers the "com.apple.PowerManagement.control" mach-service permission. If we remove the permission, everything will keep working, since the parent process duplicates the work, but it's silly and a waste of CPU cycles :-)
We should stop using MacWakeLockListener in the content process, and then remove the "com.apple.PowerManagement.control" mach permission.
Assignee | ||
Comment 1•7 years ago
|
||
https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsAppShell.mm#667-682 is where we set it up -- but I'm still learning the code to know if it's more appropriate to stop using nsAppShell, or to just wrap AddScreenWakeLockListener in `if (XRE_IsParentProcess())`.
Assignee | ||
Updated•7 years ago
|
Comment 2•7 years ago
|
||
This sounds like the Mac equivalent of bug 1360069.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → agaynor
Assignee | ||
Updated•7 years ago
|
Priority: -- → P1
Whiteboard: sbmc3
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8893387 [details]
Bug 1386308 - stop trying to change the display sleep settings from the content process;
https://reviewboard.mozilla.org/r/164506/#review169958
Attachment #8893387 -
Flags: review?(haftandilian) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a73050e842c2
stop trying to change the display sleep settings from the content process; r=haik
Keywords: checkin-needed
Comment 7•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 8•7 years ago
|
||
Dumb question, but are we forwarding wake lock requests from the content process to the parent process? Since requests to disable the screen saver while playing video come from the content process.
Assignee | ||
Comment 9•7 years ago
|
||
I didn't look at the exact mechanism it was happening by, but I confirmed that all the requests to disable sleep were happening in both content and parent, so there shouldn't be a behavior change here.
You need to log in
before you can comment on or make changes to this bug.
Description
•