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

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

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 package org.adblockplus.libadblockplus.tests;
 
 import org.adblockplus.libadblockplus.AppInfo;
 import org.adblockplus.libadblockplus.JsEngine;
 import org.adblockplus.libadblockplus.LazyLogSystem;
 import org.adblockplus.libadblockplus.LogSystem;
 import org.adblockplus.libadblockplus.ThrowingWebRequest;
 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;
 
   @Override
   protected void setUp() throws Exception
   {
     super.setUp();
 
     jsEngine = new JsEngine(AppInfo.builder().build(), createLogSystem(), createWebRequest(),
         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()
   {
     return null;
   }
 
   protected WebRequest createWebRequest()
   {
     return new ThrowingWebRequest();
   }
 
   protected Context getContext()
   {
     return getInstrumentation().getTargetContext();
   }
 }