Fix #3113: Fetch adblock data every 6 hours.

[iccub]

Summary of Changes

This pull request fixes #3113

Security review https://github.com/brave/security/issues/269

Submitter Checklist:

• Unit Tests are updated to cover new or changed functionality
• User-facing strings use NSLocalizableString()

Test Plan:

1. Launch the app
2. Open app settings -> adblock debug
3. Write down last fetch time value
4. On dev build wait 10+ minutes, on prod 6+ hours
5. Close the app or switch to another app, no need to remove it from memory
6. Open it again, open adblock debug
7. Verify that last fetch time value changed

Screenshots:

Reviewer Checklist:

• Issues include necessary QA labels:
  • QA/(Yes|No)
  • release-notes/(include|exclude)
  • bug / enhancement
• Necessary security reviews have taken place.
• Adequate unit test coverage exists to prevent regressions.
• Adequate test plan exists for QA to validate (if applicable).
• Issue is assigned to a milestone (should happen at merge time).

[iccub]

blocked to decide on fetch interval, i like to fetch more often than 24hours, set it to 6 for now

[diracdeltas]

can we add some randomization to this so it's not as much of a user fingerprint? ex: each time the fetch is supposed to occur, delay it by a random amount between 1 and 10 minutes.

[kylehickinson]

@diracdeltas This will only be non-random if the user happens to open their phone every 6 hours since it only triggered on app foreground, not on a specific timer.

[kylehickinson]

Changes look good :) Nice little debug addition you added for QA.

Not sure if we need a sec review for this since we're adding additional network calls?

[iccub]

let's request a security reivew

[soner-yuksel]

I have a suggestion. what do you think about adding resource downloading into also applicationDidEnterBackground.

Use background task and use that window efficiently. We can define a background task with

private var backgroundTaskIdentifier: UIBackgroundTaskIdentifier?
backgroundTaskIdentifier = UIApplication.shared.beginBackgroundTask {
    self.endBackgroundTask()
 }

and have an background task finish method like

private func endBackgroundTask() {
    guard let backgroundTaskIdentifier = backgroundTaskIdentifier else { return }
    UIApplication.shared.endBackgroundTask(backgroundTaskIdentifier)
    self.backgroundTaskIdentifier = nil
}

and also we can have completion block from AdBlockResourceDownloader and end the task If operation is success.

And since we have also resource downloading in foreground this will help us improving the download experience and utilize the background window

[iccub]

overkill imo, why would you want to download it both when the app goes in background and in foreground?

[soner-yuksel]

If we will download while going to background using background task, it will not need to download while app is coming foreground and occupy a thread and do processing.
Why we need to add both(background/foreground) is background task can initiate processes that doesnt take too long and there is no guarantee it will succeed so in that case foreground can initialize the process if it is needed.

But yes this is a suggestion, not necessary.