Commit 36955f04 authored by Tarun Bansal's avatar Tarun Bansal Committed by Commit Bot

Reenable DCHECKs in navigation predictor

Some of the DCHECKs were disabled due to a bug which was causing
same anchor element to be reported multiple times to the browser process.
Now, since that bug has been fixed, we can reenable some of the
DCHECKs in navigation predictor which ensure that the size occupied
by any target HREF (after aggregating across all anchor elements
that point to the same target) does not exceed 1.0.

Bug: 894092
Change-Id: Iec003a857e8b990bcc01f522e49ed8cdd6191137
Reviewed-on: https://chromium-review.googlesource.com/c/1277372
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: default avatarRyan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598875}
parent 125c8de8
......@@ -238,6 +238,18 @@ void NavigationPredictor::MergeMetricsSameTargetUrl(
prev_metric->ratio_area += metric->ratio_area;
prev_metric->ratio_visible_area += metric->ratio_visible_area;
// After merging, value of |ratio_area| can go beyond 1.0. This can
// happen, e.g., when there are 2 anchor elements pointing to the same
// target. The first anchor element occupies 90% of the viewport. The
// second one has size 0.8 times the viewport, and only part of it is
// visible in the viewport. In that case, |ratio_area| may be 1.7.
if (prev_metric->ratio_area > 1.0)
prev_metric->ratio_area = 1.0;
DCHECK_LE(0.0, prev_metric->ratio_area);
DCHECK_GE(1.0, prev_metric->ratio_area);
DCHECK_GE(1.0, prev_metric->ratio_visible_area);
// Position related metrics are tricky to merge. Another possible way to
// merge is simply add up the calculated navigation scores.
prev_metric->ratio_distance_root_top =
......@@ -391,9 +403,7 @@ double NavigationPredictor::CalculateAnchorNavigationScore(
(double)((number_of_anchors - area_rank)) / number_of_anchors;
DCHECK_LE(0, metrics.ratio_visible_area);
// TODO(tbansal): https://crbug.com/891719. Disable the check until the bug
// for duplicate anchor elements is fixed.
// DCHECK_GE(1, metrics.ratio_visible_area);
DCHECK_GE(1, metrics.ratio_visible_area);
DCHECK_LE(0, metrics.is_in_iframe);
DCHECK_GE(1, metrics.is_in_iframe);
......@@ -430,10 +440,7 @@ double NavigationPredictor::CalculateAnchorNavigationScore(
// Normalize to 100.
score = score / sum_scales_ * 100.0;
DCHECK_LE(0.0, score);
// TODO(tbansal): https://crbug.com/891719. Disable the check until the bug
// for duplicate anchor elements is fixed.
// DCHECK_GE(100.0, score);
DCHECK_GE(100.0, score);
return score;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment