Closed Bug 997872 Opened 10 years ago Closed 10 years ago

Make delay before first safebrowsing download smaller/configurable.

Categories

(Toolkit :: Safe Browsing, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: mmc, Assigned: gcp)

References

Details

Attachments

(1 file)

gcp, can you take this one? I think you know this code better than me.
Flags: needinfo?(gpascutto)
So it turns out the code already does the right thing:

  // If the user has never downloaded tables, do the check now.
  // If the user has tables, add a fuzz of a few minutes.
  var initialUpdateDelay = 3000;
  if (tableData != "") {
    // Add a fuzz of 0-5 minutes.
    initialUpdateDelay += Math.floor(Math.random() * (5 * 60 * 1000));

So on an empty profile, we'll start the first update after 3 seconds already!

But wait...that doesn't actually happen, does it?

afeBrowsing: initializing safe browsing URLs, client id  Firefox
[listmanager] Set update url: https://safebrowsing.google.com/safebrowsing/downloads?client=Firefox&appver=31.0a1&pver=2.2&key=no-google-api-key
[listmanager] Set gethash url: https://safebrowsing.google.com/safebrowsing/gethash?client=Firefox&appver=31.0a1&pver=2.2
SafeBrowsing: addMozEntries: n:1000
i:test-malware-simple
ad:1
a:1:32:40
itisatrap.org/firefox/its-an-attack.htmln:1000
i:test-phish-simple
ad:1
a:1:32:37
itisatrap.org/firefox/its-a-trap.html
SafeBrowsing: phishingEnabled: true malwareEnabled: true
[listmanager] Enabling table updates for goog-phish-shavar
[listmanager] Starting managing lists
[listmanager] startUpdateChecker repeatingUpdateDelay: 2393213
[listmanager] Enabling table updates for goog-malware-shavar
[listmanager] Starting managing lists
[listmanager] startUpdateChecker repeatingUpdateDelay: 1763766
[listmanager] Enabling table updates for goog-badbinurl-shavar
[listmanager] Starting managing lists
[listmanager] startUpdateChecker repeatingUpdateDelay: 1127278
[listmanager] Enabling table updates for 
[listmanager] Starting managing lists
[listmanager] startUpdateChecker repeatingUpdateDelay: 2122796
SafeBrowsing: init() finished
[listmanager] kickoffUpdate tableData: test-malware-simple;a:1
test-phish-simple;a:1

[listmanager] kickoffUpdate delay: 187418

Nope. Because we insert the test tables, SafeBrowsing considers we already have tables, and will apply the 0-5 minute delay.
Flags: needinfo?(gpascutto)
>[listmanager] Enabling table updates for 

Uh, this stinks of a bug in the patch for bug 997759 :( :(
1. Fix the registering of "" lists when the pref is unset.
2. Don't consider tables-we-are-not-updating when checking if this is a first update or not.

This doesn't do what the bug describes, but it turns out that if we fix this but that won't be needed: the first update will already happen 3 seconds after startup.
Assignee: nobody → gpascutto
Attachment #8409001 - Flags: review?(mmc)
Comment on attachment 8409001 [details] [diff] [review]
Patch 1. Fix delay logic for first update

Review of attachment 8409001 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/url-classifier/SafeBrowsing.jsm
@@ +15,5 @@
>  function getLists(prefName) {
> +  let pref = Services.prefs.getCharPref(prefName);
> +  if (!pref) {
> +    return [];
> +  } else {

No "else" needed, you are already returning

::: toolkit/components/url-classifier/content/listmanager.js
@@ +187,5 @@
>    // actually updating.
>    this.maybeToggleUpdateChecking();
>  }
>  
> +PROT_ListManager.prototype.kickoffUpdate_ = function (onDiskTableData)

Can you add a comment saying that this implements nsIUrlClassifierCallback?
Attachment #8409001 - Flags: review?(mmc) → review+
>Can you add a comment saying that this implements nsIUrlClassifierCallback?

I had to read https://developer.mozilla.org/en-US/docs/Creating_JavaScript_callbacks_in_components to make sense of it. I've added a comment saying "it acts as nsIUrlClassifierCallback for getTables".
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a9610eb15b2
Summary: Make delay before downloading safebrowsing lists configurable. → Make delay before first safebrowsing download smaller/configurable.
https://hg.mozilla.org/mozilla-central/rev/2a9610eb15b2
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.