1539845 - nsIObserver::Observe should be MOZ_CAN_RUN_SCRIPT

Status: NEW

(Core :: XPCOM, enhancement)

Comment 1

[Karl Tomlinson (:karlt)]

Spinning the event loop permits calls to nsIObserver::Remove(), for example, which is forbidden during notifications, so Observe() implementations must not spin the event loop. I guess some observers are script, but they are Chrome script, where running arbitrary code can be avoided.

I assume MOZ_CAN_RUN_SCRIPT is intended to mean "caller must hold strong refs" (bug 1380423), which is not necessary in general if arbitrary changes will not be made? I assume a GC during any script execution is not likely to trigger release of C++ objects except for those with lifetimes explicitly tied to JS objects. My understanding is that CC'ed objects are not freed during GC.

Comment 2

[Andrew McCreight [:mccr8]]

CC'ed objects are not freed during GC, but any time we run script CC'ed objects can be freed, because it is possible to have a nested event loop (for instance, with sync XHR), so we can end up running the CC any time we run JS.

Comment 3

[Boris Zbarsky [:bzbarsky]]

so Observe() implementations must not spin the event loop

OK, but that doesn't mean they don't.

where running arbitrary code can be avoided.

Past experience shows that it can be avoided, if one tries pretty hard (e.g. no interaction with web pages), but often isn't avoided... and JS doesn't have very good visibility into what changes it's making even if you stick to chrome JS.

I assume MOZ_CAN_RUN_SCRIPT is intended to mean "caller must hold strong refs"

That is correct.

which is not necessary in general if arbitrary changes will not be made

That is true, but in general it's hard to predict what changes random callees will or will not make if you have no idea what those callees are.

As a specific example, notifying "memory-pressure" via the observer service will cause all sorts of caches to be cleared, dropping references to things in the process.

It's possible we can scope things down somewhat if it's really needed (i.e. if we have cases where the caller can't hold strong refs), though it seems like in those cases it can use MOZ_KnownLive, if it thinks that holds. And then we can audit appropriately to see whether it holds.