Ed Lee :Mardak Reporter
	Description • 10 years ago

Bug 975475 will be providing the images/colors for the initial list.

Bug 975228 landed by avoiding leak failures when we actually had links to sites. The Linux/64 Debug bc1 test leaks include SpdySession31, so somehow one of the spdy-servers in the list (one/some/all of facebook.com, youtube.com, twitter.com see bug 975228 comment 38) is being loaded? favicon? prefetch?

The details of the leak from bug 975228 comment 32:

https://tbpl.mozilla.org/?tree=Try&rev=5da828c50df4
TEST-UNEXPECTED-FAIL | leakcheck | 4779 bytes leaked (EventTokenBucket, Mutex, ReentrantMonitor, Service, SpdySession3, ...)
leaked 2 EventTokenBucket (216 bytes)
leaked 10 Mutex (120 bytes)
leaked 3 ReentrantMonitor (48 bytes)
leaked 1 Service (64 bytes)
leaked 1 SpdySession31 (588 bytes)
leaked 3 StringAdopt (3 bytes)
leaked 1 ThirdPartyUtil (16 bytes)
leaked 1 TransportSecurityInfo (116 bytes)
leaked 1 nsCategoryObserver (60 bytes)
leaked 1 nsCookiePermission (36 bytes)
leaked 1 nsCookieService (84 bytes)
leaked 1 nsDBusService (24 bytes)
leaked 1 nsDNSRecord (28 bytes)
leaked 1 nsDNSService (88 bytes)
leaked 5 nsDeque (260 bytes)
leaked 1 nsEffectiveTLDService (52 bytes)
leaked 1 nsHashtable (44 bytes)
leaked 1 nsHostRecord (72 bytes)
leaked 2 nsHttpAuthCache::AppDataClearObserver (32 bytes)
leaked 1 nsHttpConnection (208 bytes)
leaked 1 nsHttpConnectionInfo (44 bytes)
leaked 1 nsHttpConnectionMgr (160 bytes)
leaked 1 nsHttpConnectionMgr::nsConnectionHandle (16 bytes)
leaked 1 nsHttpHandler (504 bytes)
leaked 1 nsIDNService (72 bytes)
leaked 1 nsIOService (104 bytes)
leaked 1 nsInterfaceRequestorAgg (24 bytes)
leaked 6 nsMainThreadPtrHolder<T> (72 bytes)
leaked 1 nsNSSCertificate (56 bytes)
leaked 1 nsNSSSocketInfo (196 bytes)
leaked 1 nsNetworkManagerListener (24 bytes)
leaked 1 nsObserverService (52 bytes)
leaked 1 nsPermissionManager (100 bytes)
leaked 1 nsPrefBranch (76 bytes)
leaked 1 nsSSLStatus (52 bytes)
leaked 1 nsSiteSecurityService (60 bytes)
leaked 1 nsSocketTransport (332 bytes)
leaked 1 nsSocketTransportService (148 bytes)
leaked 1 nsStreamConverterService (72 bytes)
leaked 18 nsStringBuffer (144 bytes)
leaked 7 nsTArray_base (28 bytes)
leaked 1 nsThread (124 bytes)
leaked 1 nsTimerImpl (68 bytes)
leaked 1 nsUnicodeNormalizer (12 bytes)
leaked 5 nsWeakReference (80 bytes)

	Ed Lee :Mardak Reporter
	Comment 1 • 10 years ago

Anyone have insights into how to debug this Linux/64 Debug bc1 leak failure? We believe it's related to a SPDY site, and somehow the leaktest doesn't fail when I converted all the directoryLinks to https://www.mozilla.org/

	Ed Lee :Mardak Reporter
	Comment 2 • 10 years ago

The simple fix is to sidestep the issue by Services.prefs.setCharPref(PREF_NEWTAB_DIRECTORYSOURCE, "data:application/json,{}"); at the beginning of mochitest, so all tests that happen to open about:newtab will continue to get blank tiles.

	Ed Lee :Mardak Reporter
	Comment 3 • 10 years ago

I pushed various tryserver tests that set the url to the actual site instead of mozilla.org at various positions.

Facebook #1:   Leak https://tbpl.mozilla.org/?tree=Try&rev=af2ca7c08573
YouTube  #1:   Leak https://tbpl.mozilla.org/?tree=Try&rev=dfee822f5579
Twitter  #1: NOleak https://tbpl.mozilla.org/?tree=Try&rev=f806559e46a2
YouTube  #3: NOleak https://tbpl.mozilla.org/?tree=Try&rev=efa1bd512cd1
Twitter  #6: NOleak https://tbpl.mozilla.org/?tree=Try&rev=76e617d90463
Facebook #9: NOleak https://tbpl.mozilla.org/?tree=Try&rev=1a1bd60dfbfd

All 3 of Facebook, YouTube, and Twitter have X-Firefox-Spdy: "3.1" in the response headers, so not sure why Twitter isn't causing leaks when in position #1. I would guess that the mouse pointer is over position 1 and causes some loading?

Another note is that the leak is intermittent.

	Ed Lee :Mardak Reporter
	Comment 4 • 10 years ago

(In reply to Ed Lee :Mardak from comment #3)
> Twitter  #1: NOleak https://tbpl.mozilla.org/?tree=Try&rev=f806559e46a2
> not sure why Twitter isn't causing leaks when in position #1
Turns out Twitter in spot 1 is leaking. Just at a much lower rate:

Twitter 1 leak failure out of 14
Facebook 3 leaks in 4
YouTube 3 leaks in 4

So at least it's consistent-ish.

	Ed Lee :Mardak Reporter
	Comment 5 • 10 years ago

Putting http://www.facebook.com/ (not https) in spot #1 does not cause the leak. https://tbpl.mozilla.org/?tree=Try&rev=544389210a65

	Allison Naaktgeboren :ally
	Comment 6 • 10 years ago

Could y'all annotate the strings on the placeholder tiles on about:newtab to something like BBC(placeholder) ? 

It's very confusing to click on a BBC tile and get https://www.mozilla.org/en-US/#http://www.bbc.co.uk/ instead. It looks like broken behavior instead of 'hang on, we're working on this'

	Ed Lee :Mardak Reporter
	Comment 7 • 10 years ago

(In reply to :Ally Naaktgeboren from comment #6)
> Could y'all annotate the strings on the placeholder tiles on about:newtab to
> something like BBC(placeholder) ? 
But but... there's a big PLACEHOLDER image above the string. ;) This would get fixed through bug 991853 where we'll put in actual links and images to Mozilla properties for now.

	Allison Naaktgeboren :ally
	Comment 8 • 10 years ago

(In reply to Ed Lee :Mardak from comment #7)
> (In reply to :Ally Naaktgeboren from comment #6)
> > Could y'all annotate the strings on the placeholder tiles on about:newtab to
> > something like BBC(placeholder) ? 
> But but... there's a big PLACEHOLDER image above the string. ;) This would
> get fixed through bug 991853 where we'll put in actual links and images to
> Mozilla properties for now.

Right, so it's clear that image temporary, not that the link is borked. :)

	Matthew N. [:MattN]
	Comment 9 • 10 years ago

This is from the speculative connections being made[1] from tiles on the new tab page. See bug 964358 and bug 964369 comment 3 (and the patch there) where we may disable these connections by default in tests.

[1] https://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/sites.js?rev=f3c44cfc3900#182

	Ed Lee :Mardak Reporter
	Comment 10 • 10 years ago

Ah ha! I should have looked deeper into the newtab code. I was grepping for prefetch or preload. :(

Here are some notes from maksik:

I am running tests 24x7 to repro the leak with minimal luck so far:
1) I did manage to compile debug+refcount version of fx on linux 64bit
2) running it is a major pain, as it often either hangs or exit with cascading errors: "JavaScript Error: "source.defaultView is null".
3) sometimes the VM instance gets horcked and i have to reboot it
4) I did manage to complete a run of mochitest-browser for browser/base and broswer/components which cover 99.9% of all the tests in BC1 bucket that triggers the leak.  AND.... I saw no leaks.

The item 4) above is alerting.  I am using our github repo master branch, not the fxteam branch. Which bags the question if our changes are implicated to begin with.

Given how painful the bug is, I am going to schedule a meeting to chat of how we could be systematic about reproducing the leak.
We may still have to look for a real linux box, as VM on my poor machine does not seem to be happy running these massive tests.

	Ed Lee :Mardak Reporter
	Comment 11 • 10 years ago

Bug 992611 turns off speculative connects for tests and this should prevent the leak failures.

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 13 • 10 years ago

Boriss, can you update the YouTube logo and color according to these links?

https://developers.google.com/youtube/branding_guidelines
https://www.youtube.com/yt/brand/using-logo.html

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 14 • 10 years ago

Borris, here's our ready to go list so far:

+–––––––––––––––––––––––––+
|                         |
| +–––––+ +–––––+ +–––––+ |
| |  1  | |  2  | |  3  | |
| +–––––+ +–––––+ +–––––+ |
|                         |
| +–––––+ +–––––+ +–––––+ |
| |  4  | |  5  | |  6  | |
| +–––––+ +–––––+ +–––––+ |
|                         |
| +–––––+ +–––––+ +–––––+ |
| |  7  | |  8  | |  9  | |
| +–––––+ +–––––+ +–––––+ |
|                         |
+–––––––––––––––––––––––––+            

Directory Tiles with background colors:
                                                  
1. 
2. youtube.com, #e5523f
3. twitter.com, #00b5f0
4. wikipedia.com, #ffffff
5. 
6. 
7. wired.com, #1facdf
8. 
9. 

Ready Alternatives (throw these in somewhere):
* mozilla.org, #4d4e54
* amazon.com, #000000
* trulia.com

Not (Yet) Ready Alternates:
* facebook.com, #3a5898
* reddit.com, 
* weather.com,

Not Ready Alternates:

10. lonelyplanet.com, #002f74 
11. wordpress.com, #008fc0
12. nytimes.com, #0093b9
13. eff.org, #000000
14. npr.org, #c7d2d7

	Jennifer Morrow [:Boriss] (UX)
	Comment 15 • 10 years ago

(In reply to Bryan Clark (Firefox Search PM) [:clarkbw] from comment #13)
> Boriss, can you update the YouTube logo and color according to these links?
> 
> https://developers.google.com/youtube/branding_guidelines
> https://www.youtube.com/yt/brand/using-logo.html

Attached is a white logo, background is #424242

	Ed Lee :Mardak Reporter
	Comment 18 • 10 years ago

Boriss, these logo files you've provided aren't of the tile size. How should we make them fit the tile? Just take the logo file and have it centered in the tile space and fill in with background color?

	Ed Lee :Mardak Reporter
	Comment 19 • 10 years ago

Boriss, nevermind. These show up fine when centered/centered. Originally I was confused because attachment 8392628 [details] says 112x82 or smaller and these logo files are 180x86

	Jennifer Morrow [:Boriss] (UX)
	Comment 20 • 10 years ago

Amazon wanted a version of the logo without the ".com" - new version attached

	Jennifer Morrow [:Boriss] (UX)
	Comment 21 • 10 years ago

(In reply to Ed Lee :Mardak from comment #19)
> Boriss, nevermind. These show up fine when centered/centered. Originally I
> was confused because attachment 8392628 [details] says 112x82 or smaller and
> these logo files are 180x86

Np.  It's possible you'll need different version for retina, I imagine.  I included the original images but can make 2x size if needed.

	Ed Lee :Mardak Reporter
	Comment 22 • 10 years ago

clarkbw, is this ordering and type pairing correct?

1. amazon organic
2. youtube organic
3. wired sponsored
4. wikipedia organic
5. mozilla affiliate
6. twitter organic
7. lightbeam affiliate
8. trulia sponsored
9. ? eff affiliate ? facebook organic ?

	Ed Lee :Mardak Reporter
	Comment 25 • 10 years ago

(In reply to Marina Samuel [:emtwo] from comment #23)
> Created attachment 8412131 [details] [diff] [review]
> WIP: Applying the assets available so far
Thanks, you can find more assets in bug 995001

	Ed Lee :Mardak Reporter
	Comment 28 • 10 years ago

(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #16)
> Created attachment 8407856 [details]
> Image: Trulia white logo without wordmark (preferred), background #9acb3c
Where did you get that color for Trulia? I see another document with #9ACA3C but their website seems to have a darker green #5EAB1F

	Ed Lee :Mardak Reporter
	Comment 29 • 10 years ago

There might be some image/url/title/type tweaks, but the code changes should be good.

	Ed Lee :Mardak Reporter
	Comment 31 • 10 years ago

Is this okay Boriss? I took the lightbeam wordmark from the page and resized to 180 wide then added transparency to 86px tall.. (although technically the extra transparency above/below isn't necessary).

	Drew Willcoxon :adw
	Comment 32 • 10 years ago

Comment on attachment 8412238 [details] [diff] [review]
v1

Review of attachment 8412238 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/directoryLinks.json
@@ +1,4 @@
>  {
>    "en-US": [
>      {
> +      "url": "http://www.amazon.com",

Nit: the other URLs have trailing slashes (which is more "correct" anyway).

	Ed Lee :Mardak Reporter
	Comment 33 • 10 years ago

https://hg.mozilla.org/integration/fx-team/rev/79989d4a8d24

leave-open for potentially more image/position/title/type/url changes.

	Carsten Book [:Tomcat]
	Comment 34 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/79989d4a8d24

	Marco Mucci [:MarcoM]
	Comment 35 • 10 years ago

Removing from iteration until final decisions are made regarding potentially more image/position/title/type/url changes.

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 38 • 10 years ago

Here's the "final" order/type/text/url

= Order =

1. Facebook
2. YouTube
3. Wired
4. Lightbeam
5. MoFo
6. Wikipedia
7. Trulia
8. Amazon
9. Webmaker

= Types =

- Organic -

Facebook
YouTube
Wikipedia
Amazon

- Affiliate -

Lightbeam
MoFo
Webmaker

- (Trial) Sponsored -

Wired
Trulia

= Text:URL = 

Facebook :	https://www.facebook.com/
YouTube  : 	http://www.youtube.com/
Wired 	 :      http://www.wired.com/
Lightbeam:	https://www.mozilla.org/en-US/lightbeam/
MoFo     :      http://www.mozilla.org/en-US/about/
Wikipedia:	http://en.wikipedia.org/wiki/Main_Page
Trulia   :      http://www.trulia.com/?ecampaign=tiles
Amazon   :      http://www.amazon.com/gp/bit/amazonbookmark.html?tag=mozilla-directory-tiles-20&partner=Mozilla
Webmaker	 :      https://webmaker.org/

Only reason for the "final" in quotes is the Wired creative and the Mozilla center tile are being reviewed right now.  Everything else should be good.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 42 • 10 years ago

this was backed out:
https://hg.mozilla.org/integration/fx-team/rev/24cf989be9e3

tracked in bug 1000459.

To add more to this, we have a performance regression from this patch, 5.33% TART regression on win7 and winXP.  When we backed it out, the regression went away:
http://graphs.mozilla.org/graph.html#tests=[[293,132,25]]&sel=none&displayrange=7&datatype=running

I had done some retriggers to verify it was a real regression and not a random spike of electricity or some noise:
https://tbpl.mozilla.org/?tree=Fx-Team&startdate=2014-04-29&enddate=2014-04-31&jobname=Windows%207%2032-bit%20fx-team%20talos%20svgr

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 43 • 10 years ago

oh and this also had a 2.22% regression on linux32

	Ed Lee :Mardak Reporter
	Comment 44 • 10 years ago

jmaher, the regression is the same as bug 990212 where the change that actually triggers the regression is the one line change to turn on the feature in bug 1000459. I believe the plan is to populate the tiles for TART to have the test run consistently (no matter what we change the list of links) and be more "realistic" where users will be seeing tiles with content as opposed to all empty.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 45 • 10 years ago

ah, thanks!  I had blinders on and just filed things, thanks for clarifying the relationship!

	Piotr Okoński
	Comment 46 • 10 years ago

Proposed background: #1D4F8D

Disclaimer: I do some work for LonelyPlanet and I asked their designers to prepare a logo.

	Ed Lee :Mardak Reporter
	Comment 47 • 10 years ago

Updated various ordering, inserted bbc to spot 9 (moving webmaker to 10), Wired -> WIRED, new WIRED logo, Mozilla Foundation -> The Nonprofit Behind Firefox, updated urls, crushed images

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 49 • 10 years ago

(In reply to Piotr Okoński from comment #46)
> Created attachment 8417963 [details]
> LonelyPlanet logo
> 
> Proposed background: #1D4F8D
> 
> Disclaimer: I do some work for LonelyPlanet and I asked their designers to
> prepare a logo.

Thanks Piotr, this is great!  I sent a message to someone who has our contact at Lonely Planet so we can keep in touch with them.

	Ed Lee :Mardak Reporter
	Comment 50 • 10 years ago

https://hg.mozilla.org/integration/fx-team/rev/4d19381c20ee

	Carsten Book [:Tomcat]
	Comment 51 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/4d19381c20ee