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

Issue 808198 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

FaviconCache::page_favicon_map_ can grow beyond reasonable limits in some cases

Project Member Reported by ssid@chromium.org, Feb 1 2018

Issue description

If there is a URL redirect loop on a website, all the page_urls have the same icon_url. So, we end up having only one entry in the synced_favicons cache. But the page_favicon_map_ keeps growing indefinitely. 
Related issue 750845,  732969 .
 
Components: Services>Sync
Owner: ----

Comment 2 by treib@chromium.org, Feb 19 2018

Labels: Needs-Feedback OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
I don't follow where the problem is: A redirect loop means there's a finite set of URLs. page_favicon_map_ is an std::map, so it can have at most one entry per URL. How can it grow indefinitely?
I feel like I'm missing something :)  Could you provide more details on what exactly the problem is, or maybe repro steps? Thanks!

Comment 3 by ssid@chromium.org, Feb 20 2018

Labels: -Needs-Feedback
Ah you're right, I guess it was not redirect loop, but just a lot of redirects then. I do not have repro steps, I have just observed reports from users with stack traces allocating like 4-5MB memory at FaviconCache::OnFaviconVisited() and the only way that could grow is if the icon_url is same and page_url is different. If the icon_url were different then we have a maximum of 200 favicons stored in sync and beyond that the map is cleared.

Comment 4 by treib@chromium.org, Feb 22 2018

Cc: mastiz@chromium.org
Status: Available (was: Untriaged)
Summary: FaviconCache::page_favicon_map_ can grow beyond reasonable limits in some cases (was: FaviconCache::page_favicon_map_ is not cleared in some cases)
Okay, I guess adding some limit on the maximum number of entries is the only option then?

Sign in to add a comment