Closed Bug 722479 Opened 12 years ago Closed 12 years ago

browser/components/thumbnails/test/ tests leak chrome://global/content/mozilla.xhtml

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 13
Tracking Status
firefox12 --- fixed

People

(Reporter: dao, Assigned: ttaubert)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

      No description provided.
Attached patch patch v1 (obsolete) — Splinter Review
Trivial patch.
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #593766 - Flags: review?(dao)
Attachment #593766 - Flags: review?(dao) → review+
https://hg.mozilla.org/integration/fx-team/rev/2b7cea9aa6a3
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 13
Backed out:

https://hg.mozilla.org/integration/fx-team/rev/403652510ddd

This fix only worked on OS X, not on any other OS (oranges instead).
Whiteboard: [fixed-in-fx-team]
Attached patch patch v2 (obsolete) — Splinter Review
Sent this patch to try and it works now on all platforms.
Attachment #593766 - Attachment is obsolete: true
Attachment #594128 - Flags: review?(dao)
Comment on attachment 594128 [details] [diff] [review]
patch v2

>+  cachedXULDocument = cachedXULFrame = null;

This shouldn't be needed.

What guarantees that getXULDocument won't be called multiple times, creating multiple iframes and removing only the last one?
Attached patch patch v3Splinter Review
(In reply to Dão Gottwald [:dao] from comment #5)
> What guarantees that getXULDocument won't be called multiple times, creating
> multiple iframes and removing only the last one?

Something much easier that works for multiple iframes.
Attachment #594128 - Attachment is obsolete: true
Attachment #594128 - Flags: review?(dao)
Attachment #594302 - Flags: review?(dao)
Comment on attachment 594302 [details] [diff] [review]
patch v3

registerCleanupFunction(function () { doc.body.removeChild(iframe); });

registerCleanupFunction(function () doc.body.removeChild(iframe)); translates to:
registerCleanupFunction(function () { return doc.body.removeChild(iframe); });
Attachment #594302 - Flags: review?(dao) → review+
(In reply to Dão Gottwald [:dao] from comment #7)
> registerCleanupFunction(function () doc.body.removeChild(iframe));
> translates to:
> registerCleanupFunction(function () { return doc.body.removeChild(iframe);
> });

I know, didn't think it's a big deal but actually you're right. We better be explicit about that.

https://hg.mozilla.org/integration/fx-team/rev/a7ea6d49bc69
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/a7ea6d49bc69
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Comment on attachment 594302 [details] [diff] [review]
patch v3

[Approval Request Comment]
Risk to taking this patch (and alternatives if risky): Very low risk.
String changes made by this patch: None.

Prevents the thumbnail tests from leaking.
Attachment #594302 - Flags: approval-mozilla-aurora?
Comment on attachment 594302 [details] [diff] [review]
patch v3

[Triage Comment]
Test fix for Aurora 12 - approved.
Attachment #594302 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.