New issue
Advanced search Search tips

Issue 736265 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature



Sign in to add a comment

[LargeIconService] Clients cannot properly record results of fetches from Google favicon server

Project Member Reported by jkrcal@chromium.org, Jun 23 2017

Issue description

Currently, LargeIconService provides boolean |success| to the callback from GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache(). There are multiple reasons for failures and the current solution does not allow clients of this API to track proper metrics.

We should expose more information about the status of the favicon server request.
 

Comment 1 Deleted

Comment 2 by jkrcal@chromium.org, Jun 23 2017

In particular: from the current NewTabPage.TileFaviconFetchSuccess.Server metric, we cannot tell the success rate. The reason is that we mix together 
 - 404s received from the server with 
 - cached 404s (where no request is sent out).
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 10 2017

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

commit f3e5733eeac736500e54bc25bb8db3f3c248459e
Author: Jan Krcal <jkrcal@chromium.org>
Date: Mon Jul 10 09:06:11 2017

[LargeIconService] Provide richer status to the google server callback.

This CL allows more informative metrics for clients of LargeIconService.

Bug:  736265 
Change-Id: I5f0b855db06180436a3afba845c702f615eb0f0f
Reviewed-on: https://chromium-review.googlesource.com/544931
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485210}
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/components/favicon/core/large_icon_service.cc
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/components/favicon/core/large_icon_service.h
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/components/favicon/core/large_icon_service_unittest.cc
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/components/favicon_base/favicon_callback.h
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/components/favicon_base/favicon_types.h
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/components/ntp_snippets/content_suggestions_service.cc
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/components/ntp_snippets/content_suggestions_service.h
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/components/ntp_tiles/icon_cacher_impl.cc
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/components/ntp_tiles/icon_cacher_impl.h
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/components/ntp_tiles/icon_cacher_impl_unittest.cc
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/f3e5733eeac736500e54bc25bb8db3f3c248459e/tools/metrics/histograms/histograms.xml

Comment 4 by jkrcal@chromium.org, Jul 24 2017

Status: Fixed (was: Assigned)

Sign in to add a comment