New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 696563 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , iOS
Pri: 2
Type: Task

Blocked on:
issue 644102



Sign in to add a comment

Make IconCacherImpl use only LargeIconService and pass the cached icon into the UI.

Project Member Reported by jkrcal@chromium.org, Feb 27 2017

Issue description

We are adding a new functionality to downloads icons on demand.
We should make IconCacherImpl to use this functionality for PopularSites and not talk to FaviconService directly.

Actually we should make sure we do not query the cache three times for the same icon. What we possibly do now:
 1. IconCacherImpl queries the local cache - no result, we initiate the fetch.
 2. UI queries the local cache via LargeIconService - no result, we display the gray tile
 3. IconCacherImpl gets the data, stores it into the cache and notifies the UI there is new data
 4. The UI again queries the local cache via LargeIconService and gets and displays the icon.

The workaround is that 
 - the IconCacher handles all types of tiles, talks to LargeIconService and passes the result to the UI;
 - the UI never asks the cache on its own, it only waits for the notification (containing the icon) from c++.

Does it SG, Chris?
 

Comment 1 by sfiera@chromium.org, Feb 27 2017

Looks fine on Android and I think for iOS. This might be a collection of bugs, though: one each for the component, Android and iOS.

Any thoughts about Desktop? We didn't think about that for Popular Sites because no Popular Sites, but ntp_tiles on Desktop is coming.
Components: UI>Browser>NewTabPage
One more comment for this refactoring: 
 - the UI should also pass minimum pixel size (currently 48px) and 
 - desired pixel size (48dp * scale factor)
for the icon of a tile. This will empower the cacher to download what is precisely needed.
Components: -UI>Browser>NewTabPage UI>Browser>ContentSuggestions
Labels: -Type-Bug Type-Task
Project Member

Comment 5 by sheriffbot@chromium.org, Sep 28

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: thegreenfrog@chromium.org
Status: Available (was: Untriaged)

Sign in to add a comment