Issue 4944 - Make creation of FilterEngine asynchronous.

include/AdblockPlus/FilterEngine.h

Index: include/AdblockPlus/FilterEngine.h
diff --git a/include/AdblockPlus/FilterEngine.h b/include/AdblockPlus/FilterEngine.h
index 94b376eb8caadee35db2d4b96893ee5b0702b16d..dcf15a26604b4ca6d5f1484881b74303c0348478 100644
--- a/include/AdblockPlus/FilterEngine.h
+++ b/include/AdblockPlus/FilterEngine.h
@@ -29,6 +29,7 @@
 namespace AdblockPlus
 {
   class FilterEngine;
+  typedef std::shared_ptr<FilterEngine> FilterEnginePtr;
 
   /**
    * Wrapper for an Adblock Plus filter object.
@@ -213,15 +214,40 @@ namespace AdblockPlus
     typedef std::function<void(const NotificationPtr&)> ShowNotificationCallback;
 
     /**
-     * Constructor.
+     * FilterEngine creation parameters.
+     */
+    struct CreateParameters

[Oleksandr, 2017/03/16 15:05:25]

Nit: How about CreationParameters. One might assume this is a function with a
name as you have it.

[sergei, 2017/03/16 15:50:03]

Agree, done.

+    {
+      /**
+       * `AdblockPlus::FilterEngine::Prefs` name - value list of preconfigured
+       * prefs.
+       */
+      Prefs preconfiguredPrefs;
+    };
+
+    /**
+    * Callback type invoked when FilterEngine is created.
+    */
+    typedef std::function<void(const FilterEnginePtr&)> OnCreatedCallback;
+
+    /**
+     * Asynchronously constructs FilterEngine.
      * @param jsEngine `JsEngine` instance used to run JavaScript code
      *        internally.
-     * @param preconfiguredPrefs `AdblockPlus::FilterEngine::Prefs`
-     *        name-value list of preconfigured prefs.
+     * @param onCreated A callback which is called when FilterEngine is ready
+     *        for use.
+     * @param parameters optional creation parameters.
      */
-    explicit FilterEngine(JsEnginePtr jsEngine, 
-        const Prefs& preconfiguredPrefs = Prefs()
-      );
+    static void CreateAsync(const JsEnginePtr& jsEngine,

[Eric, 2017/02/28 17:04:50]

(1) C++ has <future>.

(2) This is new code. This is not a rewrite of legacy code.

(3) An asynchronous factory should be presented in the standard C++ way, which
is with std::promise.

Among other deficiencies in a design pattern that uses a callback, there's no
good way to deal with a constructor exception. std::future handles that case
seamlessly.

I seem to remember there are some issues with <future> in VS 2012. I don't know
the details, nor do I know whether they'll cause a problem with this code. All
the more reason to concentrate on getting our tools updated, even if it means
delaying some desired features. The friction and drag from not having current
tools just keeps accumulating.

[sergei, 2017/02/28 22:18:27]

Sorry, but what is the aim of this message? We currently bound to MSVS2012 and
std::promise and std::future are completely broken here. Of course they should
be used but we cannot do it. JIC, I have not deliberately brought the
implementation of Sync which is aware about exceptions. I hope that anyway it
will go away in favor of std::promise and std::future after updating of v8 when
we can switch to toolset v140.

[Oleksandr, 2017/03/16 15:05:25]

It does not look like we'll be able to move to a newer Visual Studio in the
proposed time frame, unfortunately. So discussion about std::future doesn't make
much sense at this point.

+      const OnCreatedCallback& onCreated,
+      const CreateParameters& parameters = CreateParameters());
+
+    /**
+     * Synchronous interface to construct FilterEngine. For details see
+     * asynchronous version CreateAsync.
+     */
+    static FilterEnginePtr Create(const JsEnginePtr& jsEngine,
+      const CreateParameters& params = CreateParameters());
 
     /**
      * Retrieves the `JsEngine` instance associated with this `FilterEngine`
@@ -442,12 +468,12 @@ namespace AdblockPlus
 
   private:
     JsEnginePtr jsEngine;
-    bool initialized;
     bool firstRun;
     int updateCheckId;
     static const std::map<ContentType, std::string> contentTypes;
 
-    void InitDone(JsValueList& params);
+    explicit FilterEngine(const JsEnginePtr& jsEngine);
+
     FilterPtr CheckFilterMatch(const std::string& url,
                                ContentTypeMask contentTypeMask,
                                const std::string& documentUrl) const;