Issue 3337 - Update adblockplus reference in libadblockplus to revision hg:813682c7161e

Changes in the code are mainly introduced by #2738.

Make added JavaScript file 'cssRules.js' to be processed by `convert_js`.

Note about type for content type mask (signed 32 bit integer, int32_t): it's not unsigned and its size is 32 bit because of peculiarities of bitmasks in JavaScipt https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Operators/Bitwise_Operators.

[Oleksandr, 2016-02-04 04:14:11]

LGTM

[Eric, 2016-02-09 15:43:04]

https://codereview.adblockplus.org/29335462/diff/29335463/include/AdblockPlus...
File include/AdblockPlus/FilterEngine.h (right):

https://codereview.adblockplus.org/29335462/diff/29335463/include/AdblockPlus...
include/AdblockPlus/FilterEngine.h:294: int32_t contentTypeMask,
It would seem better to use uint32_t here, given that this type has turned into
something of a bit field.

Probably deserves a 'typedef' too, since the carrier type doesn't reflect the
semantics of a content type.

Also, calling them all "...Mask" is putting the type into the variable name.
While not against the explicit directives of our style guide, it's certainly
against the spirit of it. Better to put the type into the type name with a
'typedef'.

https://codereview.adblockplus.org/29335462/diff/29335463/test/FilterEngine.cpp
File test/FilterEngine.cpp (right):

https://codereview.adblockplus.org/29335462/diff/29335463/test/FilterEngine.c...
test/FilterEngine.cpp:283: TEST_F(FilterEngineTest, MatchesWithContentTypeMask)
There ten (or so) unit tests here combined into one. Better to split them out
individually. Easier to read results if one fails.

[sergei, 2016-02-09 17:15:47]

https://codereview.adblockplus.org/29335462/diff/29335463/include/AdblockPlus...
File include/AdblockPlus/FilterEngine.h (right):

https://codereview.adblockplus.org/29335462/diff/29335463/include/AdblockPlus...
include/AdblockPlus/FilterEngine.h:294: int32_t contentTypeMask,
On 2016/02/09 15:43:03, Eric wrote:
> It would seem better to use uint32_t here, given that this type has turned
into
> something of a bit field.
Yes, however it will be converted into signed integer by JavaScript, so it looks
fair to use signed integer here as well.
> 
> Probably deserves a 'typedef' too, since the carrier type doesn't reflect the
> semantics of a content type.
> 
> Also, calling them all "...Mask" is putting the type into the variable name.
> While not against the explicit directives of our style guide, it's certainly
> against the spirit of it. Better to put the type into the type name with a
> 'typedef'.
I have added the typedef.

https://codereview.adblockplus.org/29335462/diff/29335463/test/FilterEngine.cpp
File test/FilterEngine.cpp (right):

https://codereview.adblockplus.org/29335462/diff/29335463/test/FilterEngine.c...
test/FilterEngine.cpp:283: TEST_F(FilterEngineTest, MatchesWithContentTypeMask)
On 2016/02/09 15:43:03, Eric wrote:
> There ten (or so) unit tests here combined into one. Better to split them out
> individually. Easier to read results if one fails.

I thought about it, it definitely makes sense to create a set of individual
tests, however, having looked around, I'm not sure that it would be easier to
read them because of the following reasons.
It's convenient to have the same initialization for all of those tests because
we can see the behavior picture through comparison of the tests in that group.
The best way to do it is to use test fixture, however in that case the asserts
will be split by several lines with code and one has to pay attention to the
test case name (according to gtest framework terminology), the latter actually
reduces the readability.
Other tests around actually prove that it's not a bad way to organize such
tests.

So, I would like to keep the current version until it really becomes
inconvenient. In that case I will be happy to sort them out individually and may
be even move some of the tests from that file into a separate files.

[sergei, 2016-02-18 10:34:47]

René or Eric, could you please review it?

[sergei, 2016-04-19 17:44:04]

FYI: I'm going to push this changes tomorrow.