Closed Bug 1365877 Opened 5 years ago Closed 5 years ago

resetDatabase in SafeBrowsing test case should also reset cache

Categories

(Toolkit :: Safe Browsing, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: dimi, Assigned: dimi)

References

Details

(Whiteboard: #sbv4-m7)

Attachments

(1 file)

The purpose of classifierHelper.resetDB is to reset both database and cache.

It accomplishes this by applying an update to expire existing chunks. And since
applying an update will also clear all the caches(old design), both database and cache will be cleared.

But in Bug 1333328, we sync the caching mechanism for V2 and V4, now apply an update will not clear cache, it will only remove expired ones[1]

We should make sure resetDB will still clear the cache.

[1] https://searchfox.org/mozilla-central/source/toolkit/components/url-classifier/tests/mochitest/classifierHelper.js#110
Whiteboard: #sbv4-m7
Priority: -- → P2
Blocks: 1359299
Comment on attachment 8869294 [details]
Bug 1365877 - resetDatabase in SafeBrowsing test case should also reset cache.

https://reviewboard.mozilla.org/r/140852/#review145012

Patch looks great. I would suggest changing the commit message to capture the details you included in the bug description, as well as the extra information you told me in person. Maybe something along these lines:

"resetDatabase() is used to clear out the Safe Browsing database and cache in tests. Since bug 1333328 however we can no longer rely on updates clearing the cache.

In order to clear the cache after updating the database, resetDatabase() now calls another test-only function: reloadDatabase(). Since the cache is in memory, reloading the URL classifier will automatically clear the cache."

::: commit-message-baf05:1
(Diff revision 1)
> +Bug 1365877 - resetDB in SafeBrowsing test case should also reset cache. r?francois

It's now called `resetDatabase()` :)
Attachment #8869294 - Flags: review?(francois) → review+
Summary: resetDB in SafeBrowsing test case should also reset cache → resetDatabase in SafeBrowsing test case should also reset cache
Pushed by dlee@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b8b6ab63c4c3
resetDatabase in SafeBrowsing test case should also reset cache. r=francois
https://hg.mozilla.org/mozilla-central/rev/b8b6ab63c4c3
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.