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

Issue 776571 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

DCHECK calculating WebHistory.LocalResultMissingOnServer

Project Member Reported by s...@chromium.org, Oct 19 2017

Issue description

The logic to calculate WebHistory.LocalResultMissingOnServer is currently wrong, and thus can hit a DCHECK. The local_result_count is pre-pending, and the missing_count is post-pending.

The current logic tries is trying to preserve pre-duplicated counts as well, which we do not need to continue to support. They're already being deduped by the local db, so at this point we've lost the true information anyways. We should probably be able to simply iterate over results and count up LOCAL / (LOCAL + COMBINED).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 21 2017

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

commit 953da679604151f9d7f9f03aa6ab8429e2479a43
Author: Sky Malice <skym@chromium.org>
Date: Sat Oct 21 00:06:41 2017

Rework WebHistory.LocalResultMissingOnServer calculation.

This histogram was made incorrect during the CL
https://chromium-review.googlesource.com/c/chromium/src/+/577968 where
we started holding back pending results in certain circumstances. The
DCHECK would be hit if pending local results were later added to local
results. The counts need to both come from pre or post pending removal,
and only count each URL once.

Additionally, no longer try to count pre-de-duplicated urls.

Bug:  776571 
Change-Id: Ic6d700c636dd063efa43e841030ab651931f1a75
Reviewed-on: https://chromium-review.googlesource.com/730569
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510621}
[modify] https://crrev.com/953da679604151f9d7f9f03aa6ab8429e2479a43/components/history/core/browser/browsing_history_service.cc

Comment 2 by s...@chromium.org, Oct 30 2017

Status: Fixed (was: Assigned)

Sign in to add a comment