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

Issue 775889 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

[Content suggestions] The new code path to fetch favicons uses too limiting max_size_factor

Project Member Reported by jkrcal@chromium.org, Oct 18 2017

Issue description

When we ask to fetch a favicon of size 32px, LargeIconService specifies max_size of the image to fetch to be 64px. This limit is too restricting for the favicon server as ~5% of our news sites publish only a larger icon.

We should increase the MaxSizeFactor in LargeIconService.
 
Labels: zine-triaged
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b8e471180e6cedd7929e706f1087a1468143ecac

commit b8e471180e6cedd7929e706f1087a1468143ecac
Author: Jan Krcal <jkrcal@chromium.org>
Date: Mon Oct 23 09:34:01 2017

[Large icon service] Increase the max_size for fetching favicons.

This CL increase the minimal max_size to 256px. The previous computation
max_size = desired_size * 2 was found to be too restrictive for icons
with low desired size (such as 32px).

Bug:  775889 
Change-Id: I90d729b3146847832358918fbbeea003d6d5a287
Reviewed-on: https://chromium-review.googlesource.com/725348
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510746}
[modify] https://crrev.com/b8e471180e6cedd7929e706f1087a1468143ecac/components/favicon/core/large_icon_service.cc
[modify] https://crrev.com/b8e471180e6cedd7929e706f1087a1468143ecac/components/favicon/core/large_icon_service_unittest.cc

Comment 3 by jkrcal@chromium.org, Oct 25 2017

Status: Fixed (was: Started)

Sign in to add a comment