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 198 matching lines @@
 
     /**
      * 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;
 
     /**
      * 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.

+    struct CreationParameters
     {
       /**
        * `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 onCreated A callback which is called when FilterEngine is ready
      *        for use.
      * @param parameters optional creation parameters.
      */
     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());
+      const CreationParameters& parameters = CreationParameters());
 
     /**
      * Synchronous interface to construct FilterEngine. For details see
      * asynchronous version CreateAsync.
      */
     static FilterEnginePtr Create(const JsEnginePtr& jsEngine,
-      const CreateParameters& params = CreateParameters());
+      const CreationParameters& params = CreationParameters());
 
     /**
      * 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 224 matching lines @@
       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