1360069 - Don't send redundant DBus wakelock calls from child processes

Status: RESOLVED FIXED

(Core :: Widget: Gtk, enhancement, P2)

Description

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Assuming I understand what's going on here: nsAppShell::Init installs a WakeLockListener, currently in each process, that uses DBus to message the desktop environment to suppress the screensaver (if DBus support is configured).  But the parent process broadcasts the notifications to each content process (HalParent, subclass of WakeLockObserver), so we get n+1 DBus method calls.

This is more of a problem for sandboxing, because this needs to open (and then leaves open) a DBus connection that could be used for privilege escalation — I still don't understand DBus very well, but I suspect there's *something* on the session bus that could be leveraged into running arbitrary commands.

But, if this is always initiated in the parent process (and I think it is?), then it would suffice to just not install that WakeLockListener in child processes.  I already have a patch for this, but the question is whether it breaks things that don't have automated tests.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Attachment: Bug 1360069 - Deal with the case where WakeLockListener is never instantiated.

Otherwise, with the next patch, xpcshell tests crash because
WakeLockListener::Shutdown tries to Release() nullptr.

Review commit: https://reviewboard.mozilla.org/r/152802/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152802/

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Attachment: Bug 1360069 - Don't do unnecessary wakelock signaling from content processes.

Review commit: https://reviewboard.mozilla.org/r/152804/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152804/

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

To try to test this, I took an Ubuntu 16.04 VM, with the default desktop environment, set the screen lock timeout to 1 minute, and played a full-screen YouTube video for longer than that.

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8881645 [details]
Bug 1360069 - Deal with the case where WakeLockListener is never instantiated.

https://reviewboard.mozilla.org/r/152802/#review157966

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8881646 [details]
Bug 1360069 - Don't do unnecessary wakelock signaling from content processes.

https://reviewboard.mozilla.org/r/152804/#review157970

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3103713ebe59
Deal with the case where WakeLockListener is never instantiated. r=gcp
https://hg.mozilla.org/integration/autoland/rev/775275d2d0c5
Don't do unnecessary wakelock signaling from content processes. r=gcp

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3103713ebe59
https://hg.mozilla.org/mozilla-central/rev/775275d2d0c5