[MostLikely favicons] Decrease min_size to 0 and make params configurable
This CL changes the minimum size for fetching MostLikely favicons to 0
px. This means that we download even small icons and thus display a
colored tile instead of a gray tile.
Furthermore, the CL makes the min and desired sizes configurable by
field trial parameters to allow for more flexibility in experimenting /
launching.
Bug: 536988
Change-Id: I872243cc315b103eca9d30883f4c00026abe18eb
Reviewed-on: https://chromium-review.googlesource.com/567986
Commit-Queue: Jan Krcal <[email protected]>
Reviewed-by: Mikel Astiz <[email protected]>
Cr-Commit-Position: refs/heads/master@{#486356}diff --git a/components/ntp_tiles/icon_cacher_impl.cc b/components/ntp_tiles/icon_cacher_impl.cc
index 00bb3b16d..7b9e06ff 100644
--- a/components/ntp_tiles/icon_cacher_impl.cc
+++ b/components/ntp_tiles/icon_cacher_impl.cc
@@ -7,6 +7,7 @@
#include <utility>
#include "base/memory/ptr_util.h"
+#include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_macros.h"
#include "components/favicon/core/favicon_service.h"
#include "components/favicon/core/favicon_util.h"
@@ -16,6 +17,7 @@
#include "components/favicon_base/favicon_util.h"
#include "components/image_fetcher/core/image_decoder.h"
#include "components/image_fetcher/core/image_fetcher.h"
+#include "components/ntp_tiles/constants.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/geometry/size.h"
@@ -31,8 +33,11 @@
// TODO(jkrcal): Make the size in dip and the scale factor be passed as
// arguments from the UI so that we desire for the right size on a given device.
// See crbug.com/696563.
-constexpr int kTileIconMinSizePx = 48;
-constexpr int kTileIconDesiredSizePx = 96;
+constexpr int kDefaultTileIconMinSizePx = 1;
+constexpr int kDefaultTileIconDesiredSizePx = 96;
+
+constexpr char kTileIconMinSizePxFieldParam[] = "min_size";
+constexpr char kTileIconDesiredSizePxFieldParam[] = "desired_size";
favicon_base::IconType IconType(const PopularSites::Site& site) {
return site.large_icon_url.is_valid() ? favicon_base::TOUCH_ICON
@@ -52,6 +57,18 @@
return result.fallback_icon_style->is_default_background_color;
}
+int GetMinimumFetchingSizeForChromeSuggestionsFaviconsFromServer() {
+ return base::GetFieldTrialParamByFeatureAsInt(
+ kNtpMostLikelyFaviconsFromServerFeature, kTileIconMinSizePxFieldParam,
+ kDefaultTileIconMinSizePx);
+}
+
+int GetDesiredFetchingSizeForChromeSuggestionsFaviconsFromServer() {
+ return base::GetFieldTrialParamByFeatureAsInt(
+ kNtpMostLikelyFaviconsFromServerFeature, kTileIconDesiredSizePxFieldParam,
+ kDefaultTileIconDesiredSizePx);
+}
+
} // namespace
IconCacherImpl::IconCacherImpl(
@@ -200,7 +217,8 @@
// Desired size 0 means that we do not want the service to resize the image
// (as we will not use it anyway).
large_icon_service_->GetLargeIconOrFallbackStyle(
- page_url, kTileIconMinSizePx, /*desired_size_in_pixel=*/0,
+ page_url, GetMinimumFetchingSizeForChromeSuggestionsFaviconsFromServer(),
+ /*desired_size_in_pixel=*/0,
base::Bind(&IconCacherImpl::OnGetLargeIconOrFallbackStyleFinished,
weak_ptr_factory_.GetWeakPtr(), page_url),
&tracker_);
@@ -247,7 +265,9 @@
})");
large_icon_service_
->GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache(
- page_url, kTileIconMinSizePx, kTileIconDesiredSizePx,
+ page_url,
+ GetMinimumFetchingSizeForChromeSuggestionsFaviconsFromServer(),
+ GetDesiredFetchingSizeForChromeSuggestionsFaviconsFromServer(),
/*may_page_url_be_private=*/true, traffic_annotation,
base::Bind(&IconCacherImpl::OnMostLikelyFaviconDownloaded,
weak_ptr_factory_.GetWeakPtr(), page_url));
diff --git a/components/ntp_tiles/icon_cacher_impl.h b/components/ntp_tiles/icon_cacher_impl.h
index e15ceaf..671b020 100644
--- a/components/ntp_tiles/icon_cacher_impl.h
+++ b/components/ntp_tiles/icon_cacher_impl.h
@@ -52,6 +52,8 @@
PopularSites::Site site,
const base::Closure& icon_available,
const base::Closure& preliminary_icon_available) override;
+
+ // TODO(jkrcal): Rename all instances of "MostLikely" to "ChromeSuggestions".
void StartFetchMostLikely(const GURL& page_url,
const base::Closure& icon_available) override;
