Make JsEngine::Evaluate() return a wrapper for v8::Value to accessdifferent variable types easily

Make JsEngine::Evaluate() return a wrapper for v8::Value to access
different variable types easily

[Wladimir Palant, 2013-04-13 20:44:21]

In order to make FilterEngine work with our JS code I had to extend JsValue API
quite significantly - both changes are in the same patch now, it was simply too
complicated to untangle them. There are a few TODOs left and JsObject class
needs to go once we get rid of the stubs - but otherwise this should be mostly
final and all tests are passed.

http://codereview.adblockplus.org/10213003/diff/1/include/AdblockPlus/JsValue.h
File include/AdblockPlus/JsValue.h (right):

http://codereview.adblockplus.org/10213003/diff/1/include/AdblockPlus/JsValue...
include/AdblockPlus/JsValue.h:11: typedef std::tr1::shared_ptr<JsValue>
JsValuePtr;
I only realized now that I didn't include tr1/memory here, not sure why it still
worked.

[Felix Dahlke, 2013-04-16 15:14:33]

http://codereview.adblockplus.org/10213003/diff/6001/convert_js.py
File convert_js.py (right):

http://codereview.adblockplus.org/10213003/diff/6001/convert_js.py#newcode32
convert_js.py:32: for file in verbatimAfter:
Given how this is the same code as above, I think a function would make sense.

http://codereview.adblockplus.org/10213003/diff/6001/include/AdblockPlus/JsEn...
File include/AdblockPlus/JsEngine.h (right):

http://codereview.adblockplus.org/10213003/diff/6001/include/AdblockPlus/JsEn...
include/AdblockPlus/JsEngine.h:43: return NewValue((int64_t)val);
I'd prefer static_cast here, C-style casts are less safe.

http://codereview.adblockplus.org/10213003/diff/6001/include/AdblockPlus/JsVa...
File include/AdblockPlus/JsValue.h (right):

http://codereview.adblockplus.org/10213003/diff/6001/include/AdblockPlus/JsVa...
include/AdblockPlus/JsValue.h:49: SetProperty(name, (int64_t)val);
As above, I'd prefer static_cast.

http://codereview.adblockplus.org/10213003/diff/6001/shell/src/FiltersCommand...
File shell/src/FiltersCommand.cpp (right):

http://codereview.adblockplus.org/10213003/diff/6001/shell/src/FiltersCommand...
shell/src/FiltersCommand.cpp:16: switch ((*it)->GetProperty("type",
(int64_t)-1))
This shouldn't be necessary, since there's a GetProperty that accepts an int,
right?

http://codereview.adblockplus.org/10213003/diff/6001/src/FilterEngine.cpp
File src/FilterEngine.cpp (right):

http://codereview.adblockplus.org/10213003/diff/6001/src/FilterEngine.cpp#new...
src/FilterEngine.cpp:346: const std::vector<SubscriptionPtr>
FilterEngine::GetListedSubscriptions() const
I guess we don't want to declare the return value const if it's a copy.

[Wladimir Palant, 2013-04-17 07:59:39]

I addressed the new review comments and pushed already to prevent further
merging issues.

[Felix Dahlke, 2013-04-17 08:03:00]

LGTM