Issue #4692 - Move responsibility for engine reference from tasks to scheduler

src/FileSystemJsObject.cpp

Index: src/FileSystemJsObject.cpp
===================================================================
--- a/src/FileSystemJsObject.cpp
+++ b/src/FileSystemJsObject.cpp
@@ -33,24 +33,24 @@
 namespace
 {
   class ReadTask
+    : public TaskFunctionInterface
   {
     /**
      * Shared pointer keeps engine in existence for task thread.
      */
-    JsEnginePtr jsEngine;
+    JsEngineInternal* engine;
     std::string path;
     V8PersistentNG<v8::Function> callbackFunction;
 
   public:
     ReadTask(JsEngineInternal* engine, const std::string& path,
       V8PersistentNG<v8::Function> callbackFunction)
-      : jsEngine(engine->shared_from_this()), path(path),
+      : engine(engine), path(path),
       callbackFunction(callbackFunction)
     {}
 
-    void operator()()
+    void operator()() override
     {
-      JsEngineInternal* engine = ToInternal(jsEngine);
       std::string content;
       std::string error;
       // Read operation is long-lived. Do not lock engine during it.
@@ -79,11 +79,12 @@
   };
 
   class WriteTask
+    : public TaskFunctionInterface
   {
     /**
      * Shared pointer keeps engine in existence for task thread.
      */
-    JsEnginePtr jsEngine;
+    JsEngineInternal* engine;
     std::string path;
     std::string content;
     V8PersistentNG<v8::Function> callbackFunction;
@@ -91,13 +92,12 @@
     WriteTask(JsEngineInternal* engine,
       const std::string& path, const std::string& content,
       V8PersistentNG<v8::Function> callbackFunction)
-      : jsEngine(engine->shared_from_this()), path(path), content(content),
+      : engine(engine), path(path), content(content),
       callbackFunction(callbackFunction)
     {}
 
-    void operator()()
+    void operator()() override
     {
-      JsEngineInternal* engine = ToInternal(jsEngine);
       std::string error;
       // Write operation is long-lived. Do not lock engine during it.
       try
@@ -123,11 +123,12 @@
   };
 
   class MoveTask
+    : public TaskFunctionInterface
   {
     /**
      * Shared pointer keeps engine in existence for task thread.
      */
-    JsEnginePtr jsEngine;
+    JsEngineInternal* engine;
     std::string fromPath;
     std::string toPath;
     V8PersistentNG<v8::Function> callbackFunction;
@@ -135,13 +136,12 @@
   public:
     MoveTask(JsEngineInternal* engine, const std::string& fromPath, const std::string& toPath,
       V8PersistentNG<v8::Function> callbackFunction)
-      : jsEngine(engine->shared_from_this()), fromPath(fromPath), toPath(toPath), 
+      : engine(engine), fromPath(fromPath), toPath(toPath),
       callbackFunction(callbackFunction)
     {}
 
-    void operator()()
+    void operator()() override
     {
-      JsEngineInternal* engine(ToInternal(jsEngine));
       std::string error;
       try
       {
@@ -163,24 +163,24 @@
   };
 
   class RemoveTask
+    : public TaskFunctionInterface
   {
     /**
      * Shared pointer keeps engine in existence for task thread.
      */
-    JsEnginePtr jsEngine;
+    JsEngineInternal *engine;
     std::string path;
     V8PersistentNG<v8::Function> callbackFunction;
 
   public:
     RemoveTask(JsEngineInternal *engine, const std::string& path,
       V8PersistentNG<v8::Function> callbackFunction)
-      : jsEngine(engine->shared_from_this()), path(path),
+      : engine(engine), path(path),
       callbackFunction(callbackFunction)
     {}
 
-    void operator()()
+    void operator()() override
     {
-      JsEngineInternal* engine(ToInternal(jsEngine));
       std::string error;
       try
       {
@@ -202,24 +202,24 @@
   };
 
   class StatTask
+    : public TaskFunctionInterface
   {
     /**
      * Shared pointer keeps engine in existence for task thread.
      */
-    JsEnginePtr jsEngine;
+    JsEngineInternal *engine;
     std::string path;
     V8PersistentNG<v8::Function> callbackFunction;
 
   public:
     StatTask(JsEngineInternal* engine, const std::string& path,
       V8PersistentNG<v8::Function> callbackFunction)
-      : jsEngine(engine->shared_from_this()), path(path),
+      : engine(engine), path(path),
       callbackFunction(callbackFunction)
     {}
 
-    void operator()()
+    void operator()() override
     {
-      JsEngineInternal* engine(ToInternal(jsEngine));
       std::string error;
       FileSystem::StatResult statResult;
       try
@@ -251,7 +251,6 @@
 v8::Handle<v8::Value> ReadCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
-  std::shared_ptr<ReadTask> readTask;
   try
   {
     if (arguments.Length() != 2)
@@ -269,26 +268,23 @@
     {
       throw std::runtime_error("Second argument to _fileSystem.read must be a function");
     }
-    readTask = std::make_shared<ReadTask>(engine, firstArgument, 
+    ReadTask readTask(engine, firstArgument,
       V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[1])));
+    // Run the task
+    engine->ScheduleTask(std::move(readTask), ImmediateSingleUseThread);
+    return v8::Undefined();
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
-  // Run the task
-  engine->Schedule(AdblockPlus::MakeHeapFunction(readTask), AdblockPlus::ImmediateSingleUseThread);
-  return v8::Undefined();
 }
 
 v8::Handle<v8::Value> WriteCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
-  std::shared_ptr<WriteTask> writeTask;
   try
   {
-    v8::Isolate* isolate = arguments.GetIsolate();
-
     if (arguments.Length() != 3)
     {
       throw std::exception("_fileSystem.write requires 3 parameters");
@@ -310,22 +306,21 @@
     {
       throw std::runtime_error("Third argument to _fileSystem.write must be a function");
     }
-    writeTask = std::make_shared<WriteTask>(engine, firstArgument, secondArgument, 
+    // Run the task
+    WriteTask writeTask(engine, firstArgument, secondArgument,
       V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[2])));
+    engine->ScheduleTask(std::move(writeTask), ImmediateSingleUseThread);
+    return v8::Undefined();
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
-  engine->Schedule(AdblockPlus::MakeHeapFunction(writeTask), AdblockPlus::ImmediateSingleUseThread);
-  return v8::Undefined();
 }
 
 v8::Handle<v8::Value> MoveCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
-  std::shared_ptr<MoveTask> moveTask;
-  std::string firstArgument, secondArgument;
   try
   {
     if (arguments.Length() != 3)
@@ -333,6 +328,7 @@
       throw std::runtime_error("_fileSystem.move requires 3 parameters");
     }
     bool argumentIsString;
+    std::string firstArgument, secondArgument;
     std::tie(argumentIsString, firstArgument) = ConvertString(arguments[0]);
     if (!argumentIsString)
     {
@@ -347,23 +343,21 @@
     {
       throw std::runtime_error("Third argument to _fileSystem.move must be a function");
     }
+    // Run the task
+    MoveTask moveTask(engine, firstArgument, secondArgument,
+      V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[2])));
+    engine->ScheduleTask(std::move(moveTask), ImmediateSingleUseThread);
+    return v8::Undefined();
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
-  // Run the task
-  moveTask = std::make_shared<MoveTask>(engine, firstArgument, secondArgument, 
-    V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[2])));
-  engine->Schedule(AdblockPlus::MakeHeapFunction(moveTask), AdblockPlus::ImmediateSingleUseThread);
-  return v8::Undefined();
 }
 
 v8::Handle<v8::Value> RemoveCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
-  std::shared_ptr<RemoveTask> removeTask;
-  std::string firstArgument;
   try
   {
     if (arguments.Length() != 2)
@@ -371,6 +365,7 @@
       throw std::runtime_error("_fileSystem.remove requires 2 parameters");
     }
     bool argumentIsString;
+    std::string firstArgument;
     std::tie(argumentIsString, firstArgument) = ConvertString(arguments[0]);
     if (!argumentIsString)
     {
@@ -380,23 +375,21 @@
     {
       throw std::runtime_error("Second argument to _fileSystem.remove must be a function");
     }
+    // Run the task
+    RemoveTask removeTask(engine, firstArgument,
+      V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[1])));
+    engine->ScheduleTask(std::move(removeTask), ImmediateSingleUseThread);
+    return v8::Undefined();
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
-  // Run the task
-  removeTask = std::make_shared<RemoveTask>(engine, firstArgument,
-    V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[1])));
-  engine->Schedule(AdblockPlus::MakeHeapFunction(removeTask), AdblockPlus::ImmediateSingleUseThread);
-  return v8::Undefined();
 }
 
 v8::Handle<v8::Value> StatCallback(const v8::Arguments& arguments)
 {
   auto engine(JsEngineInternal::ExtractEngine(arguments));
-  std::shared_ptr<StatTask> statTask;
-  std::string firstArgument;
   try
   {
     if (arguments.Length() != 2)
@@ -404,6 +397,7 @@
       throw std::runtime_error("_fileSystem.stat requires 2 parameters");
     }
     bool argumentIsString;
+    std::string firstArgument;
     std::tie(argumentIsString, firstArgument) = ConvertString(arguments[0]);
     if (!argumentIsString)
     {
@@ -413,16 +407,16 @@
     {
       throw std::runtime_error("Second argument to _fileSystem.stat must be a function");
     }
+    // Run the task
+    StatTask statTask(engine, firstArgument,
+      V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[1])));
+    engine->ScheduleTask(std::move(statTask), ImmediateSingleUseThread);
+    return v8::Undefined();
   }
   catch (const std::exception& e)
   {
     return v8::ThrowException(engine->ToV8String(e.what()));
   }
-  // Run the task
-  statTask = std::make_shared<StatTask>(engine, firstArgument,
-    V8PersistentNG<v8::Function>(engine->GetIsolate(), v8::Local<v8::Function>::Cast(arguments[1])));
-  engine->Schedule(AdblockPlus::MakeHeapFunction(statTask), AdblockPlus::ImmediateSingleUseThread);
-  return v8::Undefined();
 }
 
 v8::Handle<v8::Value> ResolveCallback(const v8::Arguments& arguments)