New issue
Advanced search Search tips

Issue 896095 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Missing OfflinePage metrics reporting from the Downloads Home

Project Member Reported by carlosk@chromium.org, Oct 17

Issue description

The metric:

* Android.DownloadManager.InitialCount.Viewed.OfflinePage

is not being reported even thought they are declared histograms.
 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 19

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

commit 6195222c2431e6d83b569a4b5297c5d9be8ce890
Author: Carlos Knippschild <carlosk@chromium.org>
Date: Fri Oct 19 01:42:01 2018

Adds missing OfflinePage downloads metrics reporting

This change also splits the reporting of offline page counts between
prefetched and non-prefetched.

Bug: 896095, 894643
Change-Id: Ic3bfc53ecd2558df1b349af66bbe7d3f88bd6531
Reviewed-on: https://chromium-review.googlesource.com/c/1285750
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601015}
[modify] https://crrev.com/6195222c2431e6d83b569a4b5297c5d9be8ce890/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
[modify] https://crrev.com/6195222c2431e6d83b569a4b5297c5d9be8ce890/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java
[modify] https://crrev.com/6195222c2431e6d83b569a4b5297c5d9be8ce890/tools/metrics/histograms/histograms.xml

This has been fixed for the current Downloads Home code.
Reassigning to dtrainor@ to make sure this is carried over to the new DH.
Cc: -dtrainor@chromium.org
Owner: dtrainor@chromium.org
(now actually reassigning)
Status: Assigned (was: Untriaged)
Cc: dtrainor@chromium.org
Owner: carlosk@chromium.org
I did it too early: first this needs to be merged into M71 branch.
Sorry for the back and forth.


Labels: Merge-Request-71
I'd like to request the merging of commit 6195222c2431e6d83b569a4b5297c5d9be8ce890 into the M71 branch.

dtrainor@ has code reviewed the fix and considers it to be safe.

This change has been first release on Oct 20th with Canary 72.0.3586.0 and I confirmed today the metrics are WAI on Canary 72.0.3588.0.
+1 I consider this patch safe to merge.
Project Member

Comment 9 by sheriffbot@chromium.org, Oct 23

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 23

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec9a3902c6bcaefa0a08ac47ea31894ebdc0bf41

commit ec9a3902c6bcaefa0a08ac47ea31894ebdc0bf41
Author: Carlos Knippschild <carlosk@chromium.org>
Date: Tue Oct 23 22:08:00 2018

Adds missing OfflinePage downloads metrics reporting (M71 merge)

This change also splits the reporting of offline page counts between
prefetched and non-prefetched.

Bug: 896095, 894643
Change-Id: Ic3bfc53ecd2558df1b349af66bbe7d3f88bd6531
Reviewed-on: https://chromium-review.googlesource.com/c/1285750
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601015}(cherry picked from commit 6195222c2431e6d83b569a4b5297c5d9be8ce890)
Reviewed-on: https://chromium-review.googlesource.com/c/1297303
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#277}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/ec9a3902c6bcaefa0a08ac47ea31894ebdc0bf41/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
[modify] https://crrev.com/ec9a3902c6bcaefa0a08ac47ea31894ebdc0bf41/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java
[modify] https://crrev.com/ec9a3902c6bcaefa0a08ac47ea31894ebdc0bf41/tools/metrics/histograms/histograms.xml

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ec9a3902c6bcaefa0a08ac47ea31894ebdc0bf41

Commit: ec9a3902c6bcaefa0a08ac47ea31894ebdc0bf41
Author: carlosk@chromium.org
Commiter: carlosk@chromium.org
Date: 2018-10-23 22:08:00 +0000 UTC

Adds missing OfflinePage downloads metrics reporting (M71 merge)

This change also splits the reporting of offline page counts between
prefetched and non-prefetched.

Bug: 896095, 894643
Change-Id: Ic3bfc53ecd2558df1b349af66bbe7d3f88bd6531
Reviewed-on: https://chromium-review.googlesource.com/c/1285750
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601015}(cherry picked from commit 6195222c2431e6d83b569a4b5297c5d9be8ce890)
Reviewed-on: https://chromium-review.googlesource.com/c/1297303
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#277}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Labels: -Pri-1 Pri-2
Owner: dtrainor@chromium.org
Now off to dtrainor@ to guarantee this is kept with the new DH implementation. Also lowering priority.

Sign in to add a comment