1860680 - Content process idle observer unused?

Status: NEW

(Core :: DOM: Content Processes, task)

Description

[Andrew McCreight [:mccr8]]

ContentParent has two methods, RecvAddIdleObserver and RecvRemoveIdleObserver, that allow the content process to add itself to mIdleListeners and receive idle notifications from the parent process nsIUserIdleService. According to coverage data, this code is not actually tested. I suppose that makes sense because you'd want this to do cleanup tasks while the user was not using the browser, but it doesn't seem too likely that you'd want that in a content process. Maybe in some privileged process? However, if code isn't running in the main process, maybe you don't care as much about doing a bit of work, because it isn't going to overwhelm the main event loop for the UI.

This dates to the B2G era (bug 938889). Maybe there were more system services in that time.

Can we delete this code? It looks safe enough to me, but less code accessible from the child process is better.

I was wondering if this would let a compromised content process "monitor how long the user has been 'idle', i.e. not used their mouse or keyboard" (from nsIUserIdleService.idl) where it couldn't otherwise, which I guess could sort of be useful for an attacker, but maybe it isn't that hard to tell through some other means.

Comment 1

[:Gijs (he/him)]

My involvement with the user idle service is ancient (predates me being employed!) but I don't see any reason why you'd want to use this from the child - you would pretty much always want to use the parent process to manage this stuff. I too vote for killing it with fire. :-)

Comment 2

[Jens Stutte [:jstutte]]

IIUC there is a use of SendAddIdleObserver and we can only come from here. We can probably check what happens if we add an assertion and/or return an error there instead ?

Comment 3

[Andrew McCreight [:mccr8]]

If there's no coverage, an assertion probably won't tell us much. I guess there could be Android only uses or something. For these various apparently unused recv methods, I have wondered about adding a diagnostic assert and seeing what happens.

If it is really useless in a child process, we should make the user idle service unavailable in the child process along with removing this IPC code.