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

Issue 760496 link

Starred by 0 users

Issue metadata

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



Sign in to add a comment

Fix recording of NewTabPage.LogoShownTime

Project Member Reported by treib@chromium.org, Aug 30 2017

Issue description

This histogram is supposed to track how long it takes for Doodles to show up on the Android NTP. However, it's currently only ever recorded on the first onLogoAvailable call, which always returns the result from cache (whether we have a cached Doodle or not). So when we don't have a cached Doodle yet, we don't record how long it takes for the fresh Doodle to show up, making the whole histogram kinda useless.

I'm not sure if it's possible to fix this at the recording site, because the LogoObserver contract is weird and doesn't guarantee we'll be called a second time.
 

Comment 1 by treib@chromium.org, Aug 30 2017

Looking some more, I think in the case where we do have a cached Doodle, we record NewTabPage.LogoShown twice: Once for the cached Doodle, then again for the fresh one, even if it's identical.

Comment 2 by treib@chromium.org, Aug 30 2017

Owner: treib@chromium.org
Status: Started (was: Available)
Labels: zine-triaged
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 31 2017

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

commit 1d8f396c560f794c856c19b2e90c4644d3670623
Author: Marc Treib <treib@chromium.org>
Date: Thu Aug 31 08:54:46 2017

Fix NewTabPage.LogoShown and .LogoShownTime recording

Before this CL, .LogoShownTime was only recorded for cached logos, i.e.
it missed the case where the logo needed to be fetched from the network.
After this CL, it records the first non-null logo (if any), which was
the original intention.

This also adds a note to the description for .LogoShown explaining that
it can be recorded twice per NTP (cached + fresh), and it adds a
.FromCache vs .Fresh split.

Bug:  760496 
Change-Id: I689ce352f2f6af27d833c3de7da03169a41e2d94
Reviewed-on: https://chromium-review.googlesource.com/643298
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498792}
[modify] https://crrev.com/1d8f396c560f794c856c19b2e90c4644d3670623/chrome/android/java/src/org/chromium/chrome/browser/ntp/LogoDelegateImpl.java
[modify] https://crrev.com/1d8f396c560f794c856c19b2e90c4644d3670623/tools/metrics/histograms/histograms.xml

Comment 5 by treib@chromium.org, Aug 31 2017

Status: Fixed (was: Started)

Sign in to add a comment