Fix memory leaks in libadblockplus

I have found 2 memory leaks.
1. Persistent handles were not being disposed (unlike the Local ones, they have to be disposed manually)
2. The garbage collection did not seem to be running. The Matches() calls produce the most garbage. I have put the GC in the GetElementHidingSelectors though, since that runs only once per page, and we don't want to be overly enthusiastic with the cleanups.

With these changes the memory usage stays more or less the same for me.

[Wladimir Palant, 2013-10-08 08:12:19]

LGTM on fixing V8ValueHolder if you avoid code duplication as outlined below.
Triggering garbage collection however definitely isn't something for this
release.

http://codereview.adblockplus.org/12647002/diff/1/include/AdblockPlus/V8Value...
File include/AdblockPlus/V8ValueHolder.h (right):

http://codereview.adblockplus.org/12647002/diff/1/include/AdblockPlus/V8Value...
include/AdblockPlus/V8ValueHolder.h:58: }
Better avoid the code duplication here:

reset(0, new v8::Persistent<T>());

http://codereview.adblockplus.org/12647002/diff/1/src/FilterEngine.cpp
File src/FilterEngine.cpp (right):

http://codereview.adblockplus.org/12647002/diff/1/src/FilterEngine.cpp#newcod...
src/FilterEngine.cpp:236: jsEngine->Gc();
This is a really bad idea, it will cause unnecessary delays. Embedders are only
supposed to call V8::IdleNotification() when they are actually idle.

As it is now, the Gc() method is only useful for testing. We can turn it into a
proper wrapper for V8::IdleNotification() - only calling V8::IdleNotification()
once and returning its return value, but with an additional lock to make sure no
JS code is running. However, we would actually need to call that in the
application using libadblockplus (in the message loop or its equivalent) because
otherwise we cannot know when that application is idle. Not quite easy but worth
looking into.

[Felix Dahlke, 2013-10-10 08:36:26]

LGTM with Wladimir's issue addressed.

http://codereview.adblockplus.org/12647002/diff/1/src/FilterEngine.cpp
File src/FilterEngine.cpp (right):

http://codereview.adblockplus.org/12647002/diff/1/src/FilterEngine.cpp#newcod...
src/FilterEngine.cpp:236: jsEngine->Gc();
On 2013/10/08 08:12:19, Wladimir Palant wrote:
> This is a really bad idea, it will cause unnecessary delays. Embedders are
only
> supposed to call V8::IdleNotification() when they are actually idle.
> 
> As it is now, the Gc() method is only useful for testing. We can turn it into
a
> proper wrapper for V8::IdleNotification() - only calling
V8::IdleNotification()
> once and returning its return value, but with an additional lock to make sure
no
> JS code is running. However, we would actually need to call that in the
> application using libadblockplus (in the message loop or its equivalent)
because
> otherwise we cannot know when that application is idle. Not quite easy but
worth
> looking into.

I don't think we should look into this at this point. High memory usage is what
we get for using garbage collection to begin with, it's an issue we have on each
and every platform. The only real way to fix this is to put energy into avoiding
GC in the core code, which is what ChimeraScript would address.

I doubt that, once we have that, using v8::IdleNotification properly will gain
us much.