Last Comment Bug 775779 - social sidebar tests are marked as "leaking" the sidebar document
: social sidebar tests are marked as "leaking" the sidebar document
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: SocialAPI (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 17
Assigned To: Mark Hammond [:markh]
:
Mentors:
Depends on: 728294
Blocks: 781386
  Show dependency treegraph
 
Reported: 2012-07-19 16:29 PDT by :Gavin Sharp [email: gavin@gavinsharp.com]
Modified: 2012-08-10 01:57 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
prevent "leaking" of the sidebar document. (1.59 KB, patch)
2012-07-23 17:38 PDT, Mark Hammond [:markh]
no flags Details | Diff | Splinter Review
updated against trunk (2.16 KB, patch)
2012-07-29 23:13 PDT, Mark Hammond [:markh]
no flags Details | Diff | Splinter Review
Don't enable test in mozSocial_api due to bug 778639 (2.15 KB, patch)
2012-07-30 06:24 PDT, Mark Hammond [:markh]
markh: review-
Details | Diff | Splinter Review
Avoid skipping social tests in debug builds. (1.43 KB, patch)
2012-08-08 01:22 PDT, Mark Hammond [:markh]
gavin.sharp: review+
Details | Diff | Splinter Review

Description :Gavin Sharp [email: gavin@gavinsharp.com] 2012-07-19 16:29:58 PDT
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.
Comment 1 Tim Taubert [:ttaubert] 2012-07-22 02:21:06 PDT
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.
Comment 2 Dão Gottwald [:dao] 2012-07-22 05:02:37 PDT
(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.
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-07-22 11:33:20 PDT
Yes, we'll need to port bug 728426's fix to the social sidebar code.
Comment 4 Mark Hammond [:markh] 2012-07-23 17:38:26 PDT
Created attachment 645144 [details] [diff] [review]
prevent "leaking" of the sidebar document.

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.
Comment 5 Mark Hammond [:markh] 2012-07-29 23:13:30 PDT
Created attachment 647076 [details] [diff] [review]
updated against trunk

This version also removes the "skip if debug" check from browser_social_sidebar.js
Comment 6 Mark Hammond [:markh] 2012-07-30 06:24:34 PDT
Created attachment 647138 [details] [diff] [review]
Don't enable test in mozSocial_api due to bug 778639
Comment 7 Mark Hammond [:markh] 2012-07-31 17:21:58 PDT
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.
Comment 8 Mark Hammond [:markh] 2012-08-08 01:22:22 PDT
Created attachment 649989 [details] [diff] [review]
Avoid skipping social tests in debug builds.

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).
Comment 9 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-08-08 08:48:48 PDT
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)
Comment 10 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-08-09 16:58:04 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/a461b5f53b20
Comment 11 Ed Morley [:emorley] 2012-08-10 01:57:59 PDT
https://hg.mozilla.org/mozilla-central/rev/a461b5f53b20

Note You need to log in before you can comment on or make changes to this bug.