1342136 - Investigate why suggested site data wasn't available

Status: RESOLVED WORKSFORME

(Firefox for Android Graveyard :: Activity Stream, defect, P3)

Description

[Andrzej Hunt :ahunt]

See Bug 1342105.

We're seemingly suggesting suggested site icons when data isn't available for those sites. We should check why this is happening (maybe we're caching the icon "URL" somewhere, i.e 'suggestedsite://actual_url', and we're then hitting the scenario where site data isn't loaded as in Bug 1340957).

Comment 1

[Andrzej Hunt :ahunt]

Another possibility is that a distribution is supplying suggested sites, but doesn't provide icons for them (in which case there's nothing for us to do).

Comment 2

[Andrzej Hunt :ahunt]

(In reply to Andrzej Hunt :ahunt from comment #1)
> Another possibility is that a distribution is supplying suggested sites, but
> doesn't provide icons for them (in which case there's nothing for us to do).

But would we have distributions on nightly? This seems unlikely.

Comment 3

[Julien Wajsberg [:julienw]]

I got the issue with a nightly from Mozilla. 

I'm not sure I can give you a useful str though :(

Comment 4

[Andrzej Hunt :ahunt]

I have a hypothesis, which would happen *if* all of the below conditions are true:
1. Topsites hasn't been shown yet (meaning Suggested Sites has never been loaded)
2. A tab with one of the suggested sites is opened, and/or a suggested site is listed on a different panel (i.e. we load a favicon for one of the suggested sites) - so we're trying to load a suggested icon.
3. Icon mapping is in memory or on disk, and provided by LookupIconUrl (meaning we don't load suggestedsites in SuggestedSitePreparer).

If suggested sites have never been retrieved (i.e. the suggested sites cursor has never been created == SuggestedSites.get() has never been called), then getImageUrlForUrl()/getBackgroundColorForUrl() return null (cachedSites == null, and we return null in that case). Thus we hit this issue. A workaround is to ensure SuggestedSitesLoader runs SuggestedSites.get() at least once, _or_ fix SuggestedSites to actually load data in getImageForUrl()/getBackgroundColorForUrl() (but those methods might need to be UI thread safe, making that non-viable).

Comment 5

[Julien Wajsberg [:julienw]]

I confirm I wanted to open a site that already was in a different tab (in that case Twitter). 
Not sure how I can try the other points :)

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Unassigning ahunt: assuming he's not actively working on it.

-> P3: it's blocking as-android-notyet, which means it should be backlogged.

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

All open Activity Stream bugs are moving from the whiteboard tag, "[mobileAS]", to the Firefox for Android component, "Activity Stream", so that I can keep better track of these bugs as the new triage owner; I will send out an email shortly with additional details, caveats, etc.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I've seen this for bug 1396941: we would show these icons as placeholders first run but second run we'd show the actual icons.

That being said, this behavior should be fixed with bug 1396941. worksforme!