We can't rely on the content process to remove all of its idle observers, it may crash. Please test this reuben.

Comment on attachment 8371148 [details] [diff] [review] Patch Review of attachment 8371148 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #0) > Please test this reuben. This patch doesn't break things that use the IdleListener on b2g-desktop, but I'm seeing some weird behavior that is probably b2g-desktop specific. I'll get this on a device and see if things work and if I can reproduce the leaks. ::: dom/ipc/ContentParent.cpp @@ +3256,1 @@ > mIdleListeners.Put(aObserver, listener); Looks like you forgot to change mIdleListeners' type? @@ +3270,1 @@ > bool found = mIdleListeners.Get(aObserver, &listener); Ditto. @@ +3270,3 @@ > bool found = mIdleListeners.Get(aObserver, &listener); > if (found) { > + if (aObserver->Time() != aIdleTimeInS) { Presumably you meant listener->Time() here? @@ +3277,2 @@ > mIdleListeners.Remove(aObserver); > + listener->RemoveSelf(idleService); This is not supposed to change any semantics, right? You just factored the code into RemoveSelf because it's now used in two different places? ::: dom/ipc/ContentParent.h @@ +601,5 @@ > } // namespace dom > } // namespace mozilla > > class ParentIdleListener : public nsIObserver { > + friend class mozilla::dom::ContentParent; Is this actually needed? It doesn't look like you're using it anywhere. @@ +615,4 @@ > {} > virtual ~ParentIdleListener() {} > + > + void RemoveSelf(nsIIdleService* aIdleService); This file needs to include nsIIdleService.h.

Too edge-casey to block on at this point.

Comment on attachment 8371148 [details] [diff] [review] Patch Review of attachment 8371148 [details] [diff] [review]: ----------------------------------------------------------------- Is that still something we want to fix?

Yes.

Comment on attachment 8371148 [details] [diff] [review] Patch To be clear, this patch doesn't work. But we want to fix this bug.

I read MDN, but still not sure should I set this a duplicate of bug 1166592 or resolved as worksforme...

Looks like a DUPLICATE. Thanks for fixing it!