Issue 5556 - Update to use libadblockplus revision hg:566f64c8a2a8

libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/BaseJsEngineTest.java

Index: libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/BaseJsEngineTest.java
diff --git a/libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/BaseJsTest.java b/libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/BaseJsEngineTest.java
similarity index 88%
rename from libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/BaseJsTest.java
rename to libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/BaseJsEngineTest.java
index fc988be5c4d82ac6816b86909cb9f0f1f0ff01bb..2f512c2ea5a77ddec4fac9aa8fa7d180acbad844 100644
--- a/libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/BaseJsTest.java
+++ b/libadblockplus-android-tests/src/org/adblockplus/libadblockplus/tests/BaseJsEngineTest.java
@@ -27,7 +27,7 @@ import org.adblockplus.libadblockplus.WebRequest;
 import android.content.Context;
 import android.test.InstrumentationTestCase;
 
-public abstract class BaseJsTest extends InstrumentationTestCase
+public abstract class BaseJsEngineTest extends InstrumentationTestCase
 {
   protected JsEngine jsEngine;
 
@@ -40,6 +40,16 @@ public abstract class BaseJsTest extends InstrumentationTestCase
         getContext().getFilesDir().getAbsolutePath());
   }
 
+  @Override
+  protected void tearDown() throws Exception
+  {
+    if (jsEngine != null)

[anton, 2017/09/06 06:33:47]

original GC approach assumes that when Java instance if being GCed dispose
wrapper disposes related C++ instance automatically. So C++ instance is disposed
after Java instance is GCed.

Why is explicit GCing introduced here?

[sergei, 2017/09/07 10:33:31]

I think that manual disposing of JsEngine and FilterEngine in tearDown is a good
thing because of several reasons:
- we know the exact time when they are disposed, it means that if it crashes
during disposing or a bit later after the test, because some async operation has
just finished, we will know the exact test which led to the crash, not later
when it's already another test. In addition it can be also helpful for all sorts
of extra profiling, e.g. to see the memory consumption by a particular test (one
can easily detect a memory leakage) or to see the time needed by a test.
- although each test has its own instance of v8::Isolate V8 still shares some
internals between them, so for better tests isolation it would be better to free
all resources when a test finishes.
- ABP consumes some reasonable amount of memory, so theoretically if GC is
slightly slow then we can accumulate too much used memory and run out of memory.
It's actually not so theoretical, we have reached that point with ~100 tests in
libadblockplus on 32 bit paltform when we had infinite async operations (e.g. in
WebRequest) preventing JsEngine from being destroyed.

[anton, 2017/09/07 11:51:13]

Well, it seems to be more C++ than Java approach it can bring benefits you've
mentioned.

+    {
+      jsEngine.dispose();
+      jsEngine = null;
+    }
+  }
+
   // If the method returns null then a default implementation of the Log System
   // provided by libadblockplus is used.
   protected LogSystem createLogSystem()