Issue 2437 - Update adblockplus dependency to revision 752ffe2eeaf1

Issue 2437 - Update adblockplus dependency to revision 752ffe2eeaf1

[Felix Dahlke, 2015-04-29 16:47:38]

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/lib/...
File lib/prefs.js (right):

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/lib/...
lib/prefs.js:55: Prefs.__defineGetter__(key, function() { return values[key] });
Had to change this - jshydra doesn't handle function expressions anymore as of
https://issues.adblockplus.org/ticket/427.

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/test...
File test/UpdateCheck.cpp (right):

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/test...
test/UpdateCheck.cpp:135: "&lastVersion=0&downloadCount=0";
New parameter introduced by https://issues.adblockplus.org/ticket/537, same
below.

[Wladimir Palant, 2015-04-29 17:42:56]

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/READ...
File README.md (right):

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/READ...
README.md:12: ./ensure_dependencies.py
I still think that this should be part of the build process. Not sure what your
problem with that is but ensure_dependencies.py will merely produce a warning if
you have local changes.

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/lib/...
File lib/prefs.js (right):

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/lib/...
lib/prefs.js:55: Prefs.__defineGetter__(key, function() { return values[key] });
On 2015/04/29 16:47:38, Felix H. Dahlke wrote:
> Had to change this - jshydra doesn't handle function expressions anymore as of
> https://issues.adblockplus.org/ticket/427.

Why not change it into |() => values[key]|? Note that ideally this would also
use Object.defineProperty() (see https://issues.adblockplus.org/ticket/653).

[Felix Dahlke, 2015-04-30 05:06:11]

New patch set is up.

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/READ...
File README.md (right):

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/READ...
README.md:12: ./ensure_dependencies.py
On 2015/04/29 17:42:57, Wladimir Palant wrote:
> I still think that this should be part of the build process. Not sure what
your
> problem with that is but ensure_dependencies.py will merely produce a warning
if
> you have local changes.

Let's move that discussion here:
https://intraforum.adblockplus.org/t/calling-ensure-dependencies-py-at-build-...

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/lib/...
File lib/prefs.js (right):

http://codereview.adblockplus.org/5519479341580288/diff/5629499534213120/lib/...
lib/prefs.js:55: Prefs.__defineGetter__(key, function() { return values[key] });
On 2015/04/29 17:42:57, Wladimir Palant wrote:
> On 2015/04/29 16:47:38, Felix H. Dahlke wrote:
> > Had to change this - jshydra doesn't handle function expressions anymore as
of
> > https://issues.adblockplus.org/ticket/427.
> 
> Why not change it into |() => values[key]|? Note that ideally this would also
> use Object.defineProperty() (see https://issues.adblockplus.org/ticket/653).

Done.

[Wladimir Palant, 2015-04-30 08:30:24]

http://codereview.adblockplus.org/5519479341580288/diff/5757334940811264/lib/...
File lib/prefs.js (right):

http://codereview.adblockplus.org/5519479341580288/diff/5757334940811264/lib/...
lib/prefs.js:71: }
enumerable: true please, otherwise debugging will be no fun.

[Felix Dahlke, 2015-04-30 13:17:38]

New patch set is up.

The discussion regarding how to integrate ensure_dependencies.py (and if we want
this to be consistent across all projects) hasn't really concluded yet, so I've
left that out.

But integrating ensure_dependencies.py into the build here doesn't seem to be
straightforward anyway. If libadblockplus is built standalone, it should happen
in libadblockplus.gyp. If it is built as part of another project however, e.g.
adblockplusie, we shouldn't update dependencies twice.

But I guess it makes sense to do that in a separate patch and stick to manually
calling the script for now.

http://codereview.adblockplus.org/5519479341580288/diff/5757334940811264/lib/...
File lib/prefs.js (right):

http://codereview.adblockplus.org/5519479341580288/diff/5757334940811264/lib/...
lib/prefs.js:71: }
On 2015/04/30 08:30:24, Wladimir Palant wrote:
> enumerable: true please, otherwise debugging will be no fun.

Done.

[Wladimir Palant, 2015-04-30 18:47:04]

I guess you want to make ensure_dependencies.py run by default now?

[Felix Dahlke, 2015-04-30 20:24:55]

On 2015/04/30 18:47:04, Wladimir Palant wrote:
> I guess you want to make ensure_dependencies.py run by default now?

Yup, looking into that. As I said, not entirely straightforward here...

[Felix Dahlke, 2015-05-29 07:15:51]

On 2015/04/30 20:24:55, Felix H. Dahlke wrote:
> On 2015/04/30 18:47:04, Wladimir Palant wrote:
> > I guess you want to make ensure_dependencies.py run by default now?
> 
> Yup, looking into that. As I said, not entirely straightforward here...

Done, there's a new patch set up that invokes ensure_dependencies.py before the
build starts.

Where libadblockplus is used as a library it will be run twice, but that
shouldn't be a problem I suppose.

[Felix Dahlke, 2015-05-29 07:15:59]

http://codereview.adblockplus.org/5519479341580288/diff/5700735861784576/liba...
File libadblockplus.gyp (right):

http://codereview.adblockplus.org/5519479341580288/diff/5700735861784576/liba...
libadblockplus.gyp:25: 'outputs': ['ensure_dependencies_phony_output'],
This is kind of ugly, but the best way I could find - gyp doesn't seem to
support phony targets/actions.

[Wladimir Palant, 2015-05-29 19:24:33]

LGTM

[René Jeschke, 2015-05-29 19:34:39]

LGTM