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




Safe Browsing
6 years ago
2 years ago


(Reporter: Dolske, Unassigned)


Firefox Tracking Flags

(Not tracked)




6 years ago
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

6 years ago
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

5 years ago
Not working on this.
Assignee: dolske → nobody


4 years ago
Component: Phishing Protection → Phishing Protection
Product: Firefox → Toolkit
Priority: -- → P5
Whiteboard: [burn it with fire]
You need to log in before you can comment on or make changes to this bug.