Closed Bug 775779 Opened 12 years ago Closed 12 years ago

social sidebar tests are marked as "leaking" the sidebar document

Categories

(Firefox Graveyard :: SocialAPI, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 17

People

(Reporter: Gavin, Assigned: markh)

References

Details

Attachments

(1 file, 3 obsolete files)

See bug 775380. This applies to browser/base/content/test/browser_social_sidebar.js (and browser/base/content/test/browser_social_mozSocial_API.js which I'll be adding in bug 773351). This seems similar to bug 759711 and/or bug 728426. The tests trigger the unhiding of the sidebar, which loads a URL in its browser. When the sidebar is hidden again, the sidebar browser is hidden and set to load about:blank. For the moment I've just disabled the tests in debug builds. Hopefully bug 728294 will help with this? I'm not sure.
Now that bug 728426 has been fixed I think this leak could be gone. You still might be blamed for leaking the sidebar's outer window as long as bug 728294 hasn't landed but that should still be within the leak threshold.
(In reply to Tim Taubert [:ttaubert] from comment #1) > Now that bug 728426 has been fixed I think this leak could be gone. Nope. For some to-me-unknown reason, the social UI doesn't use the standard sidebar infrastructure but has its own sidebar on the right side of the browser.
Yes, we'll need to port bug 728426's fix to the social sidebar code.
Attached patch prevent "leaking" of the sidebar document. (obsolete) (deleted) — Splinter Review
This patch is on top of the patch in bug 773351 - it creates a new AboutBlankContentViewer before resetting to about:blank and re-enables the test on debug builds. I'll request review once 773351 lands.
Attached patch updated against trunk (obsolete) (deleted) — Splinter Review
This version also removes the "skip if debug" check from browser_social_sidebar.js
Attachment #645144 - Attachment is obsolete: true
Attachment #647076 - Flags: review?(gavin.sharp)
Attachment #647076 - Attachment is obsolete: true
Attachment #647076 - Flags: review?(gavin.sharp)
Attachment #647138 - Flags: review?(gavin.sharp)
Comment on attachment 647138 [details] [diff] [review] Don't enable test in mozSocial_api due to bug 778639 *sigh* - it's not clear that this patch has any effect on the reported leaks in current builds. However, IMO this should be a priority so we can get all our tests running in debug builds to prevent things like the "compartment mismatch" issue screwing us.
Attachment #647138 - Flags: review?(gavin.sharp) → review-
It appears bug 728294 prevented these "leaks" from being reported. I pushed the attachment to try at https://tbpl.mozilla.org/?tree=Try&rev=fb7acc56fb24 and the builds that have already finished look green (except for one orange that looks unrelated).
Attachment #647138 - Attachment is obsolete: true
Attachment #649989 - Flags: review?(gavin.sharp)
Comment on attachment 649989 [details] [diff] [review] Avoid skipping social tests in debug builds. I noticed this recently too! (https://tbpl.mozilla.org/?tree=Try&rev=d222940df46c)
Attachment #649989 - Flags: review?(gavin.sharp) → review+
No longer blocks: 776554
Assignee: nobody → mhammond
No longer blocks: 781386
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: