Issue 5183 - Provide async interface for FileSystem

include/AdblockPlus/JsEngine.h

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 39 matching lines @@
    * Shared smart pointer to a `JsEngine` instance.
    */
   typedef std::shared_ptr<JsEngine> JsEnginePtr;
 
   /**
    * A factory to construct DefaultTimer.
    */
   TimerPtr CreateDefaultTimer();
 
   /**
+   * A factory to construct DefaultFileSystem.
+   */
+  FileSystemPtr CreateDefaultFileSystem();
+
+  /**
    * A factory to construct DefaultWebRequest.
    */
   WebRequestPtr CreateDefaultWebRequest();
 
   /**
    * Scope based isolate manager. Creates a new isolate instance on
    * constructing and disposes it on destructing.
    */
   class ScopedV8Isolate
   {
@@ skipping 43 matching lines @@
     class JsWeakValuesID
     {
       friend class JsEngine;
       JsWeakValuesLists::const_iterator iterator;
     };
 
     /**
      * Creates a new JavaScript engine instance.
      * @param appInfo Information about the app.
      * @param timer Implementation of timer.
+     * @param fileSystem Implementation of filesystem.
      * @param webRequest Implementation of web request.
      * @param isolate v8::Isolate wrapper. This parameter should be considered
      *        as a temporary hack for tests, it will go away. Issue #3593.
      * @return New `JsEngine` instance.
      */
     static JsEnginePtr New(const AppInfo& appInfo = AppInfo(),
       TimerPtr timer = CreateDefaultTimer(),
+      FileSystemPtr fileSystem = CreateDefaultFileSystem(),
       WebRequestPtr webRequest = CreateDefaultWebRequest());
 
     /**
      * Registers the callback function for an event.
      * @param eventName Event name. Note that this can be any string - it's a
      *        general purpose event handling mechanism.
      * @param callback Event callback function.
      */
     void SetEventCallback(const std::string& eventName, const EventCallback& callback);
 
@@ skipping 111 matching lines @@
 
     /**
      * Converts v8 arguments to `JsValue` objects.
      * @param arguments `v8::Arguments` object containing the arguments to
      *        convert.
      * @return List of arguments converted to `const JsValue` objects.
      */
     JsValueList ConvertArguments(const v8::Arguments& arguments);
 
     /**
-     * @see `SetFileSystem()`.
+     * @return The asynchronous IFileSystem implementation.
      */
     FileSystemPtr GetFileSystem() const;

[sergei, 2017/06/16 15:05:54]

I think we can remove the getter but still have the setter which accepts sync
`FileSystem`. This backward compatibility is important to ease the update in
other projects and also in tests (though with the proposed changes in
LazyFileSystem it's not needed). For instance, we often need to quickly update
libadblockplus in libadblockplus-android but have no time to change and test
FileSystem. So, to not introduce a delay one could update FileSystem in
libadblockplus-android later when there is no pressure.

[hub, 2017/06/16 21:52:54]

I need this accessor because it is use in FileSystemJsObject. But I should
possibly change it.

And I'll restore the setter for the Sync implementation.

 
     /**
-     * Sets the `FileSystem` implementation used for all file I/O.
-     * Setting this is optional, the engine will use a `DefaultFileSystem`
-     * instance by default, which might be sufficient.
-     * @param The `FileSystem` instance to use.
-     */
-    void SetFileSystem(const FileSystemPtr& val);
-
-    /**
      * Sets the `WebRequest` implementation used for XMLHttpRequests.
      * Setting this is optional, the engine will use a `DefaultWebRequest`
      * instance by default, which might be sufficient.
      * @param The `WebRequest` instance to use.
      */
     void SetWebRequest(const WebRequestSharedPtr& val);
 
     /**
      * @see `SetLogSystem()`.
      */
@@ skipping 24 matching lines @@
     }
 
     /**
      * Notifies JS engine about critically low memory what should cause a
      * garbage collection.
      */
     void NotifyLowMemory();
   private:
     void CallTimerTask(const JsWeakValuesID& timerParamsID);
 
-    explicit JsEngine(TimerPtr timer, WebRequestPtr webRequest);
+    explicit JsEngine(TimerPtr timer, FileSystemPtr fileSystem, WebRequestPtr webRequest);
 
     JsValue GetGlobalObject();
 
     /// Isolate must be disposed only after disposing of all objects which are
     /// using it.
     ScopedV8Isolate isolate;
 
     FileSystemPtr fileSystem;
     LogSystemPtr logSystem;
     std::unique_ptr<v8::Persistent<v8::Context>> context;
     EventMap eventCallbacks;
     std::mutex eventCallbacksMutex;
     JsWeakValuesLists jsWeakValuesLists;
     std::mutex jsWeakValuesListsMutex;
     TimerPtr timer;
     WebRequestPtr webRequest;
     WebRequestSharedPtr webRequestLegacy;
   };
 }
 
 #endif