Ignore v4 completion too early will cause telemetry::URLCLASSIFIER_MATCH_RESULT gets wrong results

RESOLVED FIXED in Firefox 55

Status

()

Toolkit
Safe Browsing
P2
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: dimi, Assigned: dimi)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: #sbv4-m6)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

9 months ago
Right now we enable completion but ignore the lookup result by checking preference in nsUrlClassifierLookupCallback::Completion[1]. But this will cause us measure incorrect match result[2] because completions for v4 are ignored.

We should skip v4 completion result in nsUrlClassifierLookupCallback::HandleResults to get correct telemetry record.

[1] https://dxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#1139
[2] https://dxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#1269
(Assignee)

Updated

9 months ago
Blocks: 1167038
Whiteboard: #sbv4-m6
Comment hidden (mozreview-request)

Comment 2

9 months ago
mozreview-review
Comment on attachment 8851493 [details]
Bug 1350798 - Ignore v4 completion too early will cause telemetry::URLCLASSIFIER_MATCH_RESULT gets wrong results.

https://reviewboard.mozilla.org/r/123814/#review126362

::: commit-message-cc537:5
(Diff revision 1)
> +Bug 1350798 - Ignore v4 completion too early will cause telemetry::URLCLASSIFIER_MATCH_RESULT gets wrong results. r?francois
> +
> +Enable safebrowsing v4 completion but ignore the result by checking preference in
> +nsUrlClassifierLookupCallback::Completion may cause telemetry measure incorrect match
> +result since v4 completions will always be ingored.

typo: ignored
Attachment #8851493 - Flags: review?(francois) → review+
Priority: -- → P2
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Keywords: checkin-needed

Comment 4

9 months ago
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/770d755ed395
Ignore v4 completion too early will cause telemetry::URLCLASSIFIER_MATCH_RESULT gets wrong results. r=francois
Keywords: checkin-needed

Comment 5

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/770d755ed395
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.