Disable automatic update checks by default and add API for enabling them

[fhd]

Background

Libadblockplus is currently running regular update checks for the application (with data from AppInfo) in the background. We've added this for practical reasons: Since all of our own Libadblockplus clients (Android and IE) need update functionality. However, other Libadblockplus clients do not necessarily want to use this update mechanism, so it should be enabled explicitly.

What to change

1. Remove the update_url_release and devbuild_update_url prefs we set in Libadblockplus.
2. Remove AppInfo::developmentBuild - Libadblockplus doesn't need to know anymore.
3. Add a setter for the update URL to FilterEngine. Update checks should only happen if an URL has been set here, i.e. not by default.

[trev]

IMHO, a simpler option is to have a flag in AppInfo.

Edit: Thinking a bit more about it - probably not, that structure is being passed to JsEngine and is supposed to be generic. However, the application can remove update_url_release and update_url_devbuild preferences to indicate that it doesn't want updates.

[fhd]

How about this then:

1. We remove the default update URLs from lib/prefs.js
2. We add explicit setters for both update URLs

That way, updates will be disabled by default, and clients can enable release and devbuild update checks as they see fit.

That'll do for now. The remaining problem (more of a <=P3 thing) is that, IMO, FilterEngine just shouldn't be responsible for application update checks to begin with - they have nothing to do with "filters". I suppose it would be fine again if we rename it to "AdblockPlusEngine" or "AbpEngine" or something along those lines - but that wouldn't make it any less convoluted.

I think it would be cleaner to have a separate Updater component. That's non-trivial because it would use the same code FilterEngine uses under the hood, we'll probably need a shared base class for anything that wants to interact with the ABP code. But it would be pretty clean. What do you think, Wladimir?

[trev]

We add explicit setters for both update URLs

Then we don't need two update URLs - if the application sets them anyway then it can do the devbuild/release check itself, and we don't need to store the URLs in prefs either. I guess having the application explicitly enable automatic updates is indeed a better approach...

I think it would be cleaner to have a separate Updater component.

This is indeed non-trivial. I don't think that a shared base class is a good idea. We rather need an internal Loader component that will be used by both FilterEngine and Updater. It should be able to detect whether the JS files have been loaded already (e.g. by checking whether the API variable exists) - if not, it should load them. This should allow FilterEngine and Updater to be simple API wrappers.

[fhd]

Yeah, sounds good. Would you agree that having Updater and FilterEngine as separate components would be cleaner though?

[trev]

Yes, ideally we would have separate objects encapsulate different sets of functionality - FilterEngine is starting to get crowded. A separate Updater component would be a start.

[fhd]

Good good, here goes the follow-up: #1319.

[oleksandr]

The functionality to support this was implemented differently that is described here. See ​https://codereview.adblockplus.org/5653480979038208/ I think this issue can be closed.

[trev]

No, not really the same thing. This issue is about having applications using libadblockplus opt into update checks - only when the application can actually deal with updates. This isn't about user configuration.

[sergz]

Done as a part of refactoring.