Ben Kelly [:bkelly, not reviewing] Assignee
	Description • 8 years ago

Recently I noticed that I always get an extra top level about:blank window whenever I open a new firefox window.  While this does not take up a lot of memory, every little allocation can contribute to fragmentation, etc.

I eventually traced this back to the social-sidebar-browser <browser> tag here:

https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul#1148

I commented this tag out locally and the about:blank window went away.  Whats more, I was still able to use the "share this page" button.  The only error I noticed was this at startup:

MozSocialAPI injectController: unable to attachToWindow for resource://gre-resources/hiddenWindow.html: TypeError: containingBrowser is null injectController() MozSocialAPI.jsm:84

Shane, would it be possible to delay this initialization until the user clicks "share this page" the first time?

	Shane Caraveo (:mixedpuppy)
	Comment 1 • 8 years ago

Sidebar has nothing to do with share, they are separate containers.  If the tests pass I'm fine with the change (I had thought we weren't loading it on startup).  Probably need to do some small change in MozSocialAPI.jsm or browser-social.js to prevent an attempt to load anything there.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 2 • 8 years ago

Can you point me to the code that has social sidebar disabled? I guess that was not obvious to me.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 3 • 8 years ago

I think this is easy enough to make work in our current configuration.  The trick is getting the tests to pass.

Shane, would you be ok if I added a SocialSidebar.init() that returns a promise that must be called if you want SocialSidebar to do anything?  I would add this to the tests, but our current firefox configuration would not call it.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 4 • 8 years ago

This is a work in progress.  I need to get a few more tests passing locally.

	Shane Caraveo (:mixedpuppy)
	Comment 5 • 8 years ago

Comment on attachment 8759925 [details] [diff] [review]
Make SocialSidebar load its <browser> window lazily. r=mixedpuppy

The direction this is going looks good.  Request review from :markh or :felipc when you're ready (I'm on leave currently)

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 6 • 8 years ago

Current work-in-progress:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eaa95436afad

	Eric Rahm [:erahm]
	Comment 7 • 8 years ago

Ben are you still working on this?

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 8 • 8 years ago

I have patches, but hit a blocker in the test cases.  It would be nice to get rid of these windows, but its a pretty small payoff overall.  P3 seems appropriate.

	Shane Caraveo (:mixedpuppy)
	Comment 9 • 8 years ago

deprecation in fx51

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 10 • 8 years ago

Does this mean its being removed from browser.xul init by default?

	Shane Caraveo (:mixedpuppy)
	Comment 11 • 8 years ago

yep, landed yesterday, no longer in 51.