775851 - Refactor the terrifying JS code in toolkit/components/url-classifier

Status: REOPENED

(Toolkit :: Safe Browsing, defect, P5)

Description

[Justin Dolske [:Dolske]]

In the beginning there was bug 769960. And, lo, dolske said that a 4:1 reduction in code size and a 4096-fold increase in readability was righteous. Life was good.

And then dolske asked, "so what's the deal with this list manager, anyway"? Thusly a darkness swept forth over his mxr tabs, and a malignant blight was spotted upon the codebase. And as he cracked the seal, a sepulchral voice echoed out of the temple saying unto the toolkit and browser peers; "Go your ways, and pour out the vials of the wrath of dolske upon url-classifier. All adventurers exit immediately through Main Office."

And having begun a refactoring, dolske said "Let the code be cleansed and gathered in modern form". A purifying fire vanquished the code, and a warm and reassuring glow settled across the land.

Comment 1

[Justin Dolske [:Dolske]]

The way timers are used in listmanager.js is really confusing.

It's trying to do the following (mostly sensible) things:

* Quickly check for any updates
  - If there's no DB data, do it immediately (well, in 3 seconds)
  - Otherwise do it in 0-5 minutes (randomly chosen)
* Schedule a recurring update every 30 minutes
  - Except, well, make the first of these happen in 15-45 minutes (randomly chosen)

This all gets rolling via maybeToggleUpdateChecking(), which in turn calls startUpdateChecker() and sometimes kickoffUpdate(). The result is there can actually be 2 active timers. One from the "quickly" case (calling checkForUpdates() as a 1-shot), and one from the "recurring" case [first a 1-shot to initialUpdateCheck(), when a recurring-timer to checkForUpdates()]

I'm this going to collapse this down to a single timer with a 1-shot for first-check, a 1-shot for second-check, and a recurring-timer henceforth. This makes it easier to understand as:

1) Quickly check for an update. (immediately or in 0-5 minutes)
2) After that, schedule a check in 15-45 minutes
3) After that, schedule a recurring check in 30 minutes.

Comment 2

[Justin Dolske [:Dolske]]

Not working on this.

Comment 3

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.