1442486 - LookupCacheV4 doesn't mark the cache as primed

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, enhancement, P1)

Description

[François Marier [:francois]]

While testing my patch for bug 1441345 on Fennec, I noticed that V4 gethash requests were not being saved:

My patch is a little flaky which might be because the results that come back
from the fullhash request are not being cached:

[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::CompletionV4 [0x95595250, googpub-phish-proto, 300]
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::CompletionFinished [0x95595250, NS_OK]
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::CompletionV4 [0x95595250, googpub-phish-proto, 300]
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::CompletionFinished [0x95595250, NS_OK]
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::CompletionV4 [0x95595250, googpub-phish-proto, 300]
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::CompletionFinished [0x95595250, NS_OK]
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::CompletionV4 [0x95595250, googpub-phish-proto, 300]
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::CompletionFinished [0x95595250, NS_OK]
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::CompletionV4 [0x95595250, googpub-phish-proto, 300]
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::CompletionFinished [0x95595250, NS_OK]
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierLookupCallback::HandleResults [0x95595250, 5 results]
[1924:Main Thread]: D/UrlClassifierDbService Confirmed result 25EC334E from table googpub-phish-proto
[1924:Main Thread]: D/UrlClassifierDbService nsUrlClassifierClassifyCallback::HandleResult [0x980b2800, table googpub-phish-proto full hash JewzTjFxbQAH72XOTbsJ6ykDRZ7rRzrEytnTL4aaL8c=]
[1924:Main Thread]: D/UrlClassifierDbService Skipping result 9F261EF2 from table googpub-phish-proto (noise)
[1924:Main Thread]: D/UrlClassifierDbService Skipping result 7C47289C from table googpub-phish-proto (noise)
[1924:Main Thread]: D/UrlClassifierDbService Skipping result F02C5215 from table googpub-phish-proto (noise)
[1924:Main Thread]: D/UrlClassifierDbService Skipping result C1139F73 from table googpub-phish-proto (noise)
[1924:URL Classifier]: D/UrlClassifierDbService nsUrlClassifierDBServiceWorker::CacheCompletions [0x95683cf0]
[1924:URL Classifier]: D/UrlClassifierDbService Completion received, but table is not active, so not caching.
[1924:URL Classifier]: D/UrlClassifierDbService Completion received, but table is not active, so not caching.
[1924:URL Classifier]: D/UrlClassifierDbService Completion received, but table is not active, so not caching.
[1924:URL Classifier]: D/UrlClassifierDbService Completion received, but table is not active, so not caching.
[1924:URL Classifier]: D/UrlClassifierDbService Completion received, but table is not active, so not caching.
[1924:URL Classifier]: D/UrlClassifierDbService Applying 0 table gethashes.

Adding a bit of debugging, it seems like the google4 tables are not returned by mClassifier->ActiveTables():

[1864:URL Classifier]: D/UrlClassifierDbService Active tables: mozstd-trackwhite-digest256,mozplugin-block-digest256,base-track-digest256,test-malware-simple,test-phish-simple,test-unwanted-simple,test-harmful-simple,test-track-simple,test-trackwhite-simple,test-block-simple
[1864:URL Classifier]: D/UrlClassifierDbService Completion received, but table googpub-phish-proto is not active, so not caching.
[1864:URL Classifier]: D/UrlClassifierDbService Completion received, but table googpub-phish-proto is not active, so not caching.
[1864:URL Classifier]: D/UrlClassifierDbService Completion received, but table googpub-phish-proto is not active, so not caching.
[1864:URL Classifier]: D/UrlClassifierDbService Completion received, but table googpub-phish-proto is not active, so not caching.
[1864:URL Classifier]: D/UrlClassifierDbService Completion received, but table googpub-phish-proto is not active, so not caching.

The array of active tables is set within Classifier::RegenActiveTables(): https://searchfox.org/mozilla-central/rev/61d400da1c692453c2dc2c1cf37b616ce13dea5b/toolkit/components/url-classifier/Classifier.cpp#962

and only the V2 tables show up in the RegenActiveTables() log after updating google4:

[1864:URL Classifier]: D/UrlClassifierDbService Classifier::AsyncApplyUpdates
[1864:Classifier Update]: D/UrlClassifierDbService Step 1. ApplyUpdatesBackground on update thread.
[1864:Classifier Update]: D/UrlClassifierDbService Copy in-use directory content for update.
[1864:Classifier Update]: D/UrlClassifierDbService Applying 3 table updates.
[1864:Classifier Update]: D/UrlClassifierDbService Classifier::UpdateTableV4(googpub-phish-proto)
[1864:Classifier Update]: D/UrlClassifierDbService Classifier::UpdateTableV4(goog-harmful-proto)
[1864:Classifier Update]: D/UrlClassifierDbService Classifier::UpdateTableV4(goog-unwanted-proto)
[1864:Classifier Update]: D/UrlClassifierDbService update took 249ms
[1864:URL Classifier]: D/UrlClassifierDbService Step 2. ApplyUpdatesForeground on caller thread
[1864:URL Classifier]: D/UrlClassifierDbService Swapping directories safebrowsing-updating and safebrowsing...
[1871:URL Classifier]: D/UrlClassifierDbService Active v2 table: test-block-simple
[1871:URL Classifier]: D/UrlClassifierDbService Active v2 table: test-trackwhite-simple
[1871:URL Classifier]: D/UrlClassifierDbService Active v2 table: test-track-simple
[1871:URL Classifier]: D/UrlClassifierDbService Active v2 table: test-harmful-simple
[1871:URL Classifier]: D/UrlClassifierDbService Active v2 table: test-unwanted-simple
[1871:URL Classifier]: D/UrlClassifierDbService Active v2 table: test-phish-simple
[1871:URL Classifier]: D/UrlClassifierDbService Active v2 table: test-malware-simple
[1871:URL Classifier]: D/UrlClassifierDbService Inactive table (cache not primed): goog-unwanted-proto
[1871:URL Classifier]: D/UrlClassifierDbService Inactive table (cache not primed): goog-harmful-proto
[1871:URL Classifier]: D/UrlClassifierDbService Inactive table (cache not primed): googpub-phish-proto
[1871:URL Classifier]: D/UrlClassifierDbService Active v2 table: base-track-digest256
[1871:URL Classifier]: D/UrlClassifierDbService Active v2 table: mozplugin-block-digest256
[1871:URL Classifier]: D/UrlClassifierDbService Active v2 table: mozstd-trackwhite-digest256
[1864:URL Classifier]: D/UrlClassifierDbService Done swap in updated tables.
[1864:URL Classifier]: D/UrlClassifierDbService Step 3. Updates applied! Fire callback.

The extra debugging I added above shows that the cache is not marked as primed for V4 tables.

Comment 1

[François Marier [:francois]]

Attachment: Bug 1442486 - Mark LookupCacheV4 as primed after creating it.

RegenActiveTables() relies on mPrimed being set correctly and so
the V4 lookup cache should behave the same way as the V2 one.

The V2 lookup cache on the other hand was unnecessarily setting
mPrimed to true twice.

Review commit: https://reviewboard.mozilla.org/r/224558/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/224558/

Comment 2

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8955378 [details]
Bug 1442486 - Mark LookupCacheV4 as primed after creating it.

https://reviewboard.mozilla.org/r/224558/#review230568

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e02f0924908
Mark LookupCacheV4 as primed after creating it. r=gcp

Comment 4

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/8e02f0924908