586740 - Microformats test leak fix

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Benjamin Stover (:stechz)]

A microformat mochitest adds a microformat to a long-lived module's scope, but does not clean it up.  This fixes a leak in one of my patches that for some reason does not show up normally.

Comment 1

[Benjamin Stover (:stechz)]

Attachment: Patch

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Erm, I have never hacked on this code (perhaps you thought of me because I worked on microsummaries?).  Based on the log, Michael Kaply or Robert Sayre would be better reviewers.

Comment 3

[Mike Kaply [:mkaply]]

This looks ok to me, but shouldn't there be another remove for the other hTest add?

Comment 4

[Robert Sayre]

what is the patch that this helps?

Comment 5

[Benjamin Stover (:stechz)]

(In reply to comment #3)
> This looks ok to me, but shouldn't there be another remove for the other hTest
> add?

No, the second hTest replaces the first hTest in the module, so only one removal is necessary.

> what is the patch that this helps?

Here's a couple actually:
https://bugzilla.mozilla.org/show_bug.cgi?id=552827
https://bugzilla.mozilla.org/show_bug.cgi?id=550936

They leak in mochitest chunk 5.  Basically, we are noticing this when messageManager is being used with an addEventListener call in the frame script.

Comment 6

[Mike Kaply [:mkaply]]

And you're sure that this patch fixes those leaks?

Comment 7

[Benjamin Stover (:stechz)]

Yes.  Alon Zakai and I have verified it fixes the leak on mochitest 5 on our own respective bugs.

Comment 8

[Doug Turner (:dougt)]

Pushed to e10s.  will merge into m-c shortly:

http://hg.mozilla.org/projects/electrolysis/rev/0346acf59bc6

Comment 9

[Dão Gottwald [:dao]]

bugspam