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 ignored.
So in this patch we move the preference check after telemetry is measured and then we
ignore the result.
MozReview-Commit-ID: J29JitvW3Lc
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -889,17 +889,20 @@ nsUrlClassifierDBServiceWorker::CacheCom
activeTable = true;
break;
}
}
if (activeTable) {
TableUpdateV2* tuV2 = TableUpdate::Cast<TableUpdateV2>(
pParse->GetTableUpdate(resultsPtr->ElementAt(i).table));
- NS_ENSURE_TRUE(tuV2, NS_ERROR_FAILURE);
+ // Ignore V4 for now.
+ if (!tuV2) {
+ continue;
+ }
LOG(("CacheCompletion Addchunk %d hash %X", resultsPtr->ElementAt(i).entry.addChunk,
resultsPtr->ElementAt(i).entry.ToUint32()));
rv = tuV2->NewAddComplete(resultsPtr->ElementAt(i).entry.addChunk,
resultsPtr->ElementAt(i).entry.complete);
if (NS_FAILED(rv)) {
// We can bail without leaking here because ForgetTableUpdates
// hasn't been called yet.
@@ -1131,24 +1134,16 @@ nsUrlClassifierLookupCallback::Completio
NS_IMETHODIMP
nsUrlClassifierLookupCallback::Completion(const nsACString& completeHash,
const nsACString& tableName,
uint32_t chunkId)
{
LOG(("nsUrlClassifierLookupCallback::Completion [%p, %s, %d]",
this, PromiseFlatCString(tableName).get(), chunkId));
- if (StringEndsWith(tableName, NS_LITERAL_CSTRING("-proto")) &&
- !Preferences::GetBool(TAKE_V4_COMPLETION_RESULT_PREF,
- TAKE_V4_COMPLETION_RESULT_DEFAULT)) {
- // Bug 1331534 - We temporarily ignore hash completion result
- // for v4 tables.
- return NS_OK;
- }
-
mozilla::safebrowsing::Completion hash;
hash.Assign(completeHash);
// Send this completion to the store for caching.
if (!mCacheResults) {
mCacheResults = new CacheResultArray();
if (!mCacheResults)
return NS_ERROR_OUT_OF_MEMORY;
@@ -1247,16 +1242,24 @@ nsUrlClassifierLookupCallback::HandleRes
}
if (!confirmed) {
LOG(("Skipping result %s from table %s (not confirmed)",
result.PartialHashHex().get(), result.mTableName.get()));
continue;
}
+ if (StringEndsWith(result.mTableName, NS_LITERAL_CSTRING("-proto")) &&
+ !Preferences::GetBool(TAKE_V4_COMPLETION_RESULT_PREF,
+ TAKE_V4_COMPLETION_RESULT_DEFAULT)) {
+ // Bug 1331534 - We temporarily ignore hash completion result
+ // for v4 tables.
+ continue;
+ }
+
LOG(("Confirmed result %s from table %s",
result.PartialHashHex().get(), result.mTableName.get()));
if (tables.IndexOf(result.mTableName) == nsTArray<nsCString>::NoIndex) {
tables.AppendElement(result.mTableName);
}
if (classifyCallback) {