637116 - Leak documents+windows with replaceState, focus, event handler closure

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: testcase

This convoluted testcase causes nsGlobalWindow and nsDocument objects to leak.

It has something in common with the testcase in bug 630801: passing a weird throwing object to pushState.  So maybe the bugs are related.

Comment 1

[Peter Van der Beken [:peterv]]

Somehow the exception isn't showing up in the cycle collector.

Comment 2

[Peter Van der Beken [:peterv]]

Attachment: v1

We need to propagate the exception from parsing the JSON in nsDocShell::StringifyJSValVariant to the caller of nsHistory::PushState. Jst, mrbkap, any idea if there's a cleaner way to do this?

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

So, jst and I talked about this very patch when I was looking at bug 623850. The reason we didn't go with it is that it's possible that the top context on the context stack is unrelated to the actual caller. IMO it'd be safer (for now) to clear the pending exception and return NS_ERROR_FAILURE. That way, we'll lose the exception that was thrown, but we will preserve that *an* error was thrown, and we won't leak.

Comment 4

[Peter Van der Beken [:peterv]]

Attachment: v2

Comment 5

[Peter Van der Beken [:peterv]]

Attachment: v2

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 523436 [details] [diff] [review]
v2

r=me after changing the IIDs for the interfaces that you added [implicit_context] markings to.

Comment 7

[Peter Van der Beken [:peterv]]

http://hg.mozilla.org/mozilla-central/rev/a0adf7f6b4bd