Issue 6307 - Introduce external engine created callback

libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.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
@@ skipping 18 matching lines @@
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * Provides single instance of AdblockEngine shared between registered clients
  */
 public class SingleInstanceEngineProvider implements AdblockEngineProvider
 {
   private static final String TAG = Utils.getTag(SingleInstanceEngineProvider.class);
 
+  public interface EngineCreatedListener
+  {
+    void onAdblockEngineCreated(AdblockEngine engine);
+  }
+
+  public interface EngineDisposedListener
+  {
+    void onAdblockEngineDisposed();
+  }
+
   private Context context;
   private String basePath;
   private boolean developmentBuild;
   private String preloadedPreferenceName;
   private Map<String, Integer> urlToResourceIdMap;
   private AdblockEngine engine;
   private CountDownLatch engineCreated;
   private Long v8IsolateProviderPtr;
-  private List<Runnable> engineCreatedCallbacks = new LinkedList<Runnable>();
-  private List<Runnable> engineDisposedCallbacks = new LinkedList<Runnable>();
+  private List<EngineCreatedListener> engineCreatedListeners =
+    new LinkedList<EngineCreatedListener>();
+  private List<EngineDisposedListener> engineDisposedListeners =
+    new LinkedList<EngineDisposedListener>();

[diegocarloslima, 2018/01/26 13:04:28]

Just one minor thing, the lists could be final

 
   /*
     Simple ARC management for AdblockEngine
     Use `retain` and `release`
    */
 
   private AtomicInteger referenceCounter = new AtomicInteger(0);
 
   /**
    * Init with context
@@ skipping 25 matching lines @@
     this.urlToResourceIdMap = urlToResourceIdMap;
     return this;
   }
 
   public SingleInstanceEngineProvider useV8IsolateProvider(long ptr)
   {
     this.v8IsolateProviderPtr = ptr;
     return this;
   }
 
-  public SingleInstanceEngineProvider addEngineCreatedCallback(Runnable callback)
-  {
-    this.engineCreatedCallbacks.add(callback);
-    return this;
-  }
-
-  public void removeEngineCreatedCallback(Runnable callback)

[anton, 2018/01/24 12:51:08]

"remove.." methods are not used anywhere but could be used.
also we could add "clear.." methods too for consistency.
What do you think?

[sergei, 2018/01/24 13:33:02]

I would say if someone wants to remove or clear then just create a new instance
of SingleInstanceEngineProvider and abandon the present one.

[jens, 2018/01/26 08:15:22]

Even if Sergejs idea is also fine I would prefer to be consistent and add a
clear method too.

[anton, 2018/01/26 08:22:58]

Acknowledged.

-  {
-    this.engineCreatedCallbacks.remove(callback);
-  }
-
-  public SingleInstanceEngineProvider addEngineDisposedCallback(Runnable callback)
-  {
-    this.engineDisposedCallbacks.add(callback);
-    return this;
-  }
-
-  public void removeEngineDisposedCallback(Runnable callback)
-  {
-    this.engineDisposedCallbacks.remove(callback);
+  public SingleInstanceEngineProvider addEngineCreatedListener(EngineCreatedListener listener)
+  {
+    this.engineCreatedListeners.add(listener);
+    return this;
+  }
+
+  public void removeEngineCreatedListener(EngineCreatedListener listener)
+  {
+    this.engineCreatedListeners.remove(listener);
+  }
+
+  public void clearEngineCreatedListeners()
+  {
+    this.engineCreatedListeners.clear();
+  }
+
+  public SingleInstanceEngineProvider addEngineDisposedListener(EngineDisposedListener listener)
+  {
+    this.engineDisposedListeners.add(listener);
+    return this;
+  }
+
+  public void removeEngineDisposedListener(EngineDisposedListener listener)
+  {
+    this.engineDisposedListeners.remove(listener);
+  }
+
+  public void clearEngineDisposedListeners()
+  {
+    this.engineDisposedListeners.clear();
   }
 
   private void createAdblock()
   {
     ConnectivityManager connectivityManager =
       (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
     IsAllowedConnectionCallback isAllowedConnectionCallback =
       new IsAllowedConnectionCallbackImpl(connectivityManager);
 
     Log.d(TAG, "Creating adblock engine ...");
@@ skipping 20 matching lines @@
         context,
         urlToResourceIdMap,
         new AndroidWebRequestResourceWrapper.SharedPrefsStorage(preloadedSubscriptionsPrefs));
     }
 
     engine = builder.build();
 
     Log.d(TAG, "AdblockHelper engine created");
 
     // sometimes we need to init AdblockEngine instance, eg. set user settings
-    for (Runnable callback : engineCreatedCallbacks)
-    {
-      callback.run();
+    for (EngineCreatedListener listener : engineCreatedListeners)
+    {
+      listener.onAdblockEngineCreated(engine);
     }
   }
 
   @Override
   public synchronized boolean retain(boolean asynchronous)
   {
     boolean firstInstance = false;
 
     if (referenceCounter.getAndIncrement() == 0)
     {
@@ skipping 79 matching lines @@
 
   private void disposeAdblock()
   {
     Log.w(TAG, "Disposing adblock engine");
 
     engine.dispose();
     engine = null;
 
     // sometimes we need to deinit something after AdblockEngine instance disposed
     // eg. release user settings
-    for (Runnable callback : engineDisposedCallbacks)
-    {
-      callback.run();
+    for (EngineDisposedListener listener : engineDisposedListeners)
+    {
+      listener.onAdblockEngineDisposed();
     }
   }
 
   @Override
   public int getCounter()
   {
     return referenceCounter.get();
   }
 }