Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code
Issues
All Issues

Unified Diff: src/FileSystemJsObject.cpp

Issue 29370568: Issue #4692 - Move responsibility for engine reference from tasks to scheduler
Patch Set: Created Dec. 31, 2016, 10:37 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « include/AdblockPlus/JsEngine.h ('k') | src/JsEngine.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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)
« no previous file with comments | « include/AdblockPlus/JsEngine.h ('k') | src/JsEngine.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld