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 16 matching lines @@
 #include <mutex>
 #include <AdblockPlus/AppInfo.h>
 #include <AdblockPlus/LogSystem.h>
 #include <AdblockPlus/FileSystem.h>
 #include <AdblockPlus/JsValue.h>
 #include <AdblockPlus/WebRequest.h>
 #include <AdblockPlus/ITimer.h>
 
 namespace v8
 {
-  class Arguments;
   class Isolate;
   class Value;
   class Context;
-  template<class T> class Handle;
-  typedef Handle<Value>(*InvocationCallback)(const Arguments &args);
+  template<typename T> class FunctionCallbackInfo;
+  typedef void(*FunctionCallback)(const FunctionCallbackInfo<v8::Value>& info);
 }
 
 namespace AdblockPlus
 {
   class JsEngine;
 
   /**
    * Shared smart pointer to a `JsEngine` instance.
    */
   typedef std::shared_ptr<JsEngine> JsEnginePtr;
@@ skipping 37 matching lines @@
    * JavaScript engine used by `FilterEngine`, wraps v8.
    */
   class JsEngine : public std::enable_shared_from_this<JsEngine>
   {
     friend class JsValue;
     friend class JsContext;
 
     struct JsWeakValuesList
     {
       ~JsWeakValuesList();
-      std::vector<std::unique_ptr<v8::Persistent<v8::Value>>> values;
+      std::vector<v8::Global<v8::Value>> values;
     };
     typedef std::list<JsWeakValuesList> JsWeakValuesLists;
   public:
     /**
      * Event callback function.
      */
     typedef std::function<void(JsValueList&& params)> EventCallback;
 
     /**
      * Maps events to callback functions.
@@ skipping 88 matching lines @@
 
     /**
      * Creates a new JavaScript object.
      * @return New `JsValue` instance.
      */
     JsValue NewObject();
 
     /**
      * Creates a JavaScript function that invokes a C++ callback.
      * @param callback C++ callback to invoke. The callback receives a
-     *        `v8::Arguments` object and can use `FromArguments()` to retrieve
+     *        `v8::FunctionCallbackInfo` object and can use `FromArguments()` to retrieve
      *        the current `JsEngine`.
      * @return New `JsValue` instance.
      */
-    JsValue NewCallback(const v8::InvocationCallback& callback);
-
-    /**
-     * Returns a `JsEngine` instance contained in a `v8::Arguments` object.
+    JsValue NewCallback(const v8::FunctionCallback& callback);
+
+    /**
+     * Returns a `JsEngine` instance contained in a `v8::FunctionCallbackInfo` object.
      * Use this in callbacks created via `NewCallback()` to retrieve the current
      * `JsEngine`.
-     * @param arguments `v8::Arguments` object containing the `JsEngine`
+     * @param arguments `v8::FunctionCallbackInfo` object containing the `JsEngine`
      *        instance.
-     * @return `JsEngine` instance from `v8::Arguments`.
-     */
-    static JsEnginePtr FromArguments(const v8::Arguments& arguments);
+     * @return `JsEngine` instance from `v8::FunctionCallbackInfo`.
+     */
+    static JsEnginePtr FromArguments(const v8::FunctionCallbackInfo<v8::Value>& arguments);
 
     /**
      * Stores `JsValue`s in a way they don't keep a strong reference to
      * `JsEngine` and which are destroyed when `JsEngine` is destroyed. These
      * methods should be used when one needs to carry a JsValue in a callback
      * directly or indirectly passed to `JsEngine`.
      * The method is thread-safe.
      * @param `JsValueList` to store.
      * @return `JsWeakValuesID` of stored values which allows to restore them
      * later.
      */
     JsWeakValuesID StoreJsValues(const JsValueList& values);
 
     /**
      * Extracts and removes from `JsEngine` earlier stored `JsValue`s.
      * The method is thread-safe.
      * @param id `JsWeakValuesID` of values.
      * @return `JsValueList` of stored values.
      */
     JsValueList TakeJsValues(const JsWeakValuesID& id);
 
     /*
      * Private functionality required to implement timers.
-     * @param arguments `v8::Arguments` is the arguments received in C++
+     * @param arguments `v8::FunctionCallbackInfo` is the arguments received in C++
      * callback associated for global setTimeout method.
      */
-    static void ScheduleTimer(const v8::Arguments& arguments);
+    static void ScheduleTimer(const v8::FunctionCallbackInfo<v8::Value>& arguments);
 
     /*
      * Private functionality required to implement web requests.
-     * @param arguments `v8::Arguments` is the arguments received in C++
+     * @param arguments `v8::FunctionCallbackInfo` is the arguments received in C++
      * callback associated for global GET method.
      */
-    static void ScheduleWebRequest(const v8::Arguments& arguments);
+    static void ScheduleWebRequest(const v8::FunctionCallbackInfo<v8::Value>& arguments);
 
     /**
      * Converts v8 arguments to `JsValue` objects.
-     * @param arguments `v8::Arguments` object containing the arguments to
+     * @param arguments `v8::FunctionCallbackInfo` object containing the arguments to
      *        convert.
      * @return List of arguments converted to `const JsValue` objects.
      */
-    JsValueList ConvertArguments(const v8::Arguments& arguments);
-
-    /**
+    JsValueList ConvertArguments(const v8::FunctionCallbackInfo<v8::Value>& arguments);
+
+    /**
+     * Private functionality.
      * @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.

+    FileSystemPtr GetAsyncFileSystem() const;
+
+    /**
+     * Sets the synchronous `FileSystem` implementation used for all
+     * file I/O. Setting this is optional, the engine will use the
+     * implementation created by `CreateDefaultFileSystem()` by
+     * default, which might be sufficient.
+     * @param The `FileSystem` instance to use.
+     */
+    void SetFileSystem(const FileSystemSyncPtr& 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);
 
     /**
@@ skipping 36 matching lines @@
     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;
+    std::unique_ptr<v8::Global<v8::Context>> context;
     EventMap eventCallbacks;
     std::mutex eventCallbacksMutex;
     JsWeakValuesLists jsWeakValuesLists;
     std::mutex jsWeakValuesListsMutex;
     TimerPtr timer;
     WebRequestPtr webRequest;
     WebRequestSharedPtr webRequestLegacy;
   };
 }
 
 #endif