Implemented update checking functionality

Implemented update checking functionality

[Felix Dahlke, 2013-06-05 15:47:36]

LGTM with a few questions.

http://codereview.adblockplus.org/10800079/diff/1/include/AdblockPlus/FilterE...
File include/AdblockPlus/FilterEngine.h (right):

http://codereview.adblockplus.org/10800079/diff/1/include/AdblockPlus/FilterE...
include/AdblockPlus/FilterEngine.h:86: void ForceUpdateCheck(UpdaterCallback
callback = 0);
I suppose you considered using an std::function here? Would like to hear why
not.

http://codereview.adblockplus.org/10800079/diff/1/include/AdblockPlus/FilterE...
include/AdblockPlus/FilterEngine.h:95: void UpdateCheckDone(std::string
eventName, UpdaterCallback callback, JsValueList& params);
Any reason why eventName is not a const string reference?

http://codereview.adblockplus.org/10800079/diff/1/lib/updater.js
File lib/updater.js (right):

http://codereview.adblockplus.org/10800079/diff/1/lib/updater.js#newcode25
lib/updater.js:25: const HOURS_TO_MILLIS = 60 * 60 * 1000;
Would call that "MILLIS_PER_HOUR" but it really doesn't matter :D

http://codereview.adblockplus.org/10800079/diff/1/lib/updater.js#newcode37
lib/updater.js:37: if (Prefs.next_update_check - now > MAX_CHECK_INTERVAL)
I'm wondering what could cause this. If it's just the randomised interval
calculation below, I think we should move the check there.

http://codereview.adblockplus.org/10800079/diff/1/lib/updater.js#newcode67
lib/updater.js:67: throw new Error("Invalid update URL, HTTPS is mandatory for
updates");
Should we really hard code that? What's the problem with unencrypted HTTP?

[Wladimir Palant, 2013-06-05 18:58:21]

Addressed two of the questions with a new patch.

http://codereview.adblockplus.org/10800079/diff/1/include/AdblockPlus/FilterE...
File include/AdblockPlus/FilterEngine.h (right):

http://codereview.adblockplus.org/10800079/diff/1/include/AdblockPlus/FilterE...
include/AdblockPlus/FilterEngine.h:86: void ForceUpdateCheck(UpdaterCallback
callback = 0);
No, I somehow forgot about that option. Works fine with std::bind() then :)

http://codereview.adblockplus.org/10800079/diff/1/include/AdblockPlus/FilterE...
include/AdblockPlus/FilterEngine.h:95: void UpdateCheckDone(std::string
eventName, UpdaterCallback callback, JsValueList& params);
I had concerns where that reference would point to with a bound callback. Seems
to work however.

http://codereview.adblockplus.org/10800079/diff/1/lib/updater.js
File lib/updater.js (right):

http://codereview.adblockplus.org/10800079/diff/1/lib/updater.js#newcode25
lib/updater.js:25: const HOURS_TO_MILLIS = 60 * 60 * 1000;
On 2013/06/05 15:47:36, Felix H. Dahlke wrote:
> Would call that "MILLIS_PER_HOUR" but it really doesn't matter :D

It's a multiplication factor :)

http://codereview.adblockplus.org/10800079/diff/1/lib/updater.js#newcode37
lib/updater.js:37: if (Prefs.next_update_check - now > MAX_CHECK_INTERVAL)
Clocks can be off a lot - and sometimes they are corrected.

http://codereview.adblockplus.org/10800079/diff/1/lib/updater.js#newcode67
lib/updater.js:67: throw new Error("Invalid update URL, HTTPS is mandatory for
updates");
On 2013/06/05 15:47:36, Felix H. Dahlke wrote:
> Should we really hard code that? What's the problem with unencrypted HTTP?

Yes, we should make very sure that we secure updates. Unencrypted HTTP allows
modification on the flight, our app might be misused to push malware.

[Felix Dahlke, 2013-06-06 10:04:37]

LGTM