875924 - Defect - Topsites Thumbnails do not always appear

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Firefox Start, defect, P2)

Description

[Brian R. Bondy [:bbondy]]

1. Delete your MetroFirefox profile directories
C:\Users\Brian\AppData\Local\Mozilla\MetroFirefox
C:\Users\Brian\AppData\Roaming\Mozilla\MetroFirefox
2. Startup Metro Firefox
3. Open several sites
4. Wait for them all to load
5. Close Metro Firefox
6. Re-open Metro Firefox

Actual results: Some of the sites don't have thumbnails, they are a solid color instead.

Expected results: All of the sites have thumbnails.

Side note: For twitter in particular even if I load it several times, and click the tile, and restart several times it never loads the thumbnail.  The other tiles eventually loaded as I used the browser more.  But the main bug here is that they should show up sooner.

Comment 1

[Brian R. Bondy [:bbondy]]

Relating to the side note: Ally noted it's probably due to https.

I think the key thing on the main issue though is that if you open a url, and then switch tabs before it loads, then no thumbnail is generated.  Feel free to lower the priority on this defect.

Comment 2

[Brian R. Bondy [:bbondy]]

Thumbnails for the tab strip do show up by the way for the case in Comment 1, but not for the start screen.

Comment 3

[Brian R. Bondy [:bbondy]]

p=2

Comment 4

[Sam Foster [:sfoster] (he/him)]

BrowserUI.shouldCaptureThumbnails has a sequence of conditions which cause the thumbnail-taking to be skipped. Most of the sites I've seen where we don't get a thumbmail when one is expected (i.e. are not https) send Cache-Control: no-store headers.
We could use an in-memory thumbnail there perhaps (e.g. same canvas technique as we use for tabs). But it would be gone again when the browser next starts up and maybe is not an improvement. 
The other possibility we discussed to mitigate this issue was to find *something* to display in there. Maybe there's a msapplication-TileImage? 

I have not been able to reproduce the issue where thumbnails don't appear until later in a session. Let me know STR for that if its still an issue.

Comment 6

[Sam Foster [:sfoster] (he/him)]

I not been able to reproduce this reliably enough to diagnose so far. Maybe I'm just overlooking something simple, or maybe there's a pref to shrink the cache or some other tweak we can make to make this conditions in which this occurs come around more often? Meantime, unnassigning myself.

Comment 7

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This was a regression from bug 851130.  The expiration filter was part of the TopSitesView controller on about:start.  After we moved about:start into a tab, the filter was active only when an about:start tab was open.  This patch moves the expiration filter into the global TopSites object.  It also cleans up some unused code.

QA NOTE: This patch does not change the behavior on non-cacheable pages, where we intentionally do not capture thumbnails for privacy reasons (bug 841495).  It only fixes the problem of thumbnails that appear and then disappear, which happens if you use the browser for a while without having the Start page open.

Comment 8

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

Actually, NewTabUtils already contains code to do this; we just need to initialize it.

Comment 9

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 809563 [details] [diff] [review]
patch

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

Looks good. I still don't have reliable STR and I think QA will want them too? But I see the problem you are fixing here and it looks good.

::: browser/metro/base/content/TopSites.js
@@ +7,5 @@
>  /**
>   * singleton to provide data-level functionality to the views
>   */
>  let TopSites = {
> +  init: function () {

huh, the _initialized flag and TopSite.Site static were never used? Nice to trim some dead wood.

Comment 10

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 810035 [details] [diff] [review]
patch v2

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

Ah ha! I see we had too many inits in our inits (/me strokes beard)

Comment 11

[Matt Brubeck (:mbrubeck)]

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

QA notes: See comment 7.  Also, this bug happens if thumbnail expiration runs while about:start is not open.  The expiration runs about once an hour, so it can take a while to verify.

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/de92b36d6d5c

Comment 13

[Marco Mucci [:MarcoM]]

Temporarily reopening to add to iteration.  Will mark as resolved right after.

Comment 14

[Cornel Ionce [:noni] [Hubs QA]]

Mozilla/5.0 (Windows NT 6.2; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Windows NT 6.2; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Build ID: 20131020030202

Verified on latest nightly on Windows 8 32bit and 64bit.
The problem with the appearing/disappearing thumbnails is fixed. Tested for about two hours and did not encounter this issue anymore.
As mentioned, the thumbnails of non-cacheable pages is not captured (bug 841495).