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

Issue 896481 link

Starred by 1 user
Status: Fixed
Owner:
█ dim...@chromium.org
Last visit > 30 days ago
Closed: Nov 8
Cc:
dewittj@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Histogram ExploreSites.ClickedNTPCategoryIndex has incorrect number of buckets.

Project Member Reported by dim...@chromium.org, Oct 17 
The exact linear histogram that tracks numbers [0..N] should have N+2 histogram buckets:
[0, 1),... [N-1, N) and overflow bucket.

The current bounds for the ExploreSites.ClickedNTPCategoryIndex histogram are incorrectly specified, which makes it report incorrectly, merging 2 values into 1
buckets.jpeg
17.1 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 17 

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

commit 20db1c792df7838120597da8d046806bb81cbf63
Author: Dmitry Titov <dimich@chromium.org>
Date: Wed Oct 17 23:08:46 2018

Fix histogram ExploreSites.ClickedNTPCategoryIndex bucket count.

Bug:  896481 
Change-Id: I6e7d05e5673971d2b132e75dcaf1dfe7a858357a
Reviewed-on: https://chromium-review.googlesource.com/c/1287290
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Commit-Queue: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600588}
[modify] https://crrev.com/20db1c792df7838120597da8d046806bb81cbf63/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java

Comment 2 by dim...@chromium.org, Oct 18

Labels: Merge-Request-71
Status: Fixed (was: Started)
Project Member

Comment 3 by sheriffbot@chromium.org, Oct 19

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 4 by bugdroid1@chromium.org, Oct 19

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

commit ba1caa7b03a3d91a5869bf5b6ee85b02797f7134
Author: Dmitry Titov <dimich@chromium.org>
Date: Fri Oct 19 17:55:27 2018

Fix histogram ExploreSites.ClickedNTPCategoryIndex bucket count.

TBR=dimich@chromium.org

(cherry picked from commit 20db1c792df7838120597da8d046806bb81cbf63)

Bug:  896481 
Change-Id: I6e7d05e5673971d2b132e75dcaf1dfe7a858357a
Reviewed-on: https://chromium-review.googlesource.com/c/1287290
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Commit-Queue: Dmitry Titov <dimich@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600588}
Reviewed-on: https://chromium-review.googlesource.com/c/1291989
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#161}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/ba1caa7b03a3d91a5869bf5b6ee85b02797f7134/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java
Project Member

Comment 5 by cr-audit...@appspot.gserviceaccount.com, Oct 23

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

Commit: ba1caa7b03a3d91a5869bf5b6ee85b02797f7134
Author: dimich@chromium.org
Commiter: dimich@chromium.org
Date: 2018-10-19 17:55:27 +0000 UTC

Fix histogram ExploreSites.ClickedNTPCategoryIndex bucket count.

TBR=dimich@chromium.org

(cherry picked from commit 20db1c792df7838120597da8d046806bb81cbf63)

Bug:  896481 
Change-Id: I6e7d05e5673971d2b132e75dcaf1dfe7a858357a
Reviewed-on: https://chromium-review.googlesource.com/c/1287290
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Commit-Queue: Dmitry Titov <dimich@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600588}
Reviewed-on: https://chromium-review.googlesource.com/c/1291989
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#161}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Comment 6 by dewittj@chromium.org, Oct 24

Status: Assigned (was: Fixed)
This causes a crash.  The linear histogram code checks that MAX-MIN + 2 <= NUM_BUCKETS, but it modifies MIN=1.

Comment 7 by dewittj@chromium.org, Oct 24 

The crash is a DCHECK, may not be a blocker?
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 26 

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

commit fe1f597537a7ed68a180d066686c466df750ea8d
Author: Dmitry Titov <dimich@chromium.org>
Date: Fri Oct 26 22:23:40 2018

Fix bounds and bucket numbers for Explore Sites linear histograms to avoid DCHECK.

Bug:  896481 
Change-Id: Ib5cc008bdc896632b4ae65c14091af9cad343ee1
Reviewed-on: https://chromium-review.googlesource.com/c/1300112
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Commit-Queue: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603217}
[modify] https://crrev.com/fe1f597537a7ed68a180d066686c466df750ea8d/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategoryCardView.java
[modify] https://crrev.com/fe1f597537a7ed68a180d066686c466df750ea8d/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java

Comment 9 by dim...@chromium.org, Nov 8

Status: Fixed (was: Assigned)
►

Sign in to add a comment