Issue 4944 - Make creation of FilterEngine asynchronous.

include/AdblockPlus/FilterEngine.h

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 11 matching lines @@
 #include <map>
 #include <string>
 #include <vector>
 #include <AdblockPlus/JsEngine.h>
 #include <AdblockPlus/JsValue.h>
 #include <AdblockPlus/Notification.h>
 
 namespace AdblockPlus
 {
   class FilterEngine;
+  typedef std::shared_ptr<FilterEngine> FilterEnginePtr;
 
   /**
    * Wrapper for an Adblock Plus filter object.
    * There are no accessors for most
    * [filter properties](https://adblockplus.org/jsdoc/adblockpluscore/Filter.html),
    * use `GetProperty()` to retrieve them by name.
    */
   class Filter : public JsValue,
                  public std::enable_shared_from_this<Filter>
   {
@@ skipping 164 matching lines @@
      */
     typedef std::map<std::string, AdblockPlus::JsValuePtr> Prefs;
 
     /**
      * Callback type invoked when a new notification should be shown.
      * The parameter is the Notification object to be shown.
      */
     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`
      * instance.
      */
     JsEnginePtr GetJsEngine() const { return jsEngine; }
 
     /**
      * Checks if this is the first run of the application.
      * @return `true` if the application is running for the first time.
@@ skipping 200 matching lines @@
     /**
      * Retrieves the string representation of the supplied `ContentType`.
      * @param contentType `ContentType` value.
      * @return The string representation of `contentType`.
      * @throw `std::invalid_argument`, if an invalid `contentType` was supplied.
      */
     static std::string ContentTypeToString(ContentType contentType);
 
   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;
     void UpdateAvailable(UpdateAvailableCallback callback, JsValueList& params);
     void UpdateCheckDone(const std::string& eventName,
                          UpdateCheckDoneCallback callback, JsValueList& params);
     void FilterChanged(FilterChangeCallback callback, JsValueList& params);
     void ShowNotification(const ShowNotificationCallback& callback,
       const JsValueList& params);
     FilterPtr GetWhitelistingFilter(const std::string& url,
       ContentTypeMask contentTypeMask, const std::string& documentUrl) const;
     FilterPtr GetWhitelistingFilter(const std::string& url,
       ContentTypeMask contentTypeMask,
       const std::vector<std::string>& documentUrls) const;
   };
 }
 
 #endif