1045163 - remove delay before updating existing tables on disk, only initialize updateCheckers if updates are needed

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[[:mmc] Monica Chew (no longer reading bugmail)]

Currently the listmanager waits 0-5 minutes before updating existing tables on disk. Why?

 for (var updateUrl in this.needsUpdate_) {
    // If the user has tables, add a fuzz of a few minutes.
    if (updatingExisting) {
      // Add a fuzz of 0-5 minutes.
      initialUpdateDelay += Math.floor(Math.random() * (5 * 60 * 1000));
      log("Waiting " + initialUpdateDelay / 1000 +
          " for updating existing table from " + updateUrl);
    }

Comment 1

[[:mmc] Monica Chew (no longer reading bugmail)]

Hey gcp, do you know the history of this check?

Comment 2

[Gian-Carlo Pascutto [:gcp]]

No, I don't. I've always assumed it's historical to stop the servers from being swamped on new Firefox releases. Probably not much of an issue these days.

(It's also bad for mobile because the browser might not even have a lifetime that long.)

Comment 3

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Remove delay before updating existing tables on disk (

Comment 4

[[:mmc] Monica Chew (no longer reading bugmail)]

https://tbpl.mozilla.org/?tree=Try&rev=9caf37e24ed8

Comment 5

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Remove delay before updating existing tables on disk (

Comment 6

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8469400 [details] [diff] [review]
Remove delay before updating existing tables on disk (

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

Please re-review. I found a corner case which was hidden by the previous behavior (delay after flipping pref allowed enough time to enable/disable before the first update was kicked off).

Comment 7

[[:mmc] Monica Chew (no longer reading bugmail)]

https://tbpl.mozilla.org/?tree=Try&rev=b57848fa50f2

Comment 8

[[:mmc] Monica Chew (no longer reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f5e5b0d07766

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f5e5b0d07766

Comment 10

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8469400 [details] [diff] [review]
Remove delay before updating existing tables on disk (

Approval Request Comment
[Feature/regressing bug #]: bug 1021419
[User impact if declined]: In rare circumstances, enabling safebrowsing after being in a disabled state will fail to work until the next browser restart.
[Describe test coverage new/current, TBPL]: Manual
[Risks and why]: Low risk.
[String/UUID change made/needed]: None.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2211ef492219

Comment 12

[Gian-Carlo Pascutto [:gcp]]

Somehow we read over this part of the documentation:

https://developers.google.com/safe-browsing/developers_guide_v2?hl=en

Client Behavior:

    The first request for data MUST happen at a random interval between 0 and 5 minutes after the browser starts.

I will restore the behavior in Bug 1175562.

Comment 13

[François Marier [:francois]]

That's going to make our tests more annoying. I'll make a note of it.

Comment 14

[Gian-Carlo Pascutto [:gcp]]

https://bugzilla.mozilla.org/show_bug.cgi?id=1175562#c2

as you pointed out in the review comments we can force an immediate update by setting the nextupdate in the past.