From 5050cc9a4e1c1d152748b6872f8eed157e2cd6ef Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Mon, 1 May 2023 19:52:19 +0200 Subject: [PATCH] Code cleanup --- .../script-engine/src/v8/ArrayBufferClass.cpp | 2 +- .../script-engine/src/v8/ArrayBufferClass.h | 2 +- .../src/v8/ArrayBufferPrototype.cpp | 2 +- .../src/v8/ArrayBufferPrototype.h | 2 +- .../src/v8/ArrayBufferViewClass.cpp | 2 +- .../src/v8/ArrayBufferViewClass.h | 2 +- .../script-engine/src/v8/DataViewClass.cpp | 2 +- .../script-engine/src/v8/DataViewClass.h | 2 +- .../src/v8/DataViewPrototype.cpp | 2 +- .../script-engine/src/v8/DataViewPrototype.h | 2 +- .../src/v8/FastScriptValueUtils.cpp | 24 -- .../src/v8/ScriptContextV8Wrapper.cpp | 29 +- .../src/v8/ScriptContextV8Wrapper.h | 8 +- .../script-engine/src/v8/ScriptEngineV8.cpp | 352 +--------------- .../script-engine/src/v8/ScriptEngineV8.h | 28 +- .../src/v8/ScriptEngineV8_cast.cpp | 32 +- .../src/v8/ScriptObjectV8Proxy.cpp | 390 +----------------- .../src/v8/ScriptObjectV8Proxy.h | 16 - .../src/v8/ScriptProgramV8Wrapper.cpp | 4 - .../src/v8/ScriptProgramV8Wrapper.h | 9 - .../src/v8/ScriptValueIteratorV8Wrapper.cpp | 3 - .../src/v8/ScriptValueIteratorV8Wrapper.h | 1 - .../src/v8/ScriptValueV8Wrapper.cpp | 39 +- .../src/v8/TypedArrayPrototype.cpp | 2 +- .../src/v8/TypedArrayPrototype.h | 2 +- .../script-engine/src/v8/TypedArrays.cpp | 2 +- libraries/script-engine/src/v8/TypedArrays.h | 2 +- libraries/script-engine/src/v8/V8Lambda.h | 1 + libraries/script-engine/src/v8/V8Types.h | 44 +- 29 files changed, 57 insertions(+), 951 deletions(-) diff --git a/libraries/script-engine/src/v8/ArrayBufferClass.cpp b/libraries/script-engine/src/v8/ArrayBufferClass.cpp index 1bc20551fc..1846625776 100644 --- a/libraries/script-engine/src/v8/ArrayBufferClass.cpp +++ b/libraries/script-engine/src/v8/ArrayBufferClass.cpp @@ -21,7 +21,7 @@ #include "TypedArrays.h" -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*static const QString CLASS_NAME = "ArrayBuffer"; // FIXME: Q_DECLARE_METATYPE is global and really belongs in a shared header file, not per .cpp like this diff --git a/libraries/script-engine/src/v8/ArrayBufferClass.h b/libraries/script-engine/src/v8/ArrayBufferClass.h index 533aaa0765..ddc0b323b4 100644 --- a/libraries/script-engine/src/v8/ArrayBufferClass.h +++ b/libraries/script-engine/src/v8/ArrayBufferClass.h @@ -22,8 +22,8 @@ #include "v8.h" #include +// V8TODO Do not remove yet, this will be useful in later PRs //#include "V8Types.h" -// V8TODO /* class ScriptEngineV8; diff --git a/libraries/script-engine/src/v8/ArrayBufferPrototype.cpp b/libraries/script-engine/src/v8/ArrayBufferPrototype.cpp index 0586b25232..62d89eff8c 100644 --- a/libraries/script-engine/src/v8/ArrayBufferPrototype.cpp +++ b/libraries/script-engine/src/v8/ArrayBufferPrototype.cpp @@ -23,7 +23,7 @@ static const int QCOMPRESS_HEADER_POSITION = 0; static const int QCOMPRESS_HEADER_SIZE = 4; -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*Q_DECLARE_METATYPE(QByteArray*) ArrayBufferPrototype::ArrayBufferPrototype(QObject* parent) : QObject(parent) { diff --git a/libraries/script-engine/src/v8/ArrayBufferPrototype.h b/libraries/script-engine/src/v8/ArrayBufferPrototype.h index e601ff2603..4a39e7e909 100644 --- a/libraries/script-engine/src/v8/ArrayBufferPrototype.h +++ b/libraries/script-engine/src/v8/ArrayBufferPrototype.h @@ -17,7 +17,7 @@ #ifndef hifi_ArrayBufferPrototype_h #define hifi_ArrayBufferPrototype_h -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*#include #include "../Scriptable.h" diff --git a/libraries/script-engine/src/v8/ArrayBufferViewClass.cpp b/libraries/script-engine/src/v8/ArrayBufferViewClass.cpp index 1029440f7b..436e67e6a2 100644 --- a/libraries/script-engine/src/v8/ArrayBufferViewClass.cpp +++ b/libraries/script-engine/src/v8/ArrayBufferViewClass.cpp @@ -16,7 +16,7 @@ Q_DECLARE_METATYPE(QByteArray*) -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*ArrayBufferViewClass::ArrayBufferViewClass(ScriptEngineV8* scriptEngine) : QObject(scriptEngine), QScriptClass(scriptEngine), diff --git a/libraries/script-engine/src/v8/ArrayBufferViewClass.h b/libraries/script-engine/src/v8/ArrayBufferViewClass.h index 423372f1dd..a345ce630f 100644 --- a/libraries/script-engine/src/v8/ArrayBufferViewClass.h +++ b/libraries/script-engine/src/v8/ArrayBufferViewClass.h @@ -20,7 +20,7 @@ #include #include "V8Types.h" -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*class ScriptEngineV8; static const QString BUFFER_PROPERTY_NAME = "buffer"; diff --git a/libraries/script-engine/src/v8/DataViewClass.cpp b/libraries/script-engine/src/v8/DataViewClass.cpp index 2d5f1593f6..8d91fdab9f 100644 --- a/libraries/script-engine/src/v8/DataViewClass.cpp +++ b/libraries/script-engine/src/v8/DataViewClass.cpp @@ -13,7 +13,7 @@ #include "DataViewClass.h" -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*#include "DataViewPrototype.h" Q_DECLARE_METATYPE(QByteArray*) diff --git a/libraries/script-engine/src/v8/DataViewClass.h b/libraries/script-engine/src/v8/DataViewClass.h index 47f1b60a75..c8ed7efdea 100644 --- a/libraries/script-engine/src/v8/DataViewClass.h +++ b/libraries/script-engine/src/v8/DataViewClass.h @@ -17,7 +17,7 @@ #ifndef hifi_DataViewClass_h #define hifi_DataViewClass_h -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /* #include "ArrayBufferViewClass.h" diff --git a/libraries/script-engine/src/v8/DataViewPrototype.cpp b/libraries/script-engine/src/v8/DataViewPrototype.cpp index f2d63f7711..d54110557d 100644 --- a/libraries/script-engine/src/v8/DataViewPrototype.cpp +++ b/libraries/script-engine/src/v8/DataViewPrototype.cpp @@ -22,7 +22,7 @@ #include "ArrayBufferViewClass.h" -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*Q_DECLARE_METATYPE(QByteArray*) DataViewPrototype::DataViewPrototype(QObject* parent) : QObject(parent) { diff --git a/libraries/script-engine/src/v8/DataViewPrototype.h b/libraries/script-engine/src/v8/DataViewPrototype.h index 1e7f50bf86..c880551e16 100644 --- a/libraries/script-engine/src/v8/DataViewPrototype.h +++ b/libraries/script-engine/src/v8/DataViewPrototype.h @@ -22,7 +22,7 @@ #include "V8Types.h" #include "../Scriptable.h" -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*/// [V8] The javascript functions associated with a DataView instance prototype class DataViewPrototype : public QObject, public Scriptable { Q_OBJECT diff --git a/libraries/script-engine/src/v8/FastScriptValueUtils.cpp b/libraries/script-engine/src/v8/FastScriptValueUtils.cpp index 130ac1f7b8..07cae34688 100644 --- a/libraries/script-engine/src/v8/FastScriptValueUtils.cpp +++ b/libraries/script-engine/src/v8/FastScriptValueUtils.cpp @@ -20,22 +20,6 @@ #ifdef CONVERSIONS_OPTIMIZED_FOR_V8 ScriptValue vec3ToScriptValue(ScriptEngine* engine, const glm::vec3& vec3) { - //auto prototype = engine->globalObject().property("__hifi_vec3__"); - /*if (!prototype.hasProperty("defined") || !prototype.property("defined").toBool()) { - prototype = engine->evaluate( - "__hifi_vec3__ = Object.defineProperties({}, { " - "defined: { value: true }," - "0: { set: function(nv) { return this.x = nv; }, get: function() { return this.x; } }," - "1: { set: function(nv) { return this.y = nv; }, get: function() { return this.y; } }," - "2: { set: function(nv) { return this.z = nv; }, get: function() { return this.z; } }," - "r: { set: function(nv) { return this.x = nv; }, get: function() { return this.x; } }," - "g: { set: function(nv) { return this.y = nv; }, get: function() { return this.y; } }," - "b: { set: function(nv) { return this.z = nv; }, get: function() { return this.z; } }," - "red: { set: function(nv) { return this.x = nv; }, get: function() { return this.x; } }," - "green: { set: function(nv) { return this.y = nv; }, get: function() { return this.y; } }," - "blue: { set: function(nv) { return this.z = nv; }, get: function() { return this.z; } }" - "})"); - }*/ ScriptValue value = engine->newObject(); ScriptValueV8Wrapper *proxy = ScriptValueV8Wrapper::unwrap(value); @@ -106,11 +90,9 @@ ScriptValue vec3ToScriptValue(ScriptEngine* engine, const glm::vec3& vec3) { Q_ASSERT(false); } - //auto v8Prototype = ScriptValueV8Wrapper::fullUnwrap(engineV8, prototype); if (!v8Object->SetPrototype(context, prototype).FromMaybe(false)) { Q_ASSERT(false); } - //value.setPrototype(prototype); return value; } @@ -143,7 +125,6 @@ bool vec3FromScriptValue(const ScriptValue& object, glm::vec3& vec3) { } } else if (v8Value->IsArray()) { auto array = v8::Local::Cast(v8Value); - //QVariantList list = object.toVariant().toList(); if (array->Length() == 3) { v8::Local xValue,yValue,zValue; if (!array->Get(context, 0).ToLocal(&xValue)) { @@ -219,11 +200,6 @@ bool vec3FromScriptValue(const ScriptValue& object, glm::vec3& vec3) { } } - //f (!x.isValid() || !y.isValid() || !z.isValid()) { - // V8TODO: This breaks the sit script for some reason - //return false; - //} - vec3.x = xValue->NumberValue(context).FromMaybe(0.0); vec3.y = yValue->NumberValue(context).FromMaybe(0.0); vec3.z = zValue->NumberValue(context).FromMaybe(0.0); diff --git a/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp b/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp index 2bb5f2640c..2b22720b8f 100644 --- a/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp +++ b/libraries/script-engine/src/v8/ScriptContextV8Wrapper.cpp @@ -18,11 +18,6 @@ #include "ScriptValueV8Wrapper.h" #include "ScriptEngineLoggingV8.h" - -/*ScriptContextV8Wrapper::ScriptContextV8Wrapper(ScriptEngineV8* engine, const v8::Local context) : _functionCallbackInfo(nullptr), _propertyCallbackInfo(nullptr), _engine(engine) { - _context.Reset(_engine->getIsolate(), context); -}*/ - ScriptContextV8Wrapper::ScriptContextV8Wrapper(ScriptEngineV8* engine, const v8::Local context, ScriptContextPointer parent) : _functionCallbackInfo(nullptr), _propertyCallbackInfo(nullptr), _engine(engine), _context(engine->getIsolate(), context), _parentContext(parent) { @@ -40,6 +35,14 @@ ScriptContextV8Wrapper::ScriptContextV8Wrapper(ScriptEngineV8* engine, const v8: _context(engine->getIsolate(), context), _parentContext(parent) { } +ScriptContextV8Wrapper::~ScriptContextV8Wrapper() noexcept { + //V8TODO: what if destructor happens during shutdown and V8 isolate is already disposed of? + auto isolate = _engine->getIsolate(); + v8::Locker locker(isolate); + v8::Isolate::Scope isolateScope(isolate); + _context.Reset(); +} + ScriptContextV8Wrapper* ScriptContextV8Wrapper::unwrap(ScriptContext* val) { if (!val) { return nullptr; @@ -54,11 +57,6 @@ v8::Local ScriptContextV8Wrapper::toV8Value() const { } int ScriptContextV8Wrapper::argumentCount() const { - /*auto isolate = _engine->getIsolate(); - Q_ASSERT(isolate->IsCurrent()); - v8::HandleScope handleScope(isolate); - v8::Context::Scope contextScope(_engine->getContext());*/ - //Q_ASSERT(_functionCallbackInfo);A // V8TODO auto isolate = _engine->getIsolate(); v8::Locker locker(isolate); @@ -123,7 +121,6 @@ ScriptValue ScriptContextV8Wrapper::callee() const { //V8TODO //Can this be done with CurrentStackTrace? //V8ScriptValue result = _context->callee(); - //return ScriptValue(new ScriptValueV8Wrapper(_engine, std::move(result))); return _engine->undefinedValue(); } @@ -207,6 +204,14 @@ ScriptFunctionContextV8Wrapper::ScriptFunctionContextV8Wrapper(ScriptEngineV8* e _context.Reset(engine->getIsolate(), context); } +ScriptFunctionContextV8Wrapper::~ScriptFunctionContextV8Wrapper() { + //V8TODO: what if destructor happens during shutdown and V8 isolate is already disposed of? + auto isolate = _engine->getIsolate(); + v8::Locker locker(isolate); + v8::Isolate::Scope isolateScope(isolate); + _context.Reset(); +} + QString ScriptFunctionContextV8Wrapper::fileName() const { //V8TODO: It's not exactly like in QtScript, because there's no such context object in V8, let's return the current one for now //Maybe fetch data on creation or store stack frame? @@ -238,7 +243,7 @@ QString ScriptFunctionContextV8Wrapper::functionName() const { } ScriptFunctionContext::FunctionType ScriptFunctionContextV8Wrapper::functionType() const { - //V8TODO + //V8TODO: This is only used in debugPrint. Should we remove it? //return static_cast(_value.functionType()); return ScriptFunctionContext::FunctionType::ScriptFunction; } diff --git a/libraries/script-engine/src/v8/ScriptContextV8Wrapper.h b/libraries/script-engine/src/v8/ScriptContextV8Wrapper.h index 255ec88be9..c410587c45 100644 --- a/libraries/script-engine/src/v8/ScriptContextV8Wrapper.h +++ b/libraries/script-engine/src/v8/ScriptContextV8Wrapper.h @@ -37,7 +37,7 @@ public: // construction const v8::Local context, ScriptContextPointer parent); ScriptContextV8Wrapper(ScriptEngineV8* engine, const v8::PropertyCallbackInfo *propertyCallbackInfo, const v8::Local context, ScriptContextPointer parent); - virtual ~ScriptContextV8Wrapper() {_context.Reset();} + virtual ~ScriptContextV8Wrapper(); static ScriptContextV8Wrapper* unwrap(ScriptContext* val); @@ -60,7 +60,6 @@ private: // storage const v8::FunctionCallbackInfo *_functionCallbackInfo; const v8::PropertyCallbackInfo *_propertyCallbackInfo; ScriptEngineV8* _engine; - // V8TODO: custom destructor is needed for v8::Persistent v8::Persistent _context; ScriptContextPointer _parentContext; Q_DISABLE_COPY(ScriptContextV8Wrapper) @@ -68,9 +67,8 @@ private: // storage class ScriptFunctionContextV8Wrapper final : public ScriptFunctionContext { public: // construction - //V8TODO ScriptFunctionContextV8Wrapper(ScriptEngineV8* engine, const v8::Local context); - virtual ~ScriptFunctionContextV8Wrapper() {_context.Reset();}; + virtual ~ScriptFunctionContextV8Wrapper(); public: // ScriptFunctionContext implementation virtual QString fileName() const override; @@ -80,9 +78,7 @@ public: // ScriptFunctionContext implementation private: // storage ScriptEngineV8* _engine; - // V8TODO: custom destructor is needed for v8::Persistent (check is all other classes using different types of persistent handles have custom destructors too) v8::Persistent _context; - //V8ScriptContextInfo _value; Q_DISABLE_COPY(ScriptFunctionContextV8Wrapper) }; diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.cpp b/libraries/script-engine/src/v8/ScriptEngineV8.cpp index cdcae4d775..72544eed43 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.cpp +++ b/libraries/script-engine/src/v8/ScriptEngineV8.cpp @@ -85,7 +85,7 @@ ScriptValue ScriptEngineV8::makeError(const ScriptValue& _other, const QString& return nullValue(); } - //V8TODO + //V8TODO: do not remove until ScriptEngineV8::makeError is implemented /* auto other = _other; if (_other.constGet()->IsString()) { @@ -130,14 +130,12 @@ ScriptValue ScriptEngineV8::checkScriptSyntax(ScriptProgramPointer program) { v8::HandleScope handleScope(_v8Isolate); v8::Context::Scope contextScope(getContext()); ScriptSyntaxCheckResultPointer syntaxCheck = program->checkSyntax(); - //V8TODO if (syntaxCheck->state() != ScriptSyntaxCheckResult::Valid) { auto err = globalObject().property("SyntaxError").construct(ScriptValueList({ newValue(syntaxCheck->errorMessage()) })); err.setProperty("fileName", program->fileName()); err.setProperty("lineNumber", syntaxCheck->errorLineNumber()); err.setProperty("expressionBeginOffset", syntaxCheck->errorColumnNumber()); err.setProperty("stack", syntaxCheck->errorBacktrace()); - //err.setProperty("stack", currentContext()->backtrace().join(ScriptManager::SCRIPT_BACKTRACE_SEP)); { const auto error = syntaxCheck->errorMessage(); const auto line = QString::number(syntaxCheck->errorLineNumber()); @@ -150,32 +148,6 @@ ScriptValue ScriptEngineV8::checkScriptSyntax(ScriptProgramPointer program) { } return undefinedValue(); } -/*ScriptValue ScriptEngineV8::lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber) { - if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { - return nullValue(); - } - //V8TODO - const auto syntaxCheck = checkSyntax(sourceCode); - if (syntaxCheck.state() != QScriptSyntaxCheckResult::Valid) { - auto err = QScriptEngine::globalObject().property("SyntaxError").construct(V8ScriptValueList({ syntaxCheck.errorMessage() })); - err.setProperty("fileName", fileName); - err.setProperty("lineNumber", syntaxCheck.errorLineNumber()); - err.setProperty("expressionBeginOffset", syntaxCheck.errorColumnNumber()); - err.setProperty("stack", currentContext()->backtrace().join(ScriptManager::SCRIPT_BACKTRACE_SEP)); - { - const auto error = syntaxCheck.errorMessage(); - const auto line = QString::number(syntaxCheck.errorLineNumber()); - const auto column = QString::number(syntaxCheck.errorColumnNumber()); - // for compatibility with legacy reporting - const auto message = QString("[SyntaxError] %1 in %2:%3(%4)").arg(error, fileName, line, column); - err.setProperty("formatted", message); - } - return ScriptValue(new ScriptValueV8Wrapper(this, std::move(err))); - } - return undefinedValue(); -}*/ - - // Lambda /*ScriptValue ScriptEngineV8::newLambdaFunction(std::function operation, @@ -264,8 +236,7 @@ v8::Platform* ScriptEngineV8::getV8Platform() { } ScriptEngineV8::ScriptEngineV8(ScriptManager *manager) : ScriptEngine(manager), _evaluatingCounter(0) - //V8TODO - //_arrayBufferClass(new ArrayBufferClass(this)) + //V8TODO _arrayBufferClass(new ArrayBufferClass(this)) { _v8InitMutex.lock(); std::call_once ( _v8InitOnceFlag, [ ]{ @@ -289,7 +260,6 @@ ScriptEngineV8::ScriptEngineV8(ScriptManager *manager) : ScriptEngine(manager), #else v8::V8::SetFlagsFromString("--stack-size=256"); #endif - //v8::V8::SetFlagsFromString("--stack-size=256 --single-threaded"); v8::Platform* platform = getV8Platform(); v8::V8::InitializePlatform(platform); v8::V8::Initialize(); qCDebug(scriptengine_v8) << "V8 platform initialized"; @@ -317,18 +287,10 @@ ScriptEngineV8::ScriptEngineV8(ScriptManager *manager) : ScriptEngine(manager), registerSystemTypes(); // V8TODO: dispose of isolate on ScriptEngineV8 destruction - //v8::UniquePersistent null = v8::UniquePersistent(_v8Isolate, v8::Null(_v8Isolate)); - //_nullValue = ScriptValue(new ScriptValueV8Wrapper(this, std::move(null))); - - //V8ScriptValue undefined = v8::UniquePersistent(_v8Isolate,v8::Undefined(_v8Isolate)); - //_undefinedValue = ScriptValue(new ScriptValueV8Wrapper(this, std::move(undefined))); - // V8TODO: //QScriptEngine::setProcessEventsInterval(MSECS_PER_SECOND); } - //_currentThread = QThread::currentThread(); - //if (_scriptManager) { // V8TODO: port to V8 /*connect(this, &QScriptEngine::signalHandlerException, this, [this](const V8ScriptValue& exception) { @@ -342,8 +304,6 @@ ScriptEngineV8::ScriptEngineV8(ScriptManager *manager) : ScriptEngine(manager), emit _scriptManager->unhandledException(ScriptValue(new ScriptValueV8Wrapper(this, std::move(thrown)))); } }, Qt::DirectConnection);*/ - //moveToThread(scriptManager->thread()); - //setThread(scriptManager->thread()); //} } @@ -396,16 +356,11 @@ void ScriptEngineV8::registerValue(const QString& valueName, V8ScriptValue value } if (createProperty) { if (partsToGo > 0) { - //This was commented out - //QObject *object = new QObject; - v8::Local partValue = v8::Object::New(_v8Isolate); //newQObject(object, QScriptEngine::ScriptOwnership); - //V8ScriptValue partValue = QScriptEngine::newArray(); //newQObject(object, QScriptEngine::ScriptOwnership); + v8::Local partValue = v8::Object::New(_v8Isolate); if (!partObject->Set(context, pathPartV8, partValue).FromMaybe(false)) { Q_ASSERT(false); } } else { - //partObject = currentPath->ToObject(); - //V8TODO: do these still happen if asserts are disabled? if (!partObject->Set(context, pathPartV8, value.constGet()).FromMaybe(false)) { Q_ASSERT(false); } @@ -440,14 +395,6 @@ void ScriptEngineV8::registerGlobalObject(const QString& name, QObject* object) #ifdef THREAD_DEBUGGING qCDebug(scriptengine_v8) << "ScriptEngineV8::registerGlobalObject() called on thread [" << QThread::currentThread() << "] name:" << name; #endif - /*bool is_isolate_exit_needed = false; - if(!_v8Isolate->IsCurrent() && !_v8Locker) { - // V8TODO: Theoretically only script thread should access this, so it should be safe - _v8Locker.reset(new v8::Locker(_v8Isolate)); - _v8Isolate->Enter(); - is_isolate_exit_needed = true; - }*/ - //{ v8::Locker locker(_v8Isolate); v8::Isolate::Scope isolateScope(_v8Isolate); v8::HandleScope handleScope(_v8Isolate); @@ -457,7 +404,6 @@ void ScriptEngineV8::registerGlobalObject(const QString& name, QObject* object) v8::Local v8GlobalObject = getContext()->Global(); v8::Local v8Name = v8::String::NewFromUtf8(_v8Isolate, name.toStdString().c_str()).ToLocalChecked(); - // V8TODO: Is IsEmpty check enough or IsValid is needed too? if (!v8GlobalObject->Get(getContext(), v8Name).IsEmpty()) { if (object) { V8ScriptValue value = ScriptObjectV8Proxy::newQObject(this, object, ScriptEngine::QtOwnership); @@ -470,16 +416,9 @@ void ScriptEngineV8::registerGlobalObject(const QString& name, QObject* object) } } } - //} - /*if (is_isolate_exit_needed) { - _v8Isolate->Exit(); - _v8Locker.reset(nullptr); - }*/ } void ScriptEngineV8::registerFunction(const QString& name, ScriptEngine::FunctionSignature functionSignature, int numArguments) { - //if (QThread::currentThread() != ) { - //} if (QThread::currentThread() != thread()) { #ifdef THREAD_DEBUGGING qCDebug(scriptengine_v8) << "*** WARNING *** ScriptEngineV8::registerFunction() called on wrong thread [" << QThread::currentThread() << "], invoking on correct thread [" << thread() << "] name:" << name; @@ -494,28 +433,13 @@ void ScriptEngineV8::registerFunction(const QString& name, ScriptEngine::Functio qCDebug(scriptengine_v8) << "ScriptEngineV8::registerFunction() called on thread [" << QThread::currentThread() << "] name:" << name; #endif - /*bool is_isolate_exit_needed = false; - if(!_v8Isolate->IsCurrent() && !_v8Locker) { - // V8TODO: Theoretically only script thread should access this, so it should be safe - _v8Locker.reset(new v8::Locker(_v8Isolate)); - _v8Isolate->Enter(); - is_isolate_exit_needed = true; - } - {*/ - //auto scriptFun = static_cast(newFunction(functionSignature, numArguments).ptr())->toV8Value().constGet(); v8::Locker locker(_v8Isolate); v8::Isolate::Scope isolateScope(_v8Isolate); v8::HandleScope handleScope(_v8Isolate); v8::Context::Scope contextScope(getContext()); auto scriptFun = newFunction(functionSignature, numArguments); - //getContext()->Global().Set(); globalObject().setProperty(name, scriptFun); - /*} - if (is_isolate_exit_needed) { - _v8Isolate->Exit(); - _v8Locker.reset(nullptr); - }*/ } void ScriptEngineV8::registerFunction(const QString& parent, const QString& name, ScriptEngine::FunctionSignature functionSignature, int numArguments) { @@ -533,14 +457,6 @@ void ScriptEngineV8::registerFunction(const QString& parent, const QString& name qCDebug(scriptengine_v8) << "ScriptEngineV8::registerFunction() called on thread [" << QThread::currentThread() << "] parent:" << parent << "name:" << name; #endif - /*bool is_isolate_exit_needed = false; - if(!_v8Isolate->IsCurrent() && !_v8Locker) { - // V8TODO: Theoretically only script thread should access this, so it should be safe - _v8Locker.reset(new v8::Locker(_v8Isolate)); - _v8Isolate->Enter(); - is_isolate_exit_needed = true; - } - {*/ v8::Locker locker(_v8Isolate); v8::Isolate::Scope isolateScope(_v8Isolate); v8::HandleScope handleScope(_v8Isolate); @@ -550,11 +466,6 @@ void ScriptEngineV8::registerFunction(const QString& parent, const QString& name ScriptValue scriptFun = newFunction(functionSignature, numArguments); object.setProperty(name, scriptFun); } - /*} - if (is_isolate_exit_needed) { - _v8Isolate->Exit(); - _v8Locker.reset(nullptr); - }*/ } void ScriptEngineV8::registerGetterSetter(const QString& name, ScriptEngine::FunctionSignature getter, @@ -575,60 +486,17 @@ void ScriptEngineV8::registerGetterSetter(const QString& name, ScriptEngine::Fun qCDebug(scriptengine_v8) << "ScriptEngineV8::registerGetterSetter() called on thread [" << QThread::currentThread() << "] name:" << name << "parent:" << parent; #endif - /*bool is_isolate_exit_needed = false; - if(!_v8Isolate->IsCurrent() && !_v8Locker) { - // V8TODO: Theoretically only script thread should access this, so it should be safe - _v8Locker.reset(new v8::Locker(_v8Isolate)); - _v8Isolate->Enter(); - is_isolate_exit_needed = true; - } - {*/ v8::Locker locker(_v8Isolate); v8::Isolate::Scope isolateScope(_v8Isolate); v8::HandleScope handleScope(_v8Isolate); v8::Context::Scope contextScope(getContext()); - /*auto getterFunction = [](v8::Local property, const v8::PropertyCallbackInfo& info) { - //V8TODO: is using GetCurrentContext ok, or context wrapper needs to be added? - v8::HandleScope handleScope(info.GetIsolate()); - auto context = info.GetIsolate()->GetCurrentContext(); - v8::Context::Scope contextScope(context); - auto object = v8::Local::Cast(info.Data()); - Q_ASSERT(object->InternalFieldCount() == 2); - auto function = reinterpret_cast - (object->GetAlignedPointerFromInternalField(0)); - ScriptEngineV8 *scriptEngine = reinterpret_cast - (object->GetAlignedPointerFromInternalField(1)); - ScriptContextV8Wrapper scriptContext(scriptEngine, &info); - //V8TODO: this scriptContext needs to have FunctionCallbackInfo added - ScriptValue result = function(&scriptContext, scriptEngine); - ScriptValueV8Wrapper* unwrapped = ScriptValueV8Wrapper::unwrap(result); - info.GetReturnValue().Set(unwrapped->toV8Value().constGet()); - }; - auto setterFunction = [](v8::Local property, const v8::PropertyCallbackInfo& info) { - //V8TODO: is using GetCurrentContext ok, or context wrapper needs to be added? - v8::HandleScope handleScope(info.GetIsolate()); - auto context = info.GetIsolate()->GetCurrentContext(); - v8::Context::Scope contextScope(context); - auto object = v8::Local::Cast(info.Data()); - Q_ASSERT(object->InternalFieldCount() == 2); - auto function = reinterpret_cast - (object->GetAlignedPointerFromInternalField(0)); - ScriptEngineV8 *scriptEngine = reinterpret_cast - (object->GetAlignedPointerFromInternalField(1)); - ScriptContextV8Wrapper scriptContext(scriptEngine, &info); - //V8TODO: this scriptContext needs to have FunctionCallbackInfo added - ScriptValue result = function(&scriptContext, scriptEngine); - ScriptValueV8Wrapper* unwrapped = ScriptValueV8Wrapper::unwrap(result); - };*/ - ScriptValue setterFunction = newFunction(setter, 1); ScriptValue getterFunction = newFunction(getter); V8ScriptValue unwrappedGetter = ScriptValueV8Wrapper::fullUnwrap(this, getterFunction); V8ScriptValue unwrappedSetter = ScriptValueV8Wrapper::fullUnwrap(this, setterFunction); v8::PropertyDescriptor propertyDescriptor(unwrappedGetter.get(), unwrappedSetter.get()); - //V8TODO: Getters/setters are probably done in a different way in V8. Maybe object template is needed? if (!parent.isNull() && !parent.isEmpty()) { ScriptValue object = globalObject().property(parent); if (object.isValid()) { @@ -649,8 +517,6 @@ void ScriptEngineV8::registerGetterSetter(const QString& name, ScriptEngine::Fun qCDebug(scriptengine_v8) << "DefineProperty failed for registerGetterSetter \"" << name << "\" for parent: \"" << parent << "\""; } - //object.setProperty(name, setterFunction, ScriptValue::PropertySetter); - //object.setProperty(name, getterFunction, ScriptValue::PropertyGetter); } else { qCDebug(scriptengine_v8) << "Parent object \"" << parent << "\" for registerGetterSetter \"" << name << "\" is not valid: "; @@ -661,14 +527,7 @@ void ScriptEngineV8::registerGetterSetter(const QString& name, ScriptEngine::Fun if (!getContext()->Global()->DefineProperty(getContext(), v8propertyName, propertyDescriptor).FromMaybe(false)) { qCDebug(scriptengine_v8) << "DefineProperty failed for registerGetterSetter \"" << name << "\" for global object"; } - //globalObject().setProperty(name, setterFunction, ScriptValue::PropertySetter); - //globalObject().setProperty(name, getterFunction, ScriptValue::PropertyGetter); } - /*} - if (is_isolate_exit_needed) { - _v8Isolate->Exit(); - _v8Locker.reset(nullptr); - }*/ } v8::Local ScriptEngineV8::getContext() { @@ -721,11 +580,9 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, storeGlobalObjectContents(); v8::Local closureObject; - //v8::Local oldGlobal; v8::Local closureGlobal; ScriptValueV8Wrapper* unwrappedClosure; ScriptProgramV8Wrapper* unwrappedProgram; - //v8::Local oldContext = getContext(); { v8::Context::Scope contextScope(getContext()); @@ -736,7 +593,6 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, Q_ASSERT(false); return nullValue(); } - // V8TODO: is another context switch necessary after unwrapping closure? const auto fileName = unwrappedProgram->fileName(); const auto shortName = QUrl(fileName).fileName(); @@ -750,7 +606,6 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, } const V8ScriptValue& closure = unwrappedClosure->toV8Value(); - //const V8ScriptProgram& program = unwrappedProgram->toV8Value(); if (!closure.constGet()->IsObject()) { _evaluatingCounter--; qCDebug(scriptengine_v8) << "Unwrapped closure is not an object"; @@ -773,28 +628,13 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, Q_ASSERT(false); return nullValue(); } - //qCDebug(scriptengine_v8) << "Closure global details:" << scriptValueDebugDetailsV8(V8ScriptValue(_v8Isolate, closureGlobal)); } - //oldGlobal = _v8Context.Get(_v8Isolate)->Global(); v8::Local closureContext; - // V8TODO V8 cannot use arbitrary objects as global objects - /*if (closureGlobal->IsObject()) { -#ifdef DEBUG_JS - qCDebug(shared) << " setting global = closure.global" << shortName; -#endif - closureContext = v8::Context::New(_v8Isolate, nullptr, v8::Local(), closureGlobal); - closureContext = v8::Context::New(_v8Isolate, nullptr, v8::Local(), closureGlobal); - //setGlobalObject(global); - } else { - closureContext = v8::Context::New(_v8Isolate); - }*/ closureContext = v8::Context::New(_v8Isolate); pushContext(closureContext); ScriptValue result; - //auto context = pushContext(); - // V8TODO: a lot of functions rely on _v8Context, which was not updated here // It might cause trouble { @@ -809,27 +649,6 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, const V8ScriptProgram& program = unwrappedProgram->toV8Value(); v8::Local thiz; - // V8TODO: not sure if "this" is used at all here - /*if (!closureObject->Get(closure.constGetContext(), v8::String::NewFromUtf8(_v8Isolate, "this").ToLocalChecked()) - .ToLocal(&thiz)) { - _evaluatingCounter--; - qCDebug(scriptengine_v8) << "Empty this object in closure"; - Q_ASSERT(false); - return nullValue(); - }*/ - //thiz = closure.property("this"); - //qCDebug(scriptengine_v8) << "Closure this details:" << scriptValueDebugDetailsV8(V8ScriptValue(_v8Isolate, thiz)); - // V8TODO: - /*if (thiz->IsObject()) { -#ifdef DEBUG_JS - qCDebug(shared) << " setting this = closure.this" << shortName; -#endif - //V8TODO I don't know how to do this in V8, will adding "this" to global object work? - closureContext->Global()->Set(closureContext, v8::String::NewFromUtf8(_v8Isolate, "this").ToLocalChecked(), thiz); - //context->setThisObject(thiz); - }*/ - - //context->pushScope(closure); #ifdef DEBUG_JS qCDebug(shared) << QString("[%1] evaluateInClosure %2").arg(isEvaluating()).arg(shortName); #endif @@ -846,51 +665,22 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, } qCDebug(scriptengine_v8) << "ScriptEngineV8::evaluateInClosure: " << globalMemberNames->Length() << " objects added to global"; - /*auto oldGlobalMemberNames = oldContext->Global()->GetPropertyNames(oldContext).ToLocalChecked(); - //auto oldGlobalMemberNames = oldContext->Global()->GetPropertyNames(closureContext).ToLocalChecked(); - for (size_t i = 0; i < oldGlobalMemberNames->Length(); i++) { - auto name = oldGlobalMemberNames->Get(closureContext, i).ToLocalChecked(); - //auto name = oldGlobalMemberNames->Get(oldContext, i).ToLocalChecked(); - if(!closureContext->Global()->Set(closureContext, name, oldContext->Global()->Get(oldContext, name).ToLocalChecked()).FromMaybe(false)) { - //if(!closureContext->Global()->Set(closureContext, name, oldContext->Global()->Get(closureContext, name).ToLocalChecked()).FromMaybe(false)) { - Q_ASSERT(false); - } - }*/ // Objects from closure need to be copied to global object too // V8TODO: I'm not sure which context to use with Get auto closureMemberNames = closureObject->GetPropertyNames(closureContext).ToLocalChecked(); - //auto closureMemberNames = closureObject->GetPropertyNames(oldContext).ToLocalChecked(); for (size_t i = 0; i < closureMemberNames->Length(); i++) { auto name = closureMemberNames->Get(closureContext, i).ToLocalChecked(); - //auto name = closureMemberNames->Get(oldContext, i).ToLocalChecked(); if(!closureContext->Global()->Set(closureContext, name, closureObject->Get(closureContext, name).ToLocalChecked()).FromMaybe(false)) { - //if(!closureContext->Global()->Set(closureContext, name, closureObject->Get(oldContext, name).ToLocalChecked()).FromMaybe(false)) { Q_ASSERT(false); } } - // List members of closure global object - //QString membersString(""); - /*if (closureContext->Global()->IsObject()) { - v8::Local membersStringV8; - v8::Local object = v8::Local::Cast(closureContext->Global()); - auto names = object->GetPropertyNames(closureContext).ToLocalChecked(); - if (v8::JSON::Stringify(closureContext, names).ToLocal(&membersStringV8)) { - membersString = QString(*v8::String::Utf8Value(_v8Isolate, membersStringV8)); - } - membersString = QString(*v8::String::Utf8Value(_v8Isolate, membersStringV8)); - } else { - membersString = QString(" Is not an object"); - }*/ - //qCDebug(scriptengine_v8) << "Closure global before run:" << membersString; auto maybeResult = program.constGet()->GetUnboundScript()->BindToCurrentContext()->Run(closureContext); - //qCDebug(scriptengine_v8) << "Closure after run:" << scriptValueDebugDetailsV8(closure); v8::Local v8Result; if (!maybeResult.ToLocal(&v8Result)) { v8::String::Utf8Value utf8Value(getIsolate(), tryCatch.Exception()); QString errorMessage = QString(*utf8Value); qCWarning(scriptengine_v8) << __FUNCTION__ << "---------- hasCaught:" << errorMessage; qCWarning(scriptengine_v8) << __FUNCTION__ << "---------- tryCatch details:" << formatErrorMessageFromTryCatch(tryCatch); - //V8TODO: better error reporting } if (hasUncaughtException()) { @@ -908,20 +698,12 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, #endif popContext(); } - //This is probably unnecessary in V8 - /*if (oldGlobal.isValid()) { -#ifdef DEBUG_JS - qCDebug(shared) << " restoring global" << shortName; -#endif - setGlobalObject(oldGlobal); - }*/ _evaluatingCounter--; return result; } ScriptValue ScriptEngineV8::evaluate(const QString& sourceCode, const QString& fileName) { - //V8TODO if (QThread::currentThread() != thread()) { ScriptValue result; @@ -936,7 +718,6 @@ ScriptValue ScriptEngineV8::evaluate(const QString& sourceCode, const QString& f return result; } // Compile and check syntax - // V8TODO: Could these all be replaced with checkSyntax function from wrapper? Q_ASSERT(!_v8Isolate->IsDead()); _evaluatingCounter++; v8::Locker locker(_v8Isolate); @@ -953,26 +734,6 @@ ScriptValue ScriptEngineV8::evaluate(const QString& sourceCode, const QString& f return nullValue(); } } - //qCDebug(scriptengine_v8) << "Script compilation succesful: " << fileName; - - //V8TODO - /*auto syntaxError = lintScript(sourceCode, fileName); - if (syntaxError.isError()) { - if (!isEvaluating()) { - syntaxError.setProperty("detail", "evaluate"); - } - raiseException(syntaxError); - maybeEmitUncaughtException("lint"); - return syntaxError; - }*/ - //V8TODO - /*if (script->IsNull()) { - // can this happen? - auto err = makeError(newValue("could not create V8ScriptProgram for " + fileName)); - raiseException(err); - maybeEmitUncaughtException("compile"); - return err; - }*/ v8::Local result; v8::TryCatch tryCatchRun(getIsolate()); @@ -981,11 +742,6 @@ ScriptValue ScriptEngineV8::evaluate(const QString& sourceCode, const QString& f auto runError = tryCatchRun.Message(); ScriptValue errorValue(new ScriptValueV8Wrapper(this, V8ScriptValue(this, runError->Get()))); qCDebug(scriptengine_v8) << "Running script: \"" << fileName << "\" " << formatErrorMessageFromTryCatch(tryCatchRun); - //V8TODO - - - //raiseException(errorValue); - //maybeEmitUncaughtException("evaluate"); setUncaughtException(tryCatchRun, "script evaluation"); _evaluatingCounter--; @@ -1020,7 +776,6 @@ void ScriptEngineV8::setUncaughtException(const v8::TryCatch &tryCatch, const QS QString errorMessage = ""; QString errorBacktrace = ""; - //v8::String::Utf8Value utf8Value(getIsolate(), tryCatch.Exception()); v8::String::Utf8Value utf8Value(getIsolate(), tryCatch.Message()->Get()); ex->errorMessage = QString(*utf8Value); @@ -1068,7 +823,6 @@ QString ScriptEngineV8::formatErrorMessageFromTryCatch(v8::TryCatch &tryCatch) { int errorLineNumber = 0; QString errorMessage = ""; QString errorBacktrace = ""; - //v8::String::Utf8Value utf8Value(getIsolate(), tryCatch.Exception()); v8::String::Utf8Value utf8Value(getIsolate(), tryCatch.Message()->Get()); errorMessage = QString(*utf8Value); v8::Local exceptionMessage = tryCatch.Message(); @@ -1181,13 +935,6 @@ Q_INVOKABLE ScriptValue ScriptEngineV8::evaluate(const ScriptProgramPointer& pro return result; } _evaluatingCounter++; - /*bool is_isolate_exit_needed = false; - if(!_v8Isolate->IsCurrent() && !_v8Locker) { - // V8TODO: Theoretically only script thread should access this, so it should be safe - _v8Locker.reset(new v8::Locker(_v8Isolate)); - _v8Isolate->Enter(); - is_isolate_exit_needed = true; - }*/ ScriptValue errorValue; ScriptValue resultValue; bool hasFailed = false; @@ -1213,14 +960,6 @@ Q_INVOKABLE ScriptValue ScriptEngineV8::evaluate(const ScriptProgramPointer& pro v8::Local result; if(!hasFailed) { const V8ScriptProgram& v8Program = unwrapped->toV8Value(); - // V8TODO - /*if (qProgram.isNull()) { - // can this happen? - auto err = makeError(newValue("requested program is empty")); - raiseException(err); - maybeEmitUncaughtException("compile"); - return err; - }*/ v8::TryCatch tryCatchRun(getIsolate()); if (!v8Program.constGet()->Run(getContext()).ToLocal(&result)) { @@ -1240,10 +979,6 @@ Q_INVOKABLE ScriptValue ScriptEngineV8::evaluate(const ScriptProgramPointer& pro } } _evaluatingCounter--; - /*if (is_isolate_exit_needed) { - _v8Isolate->Exit(); - _v8Locker.reset(nullptr); - }*/ if (hasFailed) { return errorValue; } else { @@ -1293,6 +1028,7 @@ ScriptValue ScriptEngineV8::newArrayBuffer(const QByteArray& message) { std::shared_ptr backingStore(v8::ArrayBuffer::NewBackingStore(_v8Isolate, message.size())); std::memcpy(backingStore.get()->Data(), message.constData(), message.size()); auto arrayBuffer = v8::ArrayBuffer::New(_v8Isolate, backingStore); + //V8TODO: this needs to be finished and tested /*V8ScriptValue data = QScriptEngine::newVariant(QVariant::fromValue(message)); V8ScriptValue ctor = QScriptEngine::globalObject().property("ArrayBuffer"); auto array = qscriptvalue_cast(ctor.data()); @@ -1304,13 +1040,6 @@ ScriptValue ScriptEngineV8::newArrayBuffer(const QByteArray& message) { } ScriptValue ScriptEngineV8::newObject() { - /*bool is_isolate_exit_needed = false; - if(!_v8Isolate->IsCurrent() && !_v8Locker) { - // V8TODO: Theoretically only script thread should access this, so it should be safe - _v8Locker.reset(new v8::Locker(_v8Isolate)); - _v8Isolate->Enter(); - is_isolate_exit_needed = true; - }*/ ScriptValue result; { v8::Locker locker(_v8Isolate); @@ -1320,10 +1049,6 @@ ScriptValue ScriptEngineV8::newObject() { V8ScriptValue resultV8 = V8ScriptValue(this, v8::Object::New(_v8Isolate)); result = ScriptValue(new ScriptValueV8Wrapper(this, std::move(resultV8))); } - /*if (is_isolate_exit_needed) { - _v8Isolate->Exit(); - _v8Locker.reset(nullptr); - }*/ return result; } @@ -1338,10 +1063,6 @@ ScriptValue ScriptEngineV8::newMethod(QObject* object, V8ScriptValue lifetime, } ScriptProgramPointer ScriptEngineV8::newProgram(const QString& sourceCode, const QString& fileName) { - //V8TODO: is it used between isolates? - //V8TODO: should it be compiled on creation? - //V8ScriptProgram result(sourceCode, fileName); - v8::Locker locker(_v8Isolate); v8::Isolate::Scope isolateScope(_v8Isolate); v8::HandleScope handleScope(_v8Isolate); @@ -1453,20 +1174,8 @@ void ScriptEngineV8::clearExceptions() { } ScriptContext* ScriptEngineV8::currentContext() const { - //V8TODO - /*V8ScriptContext* localCtx = QScriptEngine::currentContext(); - if (!localCtx) { - return nullptr; - } - if (!_currContext || _currContext->toV8Value() != localCtx) { - _currContext = std::make_shared(const_cast(this), localCtx); - }*/ - //_currContext = std::make_shared(const_cast(this), localCtx); - /*if (!_currContext) { - // I'm not sure how to do this without discarding const - _currContext = std::make_shared(const_cast(this)); - }*/ // V8TODO: add FunctionCallbackInfo or PropertyCallbackInfo when necessary + // Is it needed? return _contexts.last().get(); } @@ -1482,16 +1191,6 @@ bool ScriptEngineV8::isEvaluating() const { ScriptValue ScriptEngineV8::newFunction(ScriptEngine::FunctionSignature fun, int length) { //V8TODO is callee() used for anything? - /*auto innerFunc = [](V8ScriptContext* _context, QScriptEngine* _engine) -> V8ScriptValue { - auto callee = _context->callee(); - QVariant funAddr = callee.property("_func").toVariant(); - ScriptEngine::FunctionSignature fun = reinterpret_cast(funAddr.toULongLong()); - ScriptEngineV8* engine = static_cast(_engine); - ScriptContextV8Wrapper context(engine, _context); - ScriptValue result = fun(&context, engine); - ScriptValueV8Wrapper* unwrapped = ScriptValueV8Wrapper::unwrap(result); - return unwrapped ? unwrapped->toV8Value() : V8ScriptValue(); - };*/ v8::Locker locker(_v8Isolate); v8::Isolate::Scope isolateScope(_v8Isolate); @@ -1518,23 +1217,12 @@ ScriptValue ScriptEngineV8::newFunction(ScriptEngine::FunctionSignature fun, int info.GetReturnValue().Set(unwrapped->toV8Value().constGet()); } }; - //auto functionTemplate = v8::FunctionTemplate::New(_v8Isolate, v8FunctionCallback, v8::Local(), v8::Local(), length); - //auto functionData = v8::Object::New(_v8Isolate); - //functionData->setIn auto functionDataTemplate = getFunctionDataTemplate(); - //auto functionDataTemplate = v8::ObjectTemplate::New(_v8Isolate); - //functionDataTemplate->SetInternalFieldCount(2); auto functionData = functionDataTemplate->NewInstance(getContext()).ToLocalChecked(); functionData->SetAlignedPointerInInternalField(0, reinterpret_cast(fun)); functionData->SetAlignedPointerInInternalField(1, reinterpret_cast(this)); - //functionData->SetInternalField(3, v8::Null(_v8Isolate)); auto v8Function = v8::Function::New(getContext(), v8FunctionCallback, functionData, length).ToLocalChecked(); - //auto functionObjectTemplate = functionTemplate->InstanceTemplate(); - //auto function = - V8ScriptValue result(this, v8Function); // = QScriptEngine::newFunction(innerFunc, length); - //auto funAddr = QScriptEngine::newVariant(QVariant(reinterpret_cast(fun))); - // V8TODO - //result.setProperty("_func", funAddr, V8ScriptValue::PropertyFlags(V8ScriptValue::ReadOnly + V8ScriptValue::Undeletable + V8ScriptValue::SkipInEnumeration)); + V8ScriptValue result(this, v8Function); return ScriptValue(new ScriptValueV8Wrapper(this, std::move(result))); } @@ -1543,7 +1231,6 @@ void ScriptEngineV8::setObjectName(const QString& name) { QObject::setObjectName(name); } -//V8TODO bool ScriptEngineV8::setProperty(const char* name, const QVariant& value) { v8::Locker locker(_v8Isolate); v8::Isolate::Scope isolateScope(_v8Isolate); @@ -1570,24 +1257,10 @@ void ScriptEngineV8::setThread(QThread* thread) { qCDebug(scriptengine_v8) << "Script engine " << objectName() << " exited isolate"; } Q_ASSERT(QObject::thread() == QThread::currentThread()); - /*if (_v8Locker) { - _v8Locker.reset(); - }*/ moveToThread(thread); qCDebug(scriptengine_v8) << "Moved script engine " << objectName() << " to different thread"; } -/*void ScriptEngineV8::enterIsolateOnThisThread() { - Q_ASSERT(thread() == QThread::currentThread()); - Q_ASSERT(!_v8Locker); - _v8Locker.reset(new v8::Locker(_v8Isolate)); - if (!_v8Isolate->IsCurrent()) { - _v8Isolate->Enter(); - qCDebug(scriptengine_v8) << "Script engine " << objectName() << " entered isolate on a new thread"; - } -}*/ - - std::shared_ptr ScriptEngineV8::uncaughtException() const { if (_uncaughtException) { return _uncaughtException->clone(); @@ -1601,8 +1274,7 @@ bool ScriptEngineV8::raiseException(const QString& error, const QString &reason) } bool ScriptEngineV8::raiseException(const ScriptValue& exception, const QString &reason) { - //V8TODO - //Q_ASSERT(false); + //V8TODO Im not sure how to finish these // qCCritical(scriptengine_v8) << "Script exception occurred: " << exception.toString(); // ScriptValueV8Wrapper* unwrapped = ScriptValueV8Wrapper::unwrap(exception); // V8ScriptValue qException = unwrapped ? unwrapped->toV8Value() : QScriptEngine::newVariant(exception.toVariant()); @@ -1626,7 +1298,6 @@ bool ScriptEngineV8::raiseException(const V8ScriptValue& exception) { _v8Isolate->ThrowException(exception.constGet()); - /*if (QScriptEngine::currentContext()) { // we have an active context / JS stack frame so throw the exception per usual QScriptEngine::currentContext()->throwValue(makeError(exception)); @@ -1741,15 +1412,6 @@ QString ScriptEngineV8::scriptValueDebugDetailsV8(const V8ScriptValue &v8Value) return parentValueQString + QString(" JSON: ") + JSONQString; } -/*QStringList ScriptEngineV8::getCurrentStackTrace() { - v8::Local stackTrace = v8::StackTrace::CurrentStackTrace(_v8Isolate, 100); - QStringList backtrace; - for (int n = 0; n < stackTrace->GetFrameCount(); n++) { - v8::Local stackFrame = stackTrace->GetFrame(_v8Isolate, n); - - } -}*/ - void ScriptEngineV8::logBacktrace(const QString &title) { QStringList backtrace = currentContext()->backtrace(); qCDebug(scriptengine_v8) << title; diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.h b/libraries/script-engine/src/v8/ScriptEngineV8.h index 94e4c36538..fd1536f33a 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.h +++ b/libraries/script-engine/src/v8/ScriptEngineV8.h @@ -78,14 +78,12 @@ public: // ScriptEngine implementation virtual ScriptValue globalObject() override; virtual bool hasUncaughtException() const override; virtual bool isEvaluating() const override; - //virtual ScriptValue lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber = 1) override; virtual ScriptValue checkScriptSyntax(ScriptProgramPointer program) override; virtual ScriptValue newArray(uint length = 0) override; virtual ScriptValue newArrayBuffer(const QByteArray& message) override; virtual ScriptValue newFunction(ScriptEngine::FunctionSignature fun, int length = 0) override; virtual ScriptValue newObject() override; - //virtual ScriptValue newObject( v8::Local ); virtual ScriptValue newMethod(QObject* object, V8ScriptValue lifetime, const QList& metas, int numMaxParams); virtual ScriptProgramPointer newProgram(const QString& sourceCode, const QString& fileName) override; @@ -124,7 +122,6 @@ public: // ScriptEngine implementation virtual void setProcessEventsInterval(int interval) override; virtual QThread* thread() const override; virtual void setThread(QThread* thread) override; - //Q_INVOKABLE virtual void enterIsolateOnThisThread() override; virtual ScriptValue undefinedValue() override; virtual std::shared_ptr uncaughtException() const override; virtual void updateMemoryCost(const qint64& deltaSize) override; @@ -153,18 +150,6 @@ protected: // brought over from BaseScriptEngine static bool IS_THREADSAFE_INVOCATION(const QThread* thread, const QString& method); public: // public non-interface methods for other QtScript-specific classes to use - /*typedef V8ScriptValue (*FunctionSignature)(v8::Local, ScriptEngineV8 *); - typedef V8ScriptValue (*FunctionWithArgSignature)(v8::Local, ScriptEngineV8 *, void *); - /// registers a global getter/setter - Q_INVOKABLE void registerGetterSetter(const QString& name, FunctionSignature getter, - FunctionSignature setter, const QString& parent = QString("")); - - /// register a global function - Q_INVOKABLE void registerFunction(const QString& name, FunctionSignature fun, int numArguments = -1); - - /// register a function as a method on a previously registered global object - Q_INVOKABLE void registerFunction(const QString& parent, const QString& name, FunctionSignature fun, - int numArguments = -1);*/ /// registers a global object by name Q_INVOKABLE void registerValue(const QString& valueName, V8ScriptValue value); @@ -193,15 +178,14 @@ public: // not for public use, but I don't like how Qt strings this along with p const v8::Local getConstContext() const; QString formatErrorMessageFromTryCatch(v8::TryCatch &tryCatch); // Useful for debugging - //QStringList getCurrentStackTrace(); virtual QStringList getCurrentScriptURLs() const override; using ObjectWrapperMap = QMap>; mutable QMutex _qobjectWrapperMapProtect; ObjectWrapperMap _qobjectWrapperMap; // Second map, from which wrappers are removed by script engine upon deletion - // V8TODO add a V8 callback that removes pointer from the map so that it gets deleted QMap> _qobjectWrapperMapV8; + // V8TODO: maybe just a single map can be used instead to increase performance? // Used by ScriptObjectV8Proxy to create JS objects referencing C++ ones v8::Local getObjectProxyTemplate(); @@ -219,17 +203,9 @@ public: // not for public use, but I don't like how Qt strings this along with p void decrementScriptValueCounter() { scriptValueCount--; }; void incrementScriptValueProxyCounter() { scriptValueProxyCount++; }; void decrementScriptValueProxyCounter() { scriptValueProxyCount--; }; - //V8TODO: do the same for other proxy objects #endif protected: - // like `newFunction`, but allows mapping inline C++ lambdas with captures as callable V8ScriptValues - // even though the context/engine parameters are redundant in most cases, the function signature matches `newFunction` - // anyway so that newLambdaFunction can be used to rapidly prototype / test utility APIs and then if becoming - // permanent more easily promoted into regular static newFunction scenarios. - /*ScriptValue newLambdaFunction(std::function operation, - const V8ScriptValue& data, - const ValueOwnership& ownership = AutoOwnership);*/ void registerSystemTypes(); @@ -249,7 +225,6 @@ protected: // V8TODO: clean up isolate when script engine is destroyed? v8::Isolate* _v8Isolate; - //v8::UniquePersistent _v8Context; struct CustomMarshal { ScriptEngine::MarshalFunction marshalFunc; @@ -263,7 +238,6 @@ protected: CustomPrototypeMap _customPrototypes; ScriptValue _nullValue; ScriptValue _undefinedValue; - //mutable ScriptContextV8Pointer _currContext; // Current context stack. Main context is first on the list and current one is last. QList _contexts; // V8TODO: release in destructor diff --git a/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp b/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp index fd62047b25..8b96fb2cbf 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp +++ b/libraries/script-engine/src/v8/ScriptEngineV8_cast.cpp @@ -53,15 +53,6 @@ void ScriptEngineV8::registerCustomType(int type, Q_DECLARE_METATYPE(ScriptValue); Q_DECLARE_METATYPE(QVariantMap); -/*static V8ScriptValue ScriptValueToV8ScriptValue(ScriptEngineV8* engine, const ScriptValue& src) { - return ScriptValueV8Wrapper::fullUnwrap(static_cast(engine), src); -}*/ - -/*static void ScriptValueFromV8ScriptValue(ScriptEngineV8* engine, const V8ScriptValue& src, ScriptValue& dest) { - //ScriptEngineV8* engine = static_cast(src.engine()); - dest = ScriptValue(new ScriptValueV8Wrapper(engine, src)); -}*/ - static ScriptValue StringListToScriptValue(ScriptEngine* engine, const QStringList& src) { int len = src.length(); ScriptValue dest = engine->newArray(len); @@ -189,9 +180,6 @@ static bool JsonArrayFromScriptValue(const ScriptValue& src, QJsonArray& dest) { // QMetaType::QJsonArray void ScriptEngineV8::registerSystemTypes() { - // V8TODO: why is this commented out? - //qScriptRegisterMetaType(this, ScriptValueToV8ScriptValue, ScriptValueFromV8ScriptValue); - scriptRegisterMetaType(static_cast(this)); scriptRegisterMetaType(this); scriptRegisterMetaType(this); @@ -308,12 +296,6 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de v8::Context::Scope contextScope(context); const v8::Local val = v8Val.constGet(); - // Conversion debugging: - /*if (destTypeId == QMetaType::QVariant && val->IsBoolean()) { - //It's for placing breakpoint here - qCDebug(scriptengine_v8) << "Conversion Debug: " << scriptValueDebugDetailsV8(v8Val); - }*/ - // if we're not particularly interested in a specific type, try to detect if we're dealing with a registered type if (destTypeId == QMetaType::UnknownType) { QObject* obj = ScriptObjectV8Proxy::unwrap(v8Val); @@ -346,10 +328,8 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de _customTypeProtect.unlock(); } if (demarshalFunc) { - //dest = QVariant(destTypeId, static_cast(NULL)); dest = QVariant(); ScriptValue wrappedVal(new ScriptValueV8Wrapper(this, v8Val)); - //Q_ASSERT(dest.constData() != nullptr); bool success = demarshalFunc(wrappedVal, dest); if(!success) dest = QVariant(); return success; @@ -374,7 +354,6 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de v8::String::Utf8Value string(_v8Isolate, val); Q_ASSERT(*string != nullptr); dest = QVariant::fromValue(QString(*string)); - //dest = QVariant::fromValue(val->ToString(_v8Context.Get(_v8Isolate)).ToLocalChecked()->); break; } if (val->IsNumber()) { @@ -406,12 +385,12 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de break; } } - // V8TODO errorMessage = QString() + "Conversion failure: " + QString(*v8::String::Utf8Value(_v8Isolate, val->ToDetailString(getConstContext()).ToLocalChecked())) + "to variant. Destination type: " + QMetaType::typeName(destTypeId) +" details: "+ scriptValueDebugDetailsV8(v8Val); qCDebug(scriptengine_v8) << errorMessage; //Q_ASSERT(false); + // V8TODO //dest = val->ToVariant(); break; case QMetaType::Bool: @@ -489,16 +468,13 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de return true; } if (val->IsBoolean()) { - //V8TODO is it right isolate? What if value from different script engine is used here dest = QVariant::fromValue(val->BooleanValue(_v8Isolate)); return true; } if (val->IsString()) { - //V8TODO is it right context? What if value from different script engine is used here v8::String::Utf8Value string(_v8Isolate, val); Q_ASSERT(*string != nullptr); dest = QVariant::fromValue(QString(*string)); - //dest = QVariant::fromValue(val->ToString(_v8Context.Get(_v8Isolate)).ToLocalChecked()->); return true; } if (val->IsNumber()) { @@ -529,7 +505,6 @@ bool ScriptEngineV8::castValueToVariant(const V8ScriptValue& v8Val, QVariant& de return true; } } - //V8TODO is v8::Array to QVariant conversion used anywhere? errorMessage = QString() + "Conversion to variant failed: " + QString(*v8::String::Utf8Value(_v8Isolate, val->ToDetailString(getConstContext()).ToLocalChecked())) + " Destination type: " + QMetaType::typeName(destTypeId) + " Value details: " + scriptValueDebugDetailsV8(v8Val); qCDebug(scriptengine_v8) << errorMessage; @@ -640,14 +615,12 @@ bool ScriptEngineV8::convertJSObjectToVariant(v8::Local object, QVar } QString ScriptEngineV8::valueType(const V8ScriptValue& v8Val) { - // V8TODO I'm not sure why is there a TODO here v8::Locker locker(_v8Isolate); v8::Isolate::Scope isolateScope(_v8Isolate); v8::HandleScope handleScope(_v8Isolate); v8::Local context = getContext(); v8::Context::Scope contextScope(context); - //v8::HandleScope handleScope(const_cast(v8Val.constGetIsolate())); const v8::Local val = v8Val.constGet(); if (val->IsUndefined()) { @@ -684,7 +657,6 @@ QString ScriptEngineV8::valueType(const V8ScriptValue& v8Val) { return dest.typeName(); } qCDebug(scriptengine_v8) << "Cast to variant failed"; - // V8TODO: what to return here? return "undefined"; } @@ -765,7 +737,7 @@ V8ScriptValue ScriptEngineV8::castVariantToValue(const QVariant& val) { } default: // check to see if this is a pointer to a QObject-derived object - // V8TODO: WeakPointerToQObject and SharedPointerToQObject were causing trouble here because some values are handled by custom prototypes instead + // WeakPointerToQObject and SharedPointerToQObject were causing trouble here because some values are handled by custom prototypes instead if (QMetaType::typeFlags(valTypeId) & (QMetaType::PointerToQObject | QMetaType::TrackingPointerToQObject)) { QObject* obj = val.value(); if (obj == nullptr) return V8ScriptValue(this, v8::Null(_v8Isolate)); diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp index fdf5f79a52..b515c3bf2c 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp @@ -24,12 +24,7 @@ #include "ScriptValueV8Wrapper.h" #include "ScriptEngineLoggingV8.h" -//V8TODO: is this needed for anything? It could cause trouble with multithreading if V8ScriptContext is v8::Persistent -//Q_DECLARE_METATYPE(V8ScriptContext*) - Q_DECLARE_METATYPE(ScriptValue) -//V8TODO: default constructor would be needed -//Q_DECLARE_METATYPE(V8ScriptValue) Q_DECLARE_METATYPE(QSharedPointer) Q_DECLARE_METATYPE(QSharedPointer) @@ -76,7 +71,6 @@ private: // storage ScriptObjectV8Proxy::ScriptObjectV8Proxy(ScriptEngineV8* engine, QObject* object, bool ownsObject, const ScriptEngine::QObjectWrapOptions& options) : _engine(engine), _wrapOptions(options), _ownsObject(ownsObject), _object(object) { - //_v8ObjectTemplate(engine->getIsolate(), v8::ObjectTemplate::New(engine->getIsolate()) Q_ASSERT(_engine != nullptr); investigate(); } @@ -90,11 +84,9 @@ V8ScriptValue ScriptObjectV8Proxy::newQObject(ScriptEngineV8* engine, QObject* o ScriptEngineV8::ObjectWrapperMap::const_iterator lookup = engine->_qobjectWrapperMap.find(object); if (lookup != engine->_qobjectWrapperMap.end()) { QSharedPointer proxy = lookup.value().lock(); - // V8TODO: is conversion to QVariant and back needed? if (proxy) { return V8ScriptValue(engine, proxy.get()->toV8Value()); } - //if (proxy) return engine->newObject(proxy.get(), engine->newVariant(QVariant::fromValue(proxy)));; } } @@ -125,12 +117,10 @@ V8ScriptValue ScriptObjectV8Proxy::newQObject(ScriptEngineV8* engine, QObject* o ScriptEngineV8::ObjectWrapperMap::const_iterator lookup = engine->_qobjectWrapperMap.find(object); if (lookup != engine->_qobjectWrapperMap.end()) { QSharedPointer proxy = lookup.value().lock(); - //if (proxy) return qengine->newObject(proxy.get(), qengine->newVariant(QVariant::fromValue(proxy)));; if (proxy) { return V8ScriptValue(engine, proxy.get()->toV8Value()); } } - // V8TODO add a V8 callback that removes pointer for the script engine owned ob from the map so that it gets deleted // register the wrapper with the engine and make sure it cleans itself up engine->_qobjectWrapperMap.insert(object, proxy); engine->_qobjectWrapperMapV8.insert(object, proxy); @@ -146,13 +136,10 @@ V8ScriptValue ScriptObjectV8Proxy::newQObject(ScriptEngineV8* engine, QObject* o if (lookupV8 != enginePtr->_qobjectWrapperMapV8.end()) { enginePtr->_qobjectWrapperMapV8.erase(lookupV8); } - //qDebug() << "ScriptObjectV8Proxy::newQObject object deleted, object count: " << enginePtr->_qobjectWrapperMapV8.size(); }); - //qDebug() << "ScriptObjectV8Proxy::newQObject object count: " << engine->_qobjectWrapperMapV8.size(); } return V8ScriptValue(engine, proxy.get()->toV8Value()); - //return qengine->newObject(proxy.get(), qengine->newVariant(QVariant::fromValue(proxy))); } ScriptObjectV8Proxy* ScriptObjectV8Proxy::unwrapProxy(const V8ScriptValue& val) { @@ -162,20 +149,17 @@ ScriptObjectV8Proxy* ScriptObjectV8Proxy::unwrapProxy(const V8ScriptValue& val) v8::HandleScope handleScope(isolate); v8::Local context = val.constGetContext(); v8::Context::Scope contextScope(context); - //V8TODO This shouldn't cause problems but I'm not sure if it's ok - //v8::HandleScope handleScope(const_cast(val.constGetIsolate())); auto v8Value = val.constGet(); Q_ASSERT(!v8Value.IsEmpty()); + if (v8Value->IsNullOrUndefined()) { return nullptr; } if (!v8Value->IsObject()) { - //qCDebug(scriptengine_v8) << "Cannot unwrap proxy - value is not an object"; return nullptr; } v8::Local v8Object = v8::Local::Cast(v8Value); if (v8Object->InternalFieldCount() != 3) { - //qCDebug(scriptengine_v8) << "Cannot unwrap proxy - wrong number of internal fields"; return nullptr; } if (v8Object->GetAlignedPointerFromInternalField(0) != internalPointsToQObjectProxy) { @@ -189,20 +173,16 @@ ScriptObjectV8Proxy* ScriptObjectV8Proxy::unwrapProxy(v8::Isolate* isolate, v8:: v8::Locker locker(isolate); v8::Isolate::Scope isolateScope(isolate); v8::HandleScope handleScope(isolate); - //V8TODO: shouldn't there be context scope here? - //v8::Local context = val.constGetContext(); - //v8::Context::Scope contextScope(context); + //V8TODO: should there be context scope here? + if (value->IsNullOrUndefined()) { - //qCDebug(scriptengine_v8) << "Cannot unwrap proxy - value is not an object"; return nullptr; } if (!value->IsObject()) { - //qCDebug(scriptengine_v8) << "Cannot unwrap proxy - value is not an object"; return nullptr; } v8::Local v8Object = v8::Local::Cast(value); if (v8Object->InternalFieldCount() != 3) { - //qCDebug(scriptengine_v8) << "Cannot unwrap proxy - wrong number of internal fields"; return nullptr; } if (v8Object->GetAlignedPointerFromInternalField(0) != internalPointsToQObjectProxy) { @@ -257,13 +237,8 @@ void ScriptObjectV8Proxy::investigate() { const QMetaObject* metaObject = qobject->metaObject(); - //auto objectTemplate = _v8ObjectTemplate.Get(_engine->getIsolate()); auto objectTemplate = _engine->getObjectProxyTemplate(); - /*auto objectTemplate = v8::ObjectTemplate::New(_engine->getIsolate()); - objectTemplate->SetInternalFieldCount(3); - objectTemplate->SetHandler(v8::NamedPropertyHandlerConfiguration(v8Get, v8Set, nullptr, nullptr, v8GetPropertyNames));*/ - //qCDebug(scriptengine_v8) << "Investigate: " << metaObject->className(); if (QString("ConsoleScriptingInterface") == metaObject->className()) { printf("ConsoleScriptingInterface"); } @@ -274,7 +249,6 @@ void ScriptObjectV8Proxy::investigate() { QMetaProperty prop = metaObject->property(idx); if (!prop.isScriptable()) continue; - //qCDebug(scriptengine_v8) << "Investigate: " << metaObject->className() << " Property: " << prop.name(); // always exclude child objects (at least until we decide otherwise) int metaTypeId = prop.userType(); if (metaTypeId != QMetaType::UnknownType) { @@ -284,7 +258,6 @@ void ScriptObjectV8Proxy::investigate() { } } - //auto v8Name = v8::String::NewFromUtf8(_engine->getIsolate(), prop.name()).ToLocalChecked(); PropertyDef& propDef = _props.insert(idx, PropertyDef(prop.name(), idx)).value(); _propNameMap.insert(prop.name(), &propDef); propDef.flags = ScriptValue::Undeletable | ScriptValue::PropertyGetter | ScriptValue::PropertySetter | @@ -298,7 +271,6 @@ void ScriptObjectV8Proxy::investigate() { QHash methodNames; for (int idx = startIdx; idx < num; ++idx) { QMetaMethod method = metaObject->method(idx); - //qCDebug(scriptengine_v8) << "Investigate: " << metaObject->className() << " Method: " << method.name(); // perhaps keep this comment? Calls (like AudioScriptingInterface::playSound) seem to expect non-public methods to be script-accessible /* if (method.access() != QMetaMethod::Public) continue;*/ @@ -333,9 +305,6 @@ void ScriptObjectV8Proxy::investigate() { signalDef.name = szName; signalDef.signal = method; _signalNameMap.insert(szName, &signalDef); - //qCDebug(scriptengine_v8) << "Utf8Value 1: " << QString(*v8::String::Utf8Value(const_cast(_engine->getIsolate()), nameString)); - //qCDebug(scriptengine_v8) << "Utf8Value 2: " << QString(*v8::String::Utf8Value(const_cast(_engine->getIsolate()), name.constGet())); - //qCDebug(scriptengine_v8) << "toQString: " << name.toQString(); methodNames.insert(szName, idx); } else { int originalMethodId = nameLookup.value(); @@ -376,10 +345,6 @@ void ScriptObjectV8Proxy::investigate() { } v8::Local v8Object = objectTemplate->NewInstance(_engine->getContext()).ToLocalChecked(); - /*if (QString(metaObject->className()) == QString("TestQObject")) { - //qDebug() << "TestQObject investigate: _methods.size: " << _methods.size(); - return; - }*/ v8Object->SetAlignedPointerInInternalField(0, const_cast(internalPointsToQObjectProxy)); v8Object->SetAlignedPointerInInternalField(1, reinterpret_cast(this)); @@ -406,9 +371,6 @@ void ScriptObjectV8Proxy::investigate() { } void ScriptObjectV8Proxy::weakHandleCallback(const v8::WeakCallbackInfo& info) { - //V8TODO: does the object need to be moved to script engine thread? - //V8TODO: why does this never get called? - //qDebug(scriptengine_v8) << "ScriptObjectV8Proxy::weakHandleCallback"; auto proxy = info.GetParameter(); proxy->_v8Object.Reset(); info.GetParameter()->_object->deleteLater(); @@ -423,41 +385,24 @@ QString ScriptObjectV8Proxy::name() const { return _object->metaObject()->className(); } -// V8TODO: check if it would be more optimal to use V8ScriptString& name or QString name ScriptObjectV8Proxy::QueryFlags ScriptObjectV8Proxy::queryProperty(const V8ScriptValue& object, const V8ScriptString& name, QueryFlags flags, uint* id) { auto isolate = _engine->getIsolate(); v8::Locker locker(isolate); v8::Isolate::Scope isolateScope(isolate); v8::HandleScope handleScope(isolate); - // V8TODO: this might be inefficient when there's large number of properties v8::Local context = _engine->getContext(); v8::Context::Scope contextScope(context); QString nameStr(*v8::String::Utf8Value(isolate, name.constGet())); // check for methods - /*for (MethodDefMap::const_iterator trans = _methods.cbegin(); trans != _methods.cend(); ++trans) { - v8::String::Utf8Value methodNameStr(isolate, trans.value().name.constGet()); - //qCDebug(scriptengine_v8) << "queryProperty : " << *nameStr << " method: " << *methodNameStr; - if (!(trans.value().name == name)) continue; - *id = trans.key() | METHOD_TYPE; - return flags & (HandlesReadAccess | HandlesWriteAccess); - }*/ MethodNameMap::const_iterator method = _methodNameMap.find(nameStr); if (method != _methodNameMap.cend()) { - //v8::String::Utf8Value methodNameStr(isolate, trans.value().name.constGet()); - //qCDebug(scriptengine_v8) << "queryProperty : " << *nameStr << " method: " << *methodNameStr; const MethodDef* methodDef = method.value(); *id = methodDef->_id | METHOD_TYPE; return flags & (HandlesReadAccess | HandlesWriteAccess); } // check for properties - /*for (PropertyDefMap::const_iterator trans = _props.cbegin(); trans != _props.cend(); ++trans) { - const PropertyDef& propDef = trans.value(); - if (!(propDef.name == name)) continue; - *id = trans.key() | PROPERTY_TYPE; - return flags & (HandlesReadAccess | HandlesWriteAccess); - }*/ PropertyNameMap::const_iterator prop = _propNameMap.find(nameStr); if (prop != _propNameMap.cend()) { const PropertyDef* propDef = prop.value(); @@ -505,9 +450,7 @@ ScriptValue::PropertyFlags ScriptObjectV8Proxy::propertyFlags(const V8ScriptValu void ScriptObjectV8Proxy::v8Get(v8::Local name, const v8::PropertyCallbackInfo& info) { v8::HandleScope handleScope(info.GetIsolate()); - //V8TODO: should there be a context scope here? v8::String::Utf8Value utf8Value(info.GetIsolate(), name); - //qCDebug(scriptengine_v8) << "Get: " << *utf8Value; v8::Local objectV8 = info.This(); ScriptObjectV8Proxy *proxy = ScriptObjectV8Proxy::unwrapProxy(info.GetIsolate(), objectV8); if (!proxy) { @@ -521,17 +464,7 @@ void ScriptObjectV8Proxy::v8Get(v8::Local name, const v8::PropertyCall Q_ASSERT(false); } v8::Local v8NameString; - /*if (name->IsString()) { - v8NameString = v8::Local::Cast(name); - } else { - if (!name->ToString(info.GetIsolate()->GetCurrentContext()).ToLocal(&v8NameString)) { - Q_ASSERT(false); - } - } - if (name->IsSymbol()) { - qCDebug(scriptengine_v8) << "ScriptObjectV8Proxy::v8Set: symbol: " + nameString.toQString(); - }*/ if (name->IsString()) { V8ScriptString nameString(proxy->_engine, v8::Local::Cast(name)); uint id; @@ -553,9 +486,7 @@ void ScriptObjectV8Proxy::v8Get(v8::Local name, const v8::PropertyCall void ScriptObjectV8Proxy::v8Set(v8::Local name, v8::Local value, const v8::PropertyCallbackInfo& info) { v8::HandleScope handleScope(info.GetIsolate()); - //V8TODO: should there be a context scope here? v8::String::Utf8Value utf8Value(info.GetIsolate(), name); - //qCDebug(scriptengine_v8) << "Set: " << *utf8Value; v8::Local objectV8 = info.This(); ScriptObjectV8Proxy *proxy = ScriptObjectV8Proxy::unwrapProxy(info.GetIsolate(), objectV8); if (!proxy) { @@ -568,18 +499,7 @@ void ScriptObjectV8Proxy::v8Set(v8::Local name, v8::Local v qCDebug(scriptengine_v8) << notStringMessage; Q_ASSERT(false); } - /*v8::Local v8NameString; - if (name->IsString()) { - v8NameString = v8::Local::Cast(name); - } else { - if (!name->ToString(info.GetIsolate()->GetCurrentContext()).ToLocal(&v8NameString)) { - Q_ASSERT(false); - } - } - if (name->IsSymbol()) { - qCDebug(scriptengine_v8) << "ScriptObjectV8Proxy::v8Set: symbol: " + nameString.toQString(); - }*/ - //V8ScriptString nameString(info.GetIsolate(), name->ToString(proxy->_engine->getContext()).ToLocalChecked()); + if (name->IsString()) { V8ScriptString nameString(proxy->_engine, v8::Local::Cast(name)); uint id; @@ -590,7 +510,7 @@ void ScriptObjectV8Proxy::v8Set(v8::Local name, v8::Local v return; } } - // V8TODO: Should it be v8::Object or v8::Local? + if (info.This()->GetInternalField(2).As()->Set(proxy->_engine->getContext(), name, value).FromMaybe(false)) { info.GetReturnValue().Set(value); } else { @@ -599,7 +519,6 @@ void ScriptObjectV8Proxy::v8Set(v8::Local name, v8::Local v } void ScriptObjectV8Proxy::v8GetPropertyNames(const v8::PropertyCallbackInfo& info) { - //qCDebug(scriptengine_v8) << "ScriptObjectV8Proxy::v8GetPropertyNames called"; v8::HandleScope handleScope(info.GetIsolate()); auto context = info.GetIsolate()->GetCurrentContext(); v8::Context::Scope contextScope(context); @@ -610,7 +529,6 @@ void ScriptObjectV8Proxy::v8GetPropertyNames(const v8::PropertyCallbackInfo_engine, objectV8); - //uint id; v8::Local properties = proxy->getPropertyNames(); v8::Local objectProperties; uint32_t propertiesLength = properties->Length(); @@ -695,7 +613,6 @@ V8ScriptValue ScriptObjectV8Proxy::property(const V8ScriptValue& object, const V qCDebug(scriptengine_v8) << "Method with QMetaType::UnknownType " << metaObject->className() << " " << (*iter).name(); } } //V8TODO: is new method created during every call? It needs to be cached instead - //bool isMethodDefined = false; v8::Local property; if(_v8Object.Get(isolate)->GetInternalField(2).As()->Get(_engine->getContext(), name.constGet()).ToLocal(&property)) { if (!property->IsUndefined()) { @@ -725,7 +642,6 @@ V8ScriptValue ScriptObjectV8Proxy::property(const V8ScriptValue& object, const V // It's not necessarily new, newQObject looks for it first in object wrapper map // V8TODO: won't ScriptEngine::ScriptOwnership cause trouble here? return ScriptObjectV8Proxy::newQObject(_engine, proxy, ScriptEngine::ScriptOwnership, options); - //return _engine->newQObject(proxy, ScriptEngine::ScriptOwnership, options); } } return V8ScriptValue(_engine, v8::Null(isolate)); @@ -777,8 +693,6 @@ ScriptVariantV8Proxy::ScriptVariantV8Proxy(ScriptEngineV8* engine, const QVarian v8::HandleScope handleScope(isolate); v8::Context::Scope contextScope(engine->getContext()); auto variantDataTemplate = _engine->getVariantDataTemplate(); - //auto variantDataTemplate = v8::ObjectTemplate::New(isolate); - //variantDataTemplate->SetInternalFieldCount(2); auto variantData = variantDataTemplate->NewInstance(engine->getContext()).ToLocalChecked(); variantData->SetAlignedPointerInInternalField(0, const_cast(internalPointsToQVariantInProxy)); // Internal field doesn't point directly to QVariant, because then alignment would need to be guaranteed in all compilers @@ -793,7 +707,6 @@ ScriptVariantV8Proxy::~ScriptVariantV8Proxy() { v8::Isolate::Scope isolateScope(isolate); v8::HandleScope handleScope(isolate); // V8TODO: Add similar deletion handling as for object proxy - //_v8ObjectTemplate.Reset(); _v8Object.Reset(); } @@ -814,9 +727,6 @@ V8ScriptValue ScriptVariantV8Proxy::newVariant(ScriptEngineV8* engine, const QVa auto proxy = new ScriptVariantV8Proxy(engine, variant, proto, protoProxy); auto variantProxyTemplate = engine->getVariantProxyTemplate(); - //auto variantProxyTemplate = v8::ObjectTemplate::New(isolate); - //variantProxyTemplate->SetInternalFieldCount(2); - //variantProxyTemplate->SetHandler(v8::NamedPropertyHandlerConfiguration(v8Get, v8Set, nullptr, nullptr, v8GetPropertyNames)); auto variantProxy = variantProxyTemplate->NewInstance(engine->getContext()).ToLocalChecked(); variantProxy->SetAlignedPointerInInternalField(0, const_cast(internalPointsToQVariantProxy)); variantProxy->SetAlignedPointerInInternalField(1, reinterpret_cast(proxy)); @@ -905,13 +815,6 @@ void ScriptVariantV8Proxy::v8Get(v8::Local name, const v8::PropertyCal } qCDebug(scriptengine_v8) << "Value not found: " << *utf8Name; - // V8TODO: this is done differently for variant proxy - use internal field of _v8Object instead? - /*v8::Local property; - if(info.This()->GetInternalField(2).As()->Get(proxy->_engine->getContext(), name).ToLocal(&property)) { - info.GetReturnValue().Set(property); - } else { - qCDebug(scriptengine_v8) << "Value not found: " << *utf8Value; - }*/ } void ScriptVariantV8Proxy::v8Set(v8::Local name, v8::Local value, const v8::PropertyCallbackInfo& info) { @@ -941,18 +844,11 @@ void ScriptVariantV8Proxy::v8Set(v8::Local name, v8::Local return; } } - // V8TODO: this is done differently for variant proxy - use internal field of _v8Object instead? - /*if (info.This()->GetInternalField(2).As()->Set(proxy->_engine->getContext(), name, value).FromMaybe(false)) { - info.GetReturnValue().Set(value); - } else { - qCDebug(scriptengine_v8) << "Set failed: " << *utf8Name; - }*/ qCDebug(scriptengine_v8) << "Set failed: " << *utf8Name; } void ScriptVariantV8Proxy::v8GetPropertyNames(const v8::PropertyCallbackInfo& info) { //V8TODO: Only methods from the prototype should be listed. - //qCDebug(scriptengine_v8) << "ScriptObjectV8Proxy::v8GetPropertyNames called"; v8::HandleScope handleScope(info.GetIsolate()); auto context = info.GetIsolate()->GetCurrentContext(); v8::Context::Scope contextScope(context); @@ -965,15 +861,6 @@ void ScriptVariantV8Proxy::v8GetPropertyNames(const v8::PropertyCallbackInfo_engine, objectV8); v8::Local properties = proxy->_proto->getPropertyNames(); v8::Local objectProperties; - // V8TODO: this is done differently for variant proxy - use internal field of _v8Object instead? - /*uint32_t propertiesLength = properties->Length(); - if (info.This()->GetInternalField(2).As()->GetPropertyNames(context).ToLocal(&objectProperties)) { - for (uint32_t n = 0; n < objectProperties->Length(); n++) { - if(!properties->Set(context, propertiesLength+n, objectProperties->Get(context, n).ToLocalChecked()).FromMaybe(false)) { - qCDebug(scriptengine_v8) << "ScriptObjectV8Proxy::v8GetPropertyNames: Cannot add member name"; - } - } - }*/ info.GetReturnValue().Set(properties); } @@ -996,7 +883,6 @@ ScriptMethodV8Proxy::ScriptMethodV8Proxy(ScriptEngineV8* engine, QObject* object } ScriptMethodV8Proxy::~ScriptMethodV8Proxy() { - //qCDebug(scriptengine_v8) << "ScriptMethodV8Proxy destroyed"; auto isolate = _engine->getIsolate(); v8::Locker locker(isolate); v8::Isolate::Scope isolateScope(isolate); @@ -1005,7 +891,6 @@ ScriptMethodV8Proxy::~ScriptMethodV8Proxy() { } void ScriptMethodV8Proxy::weakHandleCallback(const v8::WeakCallbackInfo& info) { - //qDebug(scriptengine_v8) << "ScriptMethodV8Proxy::weakHandleCallback"; auto proxy = info.GetParameter(); proxy->_objectLifetime.Reset(); info.GetParameter()->deleteLater(); @@ -1019,12 +904,8 @@ V8ScriptValue ScriptMethodV8Proxy::newMethod(ScriptEngineV8* engine, QObject* ob v8::HandleScope handleScope(isolate); v8::Context::Scope contextScope(engine->getContext()); auto methodDataTemplate = engine->getMethodDataTemplate(); - //auto methodDataTemplate = v8::ObjectTemplate::New(isolate); - //methodDataTemplate->SetInternalFieldCount(2); auto methodData = methodDataTemplate->NewInstance(engine->getContext()).ToLocalChecked(); methodData->SetAlignedPointerInInternalField(0, const_cast(internalPointsToMethodProxy)); - // V8TODO it needs to be deleted somehow on object destruction - // weak persistent callback would do this methodData->SetAlignedPointerInInternalField(1, reinterpret_cast(new ScriptMethodV8Proxy(engine, object, lifetime, metas, numMaxParams))); auto v8Function = v8::Function::New(engine->getContext(), callback, methodData, numMaxParams).ToLocalChecked(); return V8ScriptValue(engine, v8Function); @@ -1042,31 +923,16 @@ QString ScriptMethodV8Proxy::fullName() const { return QString("%1::%2").arg(_object->metaObject()->className(), firstMethod.name()); } -// V8TODO -/*bool ScriptMethodV8Proxy::supportsExtension(Extension extension) const { - switch (extension) { - case Callable: - return true; - default: - return false; - } -}*/ - void ScriptMethodV8Proxy::callback(const v8::FunctionCallbackInfo& arguments) { v8::Locker locker(arguments.GetIsolate()); v8::Isolate::Scope isolateScope(arguments.GetIsolate()); v8::HandleScope handleScope(arguments.GetIsolate()); - //Q_ASSERT(!arguments.GetIsolate()->GetCurrentContext().IsEmpty()); v8::Context::Scope contextScope(arguments.GetIsolate()->GetCurrentContext()); if (!arguments.Data()->IsObject()) { arguments.GetIsolate()->ThrowError("Method value is not an object"); return; } v8::Local data = v8::Local::Cast(arguments.Data()); - /*if (!arguments.Data()->IsCallable()) { - arguments.GetIsolate()->ThrowError("Method value is not callable"); - return; - }*/ if (data->InternalFieldCount() != 2) { arguments.GetIsolate()->ThrowError("Incorrect number of internal fields during method call"); return; @@ -1092,8 +958,6 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume return; } - //v8::HandleScope handleScope(_engine->getIsolate()); - int scriptNumArgs = arguments.Length(); int numArgs = std::min(scriptNumArgs, _numMaxParams); @@ -1113,7 +977,6 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume int bestMeta = 0; int bestConversionPenaltyScore = 0; - for (int i = 0; i < num_metas; i++) { const QMetaMethod& meta = _metas[i]; int methodNumArgs = meta.parameterCount(); @@ -1208,16 +1071,6 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume return; } else if (returnTypeId == scriptValueTypeId) { ScriptValue result; - /*if (_metas.front().name() == "createGarbageCollectorDebuggingObject") { - //qDebug() << "createGarbageCollectorDebuggingObject"; - return; - }*/ - /*const char* typeName = meta.typeName(); - QVariant qRetVal(returnTypeId, static_cast(NULL)); - QGenericReturnArgument sRetVal(typeName, const_cast(qRetVal.constData())); - bool success = meta.invoke(qobject, Qt::DirectConnection, sRetVal, qGenArgs[0], - qGenArgs[1], qGenArgs[2], qGenArgs[3], qGenArgs[4], qGenArgs[5], qGenArgs[6], - qGenArgs[7], qGenArgs[8], qGenArgs[9]);*/ bool success = meta.invoke(qobject, Qt::DirectConnection, Q_RETURN_ARG(ScriptValue, result), qGenArgs[0], qGenArgs[1], qGenArgs[2], qGenArgs[3], qGenArgs[4], qGenArgs[5], qGenArgs[6], qGenArgs[7], qGenArgs[8], qGenArgs[9]); @@ -1226,7 +1079,6 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume return; } V8ScriptValue v8Result = ScriptValueV8Wrapper::fullUnwrap(_engine, result); - //V8ScriptValue v8Result = _engine->castVariantToValue(qRetVal); arguments.GetReturnValue().Set(v8Result.get()); return; } else { @@ -1271,8 +1123,6 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume .arg(fullName()).arg(arg+1).arg(argTypeName, methodTypeName); qCDebug(scriptengine_v8) << errorMessage << "\n Backtrace:" << _engine->currentContext()->backtrace(); isolate->ThrowError(v8::String::NewFromUtf8(isolate, errorMessage.toStdString().c_str()).ToLocalChecked()); - //context->throwError(V8ScriptContext::TypeError, QString("Native call of %1 failed: Cannot convert parameter %2 from %3 to %4") - // .arg(fullName()).arg(arg+1).arg(argTypeName, methodTypeName)); return; } } @@ -1280,179 +1130,10 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume QString errorMessage = QString("Native call of %1 failed: could not locate an overload with the requested arguments").arg(fullName()); qCDebug(scriptengine_v8) << errorMessage; isolate->ThrowError(v8::String::NewFromUtf8(isolate, errorMessage.toStdString().c_str()).ToLocalChecked()); - // V8TODO: it happens sometimes for some reason Q_ASSERT(false); // really shouldn't have gotten here -- it didn't work before and it's working now? return; } -//V8TODO: was this used anywhere? -/*QVariant ScriptMethodV8Proxy::extension(Extension extension, const QVariant& argument) { - if (extension != Callable) return QVariant(); - V8ScriptContext* context = qvariant_cast(argument); - - QObject* qobject = _object; - if (!qobject) { - context->throwError(V8ScriptContext::ReferenceError, "Referencing deleted native object"); - return QVariant(); - } - - int scriptNumArgs = context->argumentCount(); - int numArgs = std::min(scriptNumArgs, _numMaxParams); - - const int scriptValueTypeId = qMetaTypeId(); - - int parameterConversionFailureId = 0; - int parameterConversionFailureCount = 0; - - int num_metas = _metas.size(); - QVector< QList > qScriptArgLists; - QVector< QVector > qGenArgsVectors; - QVector< QList > qVarArgLists; - qScriptArgLists.resize(num_metas); - qGenArgsVectors.resize(num_metas); - qVarArgLists.resize(num_metas); - bool isValidMetaSelected = false; - int bestMeta = 0; - int bestConversionPenaltyScore = 0; - - for (int i = 0; i < num_metas; i++) { - const QMetaMethod& meta = _metas[i]; - int methodNumArgs = meta.parameterCount(); - if (methodNumArgs != numArgs) { - continue; - } - - qGenArgsVectors[i].resize(10); - int conversionPenaltyScore = 0; - int conversionFailures = 0; - - for (int arg = 0; arg < numArgs; ++arg) { - int methodArgTypeId = meta.parameterType(arg); - Q_ASSERT(methodArgTypeId != QMetaType::UnknownType); - V8ScriptValue argVal = context->argument(arg); - if (methodArgTypeId == scriptValueTypeId) { - qScriptArgLists[i].append(ScriptValue(new ScriptValueV8Wrapper(_engine, argVal))); - qGenArgsVectors[i][arg] = Q_ARG(ScriptValue, qScriptArgLists[i].back()); - } else if (methodArgTypeId == QMetaType::QVariant) { - qVarArgLists[i].append(argVal.toVariant()); - qGenArgsVectors[i][arg] = Q_ARG(QVariant, qVarArgLists[i].back()); - } else { - QVariant varArgVal; - if (!_engine->castValueToVariant(argVal, varArgVal, methodArgTypeId)) { - conversionFailures++; - } else { - qVarArgLists[i].append(varArgVal); - const QVariant& converted = qVarArgLists[i].back(); - conversionPenaltyScore = _engine->computeCastPenalty(argVal, methodArgTypeId); - - // a lot of type conversion assistance thanks to https://stackoverflow.com/questions/28457819/qt-invoke-method-with-qvariant - // A const_cast is needed because calling data() would detach the QVariant. - qGenArgsVectors[i][arg] = - QGenericArgument(QMetaType::typeName(converted.userType()), const_cast(converted.constData())); - } - } - } - if (conversionFailures) { - if (conversionFailures < parameterConversionFailureCount || !parameterConversionFailureCount) { - parameterConversionFailureCount = conversionFailures; - parameterConversionFailureId = meta.methodIndex(); - } - continue; - } - - if (!isValidMetaSelected) { - isValidMetaSelected = true; - bestMeta = i; - bestConversionPenaltyScore = conversionPenaltyScore; - } - if (isValidMetaSelected && bestConversionPenaltyScore > conversionPenaltyScore) { - bestMeta = i; - bestConversionPenaltyScore = conversionPenaltyScore; - } - } - - if (isValidMetaSelected) { - ScriptContextV8Wrapper ourContext(_engine, context); - ScriptContextGuard guard(&ourContext); - const QMetaMethod& meta = _metas[bestMeta]; - int returnTypeId = meta.returnType(); - QVector &qGenArgs = qGenArgsVectors[bestMeta]; - - // The Qt MOC engine will automatically call qRegisterMetaType on invokable parameters and properties, but there's - // nothing in there for return values so these need to be explicitly runtime-registered! - Q_ASSERT(returnTypeId != QMetaType::UnknownType); - if (returnTypeId == QMetaType::UnknownType) { - _engine->getIsolate()->ThrowError(v8::String::NewFromUtf8(_engine->getIsolate(), QString("Cannot call native function %1, its return value has not been registered with Qt").arg(fullName()).toStdString().c_str()).ToLocalChecked()); - return QVariant(); - } else if (returnTypeId == QMetaType::Void) { - bool success = meta.invoke(qobject, Qt::DirectConnection, qGenArgs[0], qGenArgs[1], qGenArgs[2], qGenArgs[3], - qGenArgs[4], qGenArgs[5], qGenArgs[6], qGenArgs[7], qGenArgs[8], qGenArgs[9]); - if (!success) { - _engine->getIsolate()->ThrowError(v8::String::NewFromUtf8(_engine->getIsolate(), QString("Unexpected: Native call of %1 failed").arg(fullName()).toStdString().c_str()).ToLocalChecked()); - } - return QVariant(); - } else if (returnTypeId == scriptValueTypeId) { - ScriptValue result; - bool success = meta.invoke(qobject, Qt::DirectConnection, Q_RETURN_ARG(ScriptValue, result), qGenArgs[0], - qGenArgs[1], qGenArgs[2], qGenArgs[3], qGenArgs[4], qGenArgs[5], qGenArgs[6], - qGenArgs[7], qGenArgs[8], qGenArgs[9]); - if (!success) { - _engine->getIsolate()->ThrowError(v8::String::NewFromUtf8(_engine->getIsolate(), QString("Unexpected: Native call of %1 failed").arg(fullName()).toStdString().c_str()).ToLocalChecked()); - return QVariant(); - } - V8ScriptValue qResult = ScriptValueV8Wrapper::fullUnwrap(_engine, result); - return QVariant::fromValue(qResult); - } else { - // a lot of type conversion assistance thanks to https://stackoverflow.com/questions/28457819/qt-invoke-method-with-qvariant - const char* typeName = meta.typeName(); - QVariant qRetVal(returnTypeId, static_cast(NULL)); - QGenericReturnArgument sRetVal(typeName, const_cast(qRetVal.constData())); - - bool success = - meta.invoke(qobject, Qt::DirectConnection, sRetVal, qGenArgs[0], qGenArgs[1], qGenArgs[2], qGenArgs[3], - qGenArgs[4], qGenArgs[5], qGenArgs[6], qGenArgs[7], qGenArgs[8], qGenArgs[9]); - if (!success) { - _engine->getIsolate()->ThrowError(v8::String::NewFromUtf8(_engine->getIsolate(), QString("Unexpected: Native call of %1 failed").arg(fullName()).toStdString().c_str()).ToLocalChecked()); - return QVariant(); - } - V8ScriptValue qResult = _engine->castVariantToValue(qRetVal); - return QVariant::fromValue(qResult); - } - } - - // we failed to convert the call to C++, try to create a somewhat sane error message - if (parameterConversionFailureCount == 0) { - _engine->getIsolate()->ThrowError(v8::String::NewFromUtf8(_engine->getIsolate(), QString("Native call of %1 failed: unexpected parameter count").arg(fullName()).toStdString().c_str()).ToLocalChecked()); - return QVariant(); - } - - const QMetaMethod& meta = _object->metaObject()->method(parameterConversionFailureId); - int methodNumArgs = meta.parameterCount(); - Q_ASSERT(methodNumArgs == numArgs); - - for (int arg = 0; arg < numArgs; ++arg) { - int methodArgTypeId = meta.parameterType(arg); - Q_ASSERT(methodArgTypeId != QMetaType::UnknownType); - V8ScriptValue argVal = context->argument(arg); - if (methodArgTypeId != scriptValueTypeId && methodArgTypeId != QMetaType::QVariant) { - QVariant varArgVal; - if (!_engine->castValueToVariant(argVal, varArgVal, methodArgTypeId)) { - QByteArray methodTypeName = QMetaType(methodArgTypeId).name(); - QByteArray argTypeName = _engine->valueType(argVal).toLatin1(); - _engine->getIsolate()->ThrowError(v8::String::NewFromUtf8(_engine->getIsolate(), QString("Native call of %1 failed: Cannot convert parameter %2 from %3 to %4") - .arg(fullName()).arg(arg+1).arg(argTypeName, methodTypeName).toStdString().c_str()).ToLocalChecked()); - context->throwError(V8ScriptContext::TypeError, QString("Native call of %1 failed: Cannot convert parameter %2 from %3 to %4") - .arg(fullName()).arg(arg+1).arg(argTypeName, methodTypeName)); - return QVariant(); - } - } - } - - context->throwError(QString("Native call of %1 failed: could not locate an overload with the requested arguments").arg(fullName())); - Q_ASSERT(false); // really shouldn't have gotten here -- it didn't work before and it's working now? - return QVariant(); -}*/ - ScriptSignalV8Proxy::ScriptSignalV8Proxy(ScriptEngineV8* engine, QObject* object, V8ScriptValue lifetime, const QMetaMethod& meta) : _engine(engine), _object(object), _meta(meta), _metaCallId(discoverMetaCallIdx()) { auto isolate = _engine->getIsolate(); @@ -1475,7 +1156,6 @@ ScriptSignalV8Proxy::~ScriptSignalV8Proxy() { } void ScriptSignalV8Proxy::weakHandleCallback(const v8::WeakCallbackInfo& info) { - //qDebug(scriptengine_v8) << "ScriptSignalV8Proxy::weakHandleCallback"; auto proxy = info.GetParameter(); proxy->_objectLifetime.Reset(); proxy->deleteLater(); @@ -1512,17 +1192,6 @@ int ScriptSignalV8Proxy::qt_metacall(QMetaObject::Call call, int id, void** argu v8::Locker locker(isolate); v8::Isolate::Scope isolateScope(isolate); - //V8ScriptValueList args(isolate, v8::Null(isolate)); - // Moved to inside of the lock - could it cause crashes if left here? - /*v8::Local args[Q_METAMETHOD_INVOKE_MAX_ARGS]; - int numArgs = _meta.parameterCount(); - for (int arg = 0; arg < numArgs; ++arg) { - int methodArgTypeId = _meta.parameterType(arg); - Q_ASSERT(methodArgTypeId != QMetaType::UnknownType); - QVariant argValue(methodArgTypeId, arguments[arg+1]); - args[arg] = _engine->castVariantToValue(argValue).get(); - }*/ - QList connections; withReadLock([&]{ connections = _connections; @@ -1530,9 +1199,7 @@ int ScriptSignalV8Proxy::qt_metacall(QMetaObject::Call call, int id, void** argu // V8TODO: this may cause deadlocks on connect/disconnect, so the connect/disconnect procedure needs to be reworked. // It should probably add events to a separate list that would be processed before and after all the events for the signal. - //withReadLock([&]{ { - // V8TODO: check all other lambda functions to make sure they have handle scope - could they cause crashes otherwise? v8::HandleScope handleScope(isolate); // V8TODO: should we use function creation context, or context in which connect happened? auto context = _engine->getContext(); @@ -1545,23 +1212,9 @@ int ScriptSignalV8Proxy::qt_metacall(QMetaObject::Call call, int id, void** argu QVariant argValue(methodArgTypeId, arguments[arg + 1]); args[arg] = _engine->castVariantToValue(argValue).get(); } - //for (ConnectionList::iterator iter = _connections.begin(); iter != _connections.end(); ++iter) { for (ConnectionList::iterator iter = connections.begin(); iter != connections.end(); ++iter) { Connection& conn = *iter; { - /*if (!conn.callback.get()->IsFunction()) { - auto stringV8 = conn.callback.get()->ToDetailString(context).ToLocalChecked(); - QString error = *v8::String::Utf8Value(_engine->getIsolate(), stringV8); - qCDebug(scriptengine_v8) << error; - Q_ASSERT(false); - } - v8::Local callback = v8::Local::Cast(conn.callback.get()); - auto functionContext = callback->CreationContext(); - v8::Context::Scope functionContextScope(functionContext); - _engine->pushContext(functionContext);*/ - /*auto functionContext = _v8Context.Get(_engine->getIsolate()); - _engine->pushContext(functionContext); - v8::Context::Scope functionContextScope(functionContext);*/ auto functionContext = context; Q_ASSERT(!conn.callback.get().IsEmpty()); @@ -1595,11 +1248,9 @@ int ScriptSignalV8Proxy::qt_metacall(QMetaObject::Call call, int id, void** argu _engine->setUncaughtException(tryCatch, "Error in signal proxy"); } - //_engine->popContext(); } } } - //}); #ifdef SCRIPT_EVENT_PERFORMANCE_STATISTICS _totalCallTime_s += callTimer.elapsed() / 1000.0f; @@ -1630,10 +1281,6 @@ ScriptSignalV8Proxy::ConnectionList::iterator ScriptSignalV8Proxy::findConnectio return iterOut; } -/*void ScriptSignalV8Proxy::connect(ScriptValue arg0) { - connect(arg0, ScriptValue(_engine->getIsolate(), v8::Undefined(_engine->getIsolate()))); -}*/ - void ScriptSignalV8Proxy::connect(ScriptValue arg0, ScriptValue arg1) { v8::Isolate *isolate = _engine->getIsolate(); v8::Locker locker(isolate); @@ -1646,8 +1293,6 @@ void ScriptSignalV8Proxy::connect(ScriptValue arg0, ScriptValue arg1) { return; } - //v8::HandleScope handleScope(isolate); - // untangle the arguments V8ScriptValue callback(_engine, v8::Null(isolate)); V8ScriptValue callbackThis(_engine, v8::Null(isolate)); @@ -1697,15 +1342,12 @@ void ScriptSignalV8Proxy::connect(ScriptValue arg0, ScriptValue arg1) { ScriptSignalV8Proxy* thisProxy = dynamic_cast(ScriptObjectV8Proxy::unwrapProxy(v8ThisObject)->toQObject()); Q_ASSERT(thisProxy); qCDebug(scriptengine_v8) << "ScriptSignalV8Proxy::connect: " << thisProxy->fullName() << " fullName: " << fullName(); - //Q_ASSERT(destFunction->InternalFieldCount() == 4); - //Q_ASSERT(destData.get()->IsArray()); - //v8::Local destData = destFunction->GetInternalField(3); if (!destFunction->Get(destFunctionContext, destDataName).ToLocal(&destData)) { Q_ASSERT(false); } if (destData->IsArray()) { v8::Local destArray = v8::Local::Cast(destData); - int length = destArray->Length();//destData.property("length").toInteger(); + int length = destArray->Length(); // V8TODO: Maybe copying array is unnecessary? v8::Local newArray = v8::Array::New(isolate, length + 1); bool foundIt = false; @@ -1726,7 +1368,6 @@ void ScriptSignalV8Proxy::connect(ScriptValue arg0, ScriptValue arg1) { } } if (!newArray->Set(destFunctionContext, length, v8ThisObject.get()).FromMaybe(false)) { - //if (!newArray->Set(destFunctionContext, length, v8ThisObject.get()).FromMaybe(false)) { Q_ASSERT(false); } if (!destFunction->Set(destFunctionContext, destDataName, newArray).FromMaybe(false)) { @@ -1741,16 +1382,9 @@ void ScriptSignalV8Proxy::connect(ScriptValue arg0, ScriptValue arg1) { Q_ASSERT(false); } } - /*{ - V8ScriptValueList args; - args << thisObject(); - destData.property("push").call(destData, args); - }*/ // add this to our internal list of connections Connection newConnection(callbackThis, callback); - //newConn.callback = callback; - //newConn.thisValue = callbackThis; withWriteLock([&]{ _connections.append(newConnection); @@ -1764,10 +1398,6 @@ void ScriptSignalV8Proxy::connect(ScriptValue arg0, ScriptValue arg1) { } } -/*void ScriptSignalV8Proxy::disconnect(ScriptValue arg0) { - disconnect(arg0, V8ScriptValue(_engine->getIsolate(), v8::Undefined(_engine->getIsolate()))); -}*/ - void ScriptSignalV8Proxy::disconnect(ScriptValue arg0, ScriptValue arg1) { QObject* qobject = _object; v8::Isolate *isolate = _engine->getIsolate(); @@ -1832,18 +1462,15 @@ void ScriptSignalV8Proxy::disconnect(ScriptValue arg0, ScriptValue arg1) { ScriptSignalV8Proxy* thisProxy = dynamic_cast(ScriptObjectV8Proxy::unwrapProxy(v8ThisObject)->toQObject()); Q_ASSERT(thisProxy); qCDebug(scriptengine_v8) << "ScriptSignalV8Proxy::disconnect: " << thisProxy->fullName() << " fullName: " << fullName(); - //V8ScriptValue destData = callback.data(); - //Q_ASSERT(destData->IsArray()); if (!destFunction->Get(destFunctionContext, destDataName).ToLocal(&destData)) { Q_ASSERT(false); } if (destData->IsArray()) { v8::Local destArray = v8::Local::Cast(destData); - int length = destArray->Length();//destData.property("length").toInteger(); + int length = destArray->Length(); v8::Local newArray = v8::Array::New(isolate, length - 1); bool foundIt = false; int newIndex = 0; - //for (int idx = 0; idx < length && !foundIt; ++idx) { for (int idx = 0; idx < length; ++idx) { v8::Local entry = destArray->Get(destFunctionContext, idx).ToLocalChecked(); // For debugging: @@ -1863,9 +1490,6 @@ void ScriptSignalV8Proxy::disconnect(ScriptValue arg0, ScriptValue arg1) { //V8TODO: compare proxies instead? foundIt = true; qCDebug(scriptengine_v8) << "ScriptSignalV8Proxy::disconnect foundIt"; - //V8ScriptValueList args; - //args << idx << 1; - //destData.property("splice").call(destData, args); } else { if (!newArray->Set(destFunctionContext, newIndex, entry).FromMaybe(false)) { Q_ASSERT(false); diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h index 6f29272d4a..a1241a54f8 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.h @@ -133,11 +133,7 @@ private: // storage const bool _ownsObject; QPointer _object; // Handle for its own object - // V8TODO Maybe depending on object ownership it should be different type of handles? For example weak persistent would allow - // script engine-owned objects to be garbage collected. This will also need adding a garbage collector callback from V8 - // to let proxy know that it is not valid anymore v8::Persistent _v8Object; - int pointerCorruptionTest = 12345678; Q_DISABLE_COPY(ScriptObjectV8Proxy) }; @@ -171,7 +167,6 @@ public: // construction static QVariant* unwrapQVariantPointer(v8::Isolate* isolate, const v8::Local &value); static QVariant unwrap(const V8ScriptValue& val); inline QVariant toQVariant() const { return _variant; } - //inline QVariant toV8Value() const { return _variant; } inline v8::Local toV8Value() const { v8::EscapableHandleScope handleScope(_engine->getIsolate()); return handleScope.Escape(_v8Object.Get(_engine->getIsolate())); @@ -188,9 +183,6 @@ public: // QScriptClass implementation virtual ScriptValue::PropertyFlags propertyFlags(const V8ScriptValue& object, const V8ScriptString& name, uint id) { return _proto->propertyFlags(object, name, id); } - /*virtual QueryFlags queryProperty(const V8ScriptValue& object, const V8ScriptString& name, QueryFlags flags, uint* id) { - return _proto->queryProperty(object, name, flags, id); - }*/ virtual void setProperty(V8ScriptValue& object, const V8ScriptString& name, uint id, const V8ScriptValue& value) { return _proto->setProperty(object, name, id, value); } @@ -204,7 +196,6 @@ private: V8ScriptValue _scriptProto; ScriptObjectV8Proxy* _proto; QString _name; - //v8::UniquePersistent _v8ObjectTemplate; v8::UniquePersistent _v8Object; Q_DISABLE_COPY(ScriptVariantV8Proxy) @@ -219,10 +210,8 @@ public: // construction public: // QScriptClass implementation virtual QString name() const { return fullName(); } - //virtual bool supportsExtension(Extension extension) const; static void callback(const v8::FunctionCallbackInfo& arguments); void call(const v8::FunctionCallbackInfo& arguments); - //virtual QVariant extension(Extension extension, const QVariant& argument = QVariant()); static V8ScriptValue newMethod(ScriptEngineV8* engine, QObject* object, V8ScriptValue lifetime, const QList& metas, int numMaxParams); @@ -250,8 +239,6 @@ public: // API // arg1 was had Null default value, but that needs isolate pointer in V8 Q_INVOKABLE virtual void connect(ScriptValue arg0, ScriptValue arg1 = ScriptValue()) = 0; Q_INVOKABLE virtual void disconnect(ScriptValue arg0, ScriptValue arg1 = ScriptValue()) = 0; - //Q_INVOKABLE virtual void connect(ScriptValue arg0) = 0; - //Q_INVOKABLE virtual void disconnect(ScriptValue arg0) = 0; }; class ScriptSignalV8Proxy final : public ScriptSignalV8ProxyBase, public ReadWriteLockable { @@ -284,9 +271,6 @@ public: // API //Moved to public temporarily for debugging: QString fullName() const; - //virtual void connect(V8ScriptValue arg0) override; - //virtual void disconnect(V8ScriptValue arg0) override; - private: // storage ScriptEngineV8* _engine; diff --git a/libraries/script-engine/src/v8/ScriptProgramV8Wrapper.cpp b/libraries/script-engine/src/v8/ScriptProgramV8Wrapper.cpp index a908489a31..71a0874513 100644 --- a/libraries/script-engine/src/v8/ScriptProgramV8Wrapper.cpp +++ b/libraries/script-engine/src/v8/ScriptProgramV8Wrapper.cpp @@ -35,8 +35,6 @@ ScriptSyntaxCheckResultPointer ScriptProgramV8Wrapper::checkSyntax() { bool ScriptProgramV8Wrapper::compile() { if (_isCompiled) { - // V8TODO is there a case where source code changes later and needs to be compiled again? - // V8TODO is same program used from multiple isolates return true; } auto isolate = _engine->getIsolate(); @@ -49,7 +47,6 @@ bool ScriptProgramV8Wrapper::compile() { int errorLineNumber = 0; QString errorMessage = ""; QString errorBacktrace = ""; - //ScriptSyntaxCheckResult::State state; v8::TryCatch tryCatch(isolate); v8::ScriptOrigin scriptOrigin(isolate, v8::String::NewFromUtf8(isolate, _url.toStdString().c_str()).ToLocalChecked()); v8::Local script; @@ -77,7 +74,6 @@ bool ScriptProgramV8Wrapper::compile() { } } } - //V8TODO _compileResult = ScriptSyntaxCheckResultV8Wrapper(ScriptSyntaxCheckResult::Error, errorColumnNumber, errorLineNumber, errorMessage, errorBacktrace); return false; } diff --git a/libraries/script-engine/src/v8/ScriptProgramV8Wrapper.h b/libraries/script-engine/src/v8/ScriptProgramV8Wrapper.h index 063c23f877..f374964b61 100644 --- a/libraries/script-engine/src/v8/ScriptProgramV8Wrapper.h +++ b/libraries/script-engine/src/v8/ScriptProgramV8Wrapper.h @@ -51,10 +51,6 @@ private: // storage /// [V8] Implements ScriptProgram for V8 and translates calls for V8ScriptProgram class ScriptProgramV8Wrapper final : public ScriptProgram { public: // construction - /*inline ScriptProgramV8Wrapper(ScriptEngineV8* engine, const V8ScriptProgram& value) : - _engine(engine), _value(value) {}*/ - //inline ScriptProgramV8Wrapper(ScriptEngineV8* engine, V8ScriptProgram&& value) : - // _engine(engine), _value(std::move(value)) {} inline ScriptProgramV8Wrapper(ScriptEngineV8* engine, QString source, QString url) : _engine(engine), _source(source), _url(url), _value(_engine) { auto isolate = _engine->getIsolate(); @@ -62,10 +58,6 @@ public: // construction v8::Isolate::Scope isolateScope(isolate); v8::HandleScope handleScope(isolate); v8::Context::Scope contextScope(_engine->getContext()); - // V8TODO: In implicit copy assignment operator for 'V8ScriptValueTemplate' - // first required here /home/ksuprynowicz/overte/overte_v8/libraries/script-engine/src/v8/V8Types.h:45:5: - // warning: definition of implicit copy assignment operator for 'V8ScriptValueTemplate' is deprecated - // because it has a user-provided copy constructor _value = V8ScriptProgram(engine, v8::Local()); } static ScriptProgramV8Wrapper* unwrap(ScriptProgramPointer val); @@ -84,7 +76,6 @@ private: // storage V8ScriptProgram _value; bool _isCompiled = false; ScriptSyntaxCheckResultV8Wrapper _compileResult; - //V8TODO: how to make program run on multiple isolates? }; #endif // hifi_ScriptValueV8Wrapper_h diff --git a/libraries/script-engine/src/v8/ScriptValueIteratorV8Wrapper.cpp b/libraries/script-engine/src/v8/ScriptValueIteratorV8Wrapper.cpp index 83ccb25c53..7bfff6edd0 100644 --- a/libraries/script-engine/src/v8/ScriptValueIteratorV8Wrapper.cpp +++ b/libraries/script-engine/src/v8/ScriptValueIteratorV8Wrapper.cpp @@ -20,7 +20,6 @@ V8ScriptValueIterator::V8ScriptValueIterator(ScriptEngineV8* engine, v8::LocalgetContext()); auto context = _context.Get(isolate); v8::Context::Scope contextScope(context); @@ -83,7 +82,6 @@ V8ScriptValue V8ScriptValueIterator::value() { if (!_object.Get(isolate)->Get(context, propertyName->ToString(context).ToLocalChecked()).ToLocal(&v8Value)) { Q_ASSERT(false); } - //V8TODO: sometimes no value gets written to v8Value. This needs to be investigated and fixed. if (v8Value.IsEmpty()) { qDebug() << "V8ScriptValueIterator::value: value handle is empty for key: " << *v8::String::Utf8Value(isolate, propertyName->ToString(context).ToLocalChecked()); v8Value = v8::Undefined(isolate); @@ -93,7 +91,6 @@ V8ScriptValue V8ScriptValueIterator::value() { ScriptValue::PropertyFlags ScriptValueIteratorV8Wrapper::flags() const { //V8TODO - //return (ScriptValue::PropertyFlags)(int)_value.flags(); return ScriptValue::PropertyFlags(); } diff --git a/libraries/script-engine/src/v8/ScriptValueIteratorV8Wrapper.h b/libraries/script-engine/src/v8/ScriptValueIteratorV8Wrapper.h index 673b9e084c..99d98a6258 100644 --- a/libraries/script-engine/src/v8/ScriptValueIteratorV8Wrapper.h +++ b/libraries/script-engine/src/v8/ScriptValueIteratorV8Wrapper.h @@ -33,7 +33,6 @@ public: void next(); V8ScriptValue value(); private: - // V8TODO: maybe these should be WeakPersistent? v8::UniquePersistent _propertyNames; v8::UniquePersistent _object; v8::UniquePersistent _context; diff --git a/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp b/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp index 4b8c12abd5..0397dbc272 100644 --- a/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp +++ b/libraries/script-engine/src/v8/ScriptValueV8Wrapper.cpp @@ -21,7 +21,7 @@ void ScriptValueV8Wrapper::release() { // V8TODO: maybe add an assert to check if it happens on script engine thread? - // With v8::Locker in V8ScriptValue such requirement shouldn't be necessary but deleting on different thread can cause deadlocks sometimes + // With v8::Locker in V8ScriptValue such requirement shouldn't be necessary but could deleting it on different thread cause deadlocks sometimes? delete this; } @@ -45,7 +45,6 @@ V8ScriptValue ScriptValueV8Wrapper::fullUnwrap(const ScriptValue& value) const { ScriptValueV8Wrapper* unwrapped = unwrap(value); if (unwrapped) { if (unwrapped->engine().get() != _engine) { - //return _engine->toScriptValue(unwrapped->toVariant()); return _engine->castVariantToValue(unwrapped->toVariant()); } else { return unwrapped->toV8Value(); @@ -59,7 +58,6 @@ V8ScriptValue ScriptValueV8Wrapper::fullUnwrap(ScriptEngineV8* engine, const Scr ScriptValueV8Wrapper* unwrapped = unwrap(value); if (unwrapped) { if (unwrapped->engine().get() != engine) { - //return static_cast(engine)->toScriptValue(unwrapped->toVariant()); return engine->castVariantToValue(unwrapped->toVariant()); } else { return unwrapped->toV8Value(); @@ -78,9 +76,7 @@ ScriptValue ScriptValueV8Wrapper::call(const ScriptValue& thisObject, const Scri auto context = _engine->getContext(); v8::Context::Scope contextScope(context); V8ScriptValue v8This = fullUnwrap(thisObject); - //V8ScriptValueList qArgs; Q_ASSERT(args.length() <= Q_METAMETHOD_INVOKE_MAX_ARGS); - //V8TODO I'm not sure how else to do this since v8::Local should probably be on stack, not heap v8::Local v8Args[Q_METAMETHOD_INVOKE_MAX_ARGS]; int argIndex = 0; for (ScriptValueList::const_iterator iter = args.begin(); iter != args.end(); ++iter) { @@ -92,15 +88,10 @@ ScriptValue ScriptValueV8Wrapper::call(const ScriptValue& thisObject, const Scri v8::Local recv; if (v8This.get()->IsObject()) { recv = v8This.get(); - //qCDebug(scriptengine_v8) << "V8 This: " << _engine->scriptValueDebugDetailsV8(v8This); }else{ recv = _engine->getContext()->Global(); - //recv = v8::Null(isolate); - //qCDebug(scriptengine_v8) << "global"; } - //qCDebug(scriptengine_v8) << "V8 Call: " << *v8::String::Utf8Value(isolate, v8This.get()->TypeOf(isolate)); auto maybeResult = v8Function->Call(_engine->getContext(), recv, args.length(), v8Args); - // V8TODO: Exceptions don't seem to actually be caught here? if (tryCatch.HasCaught()) { qCDebug(scriptengine_v8) << "Function call failed: \"" << _engine->formatErrorMessageFromTryCatch(tryCatch); } @@ -126,6 +117,7 @@ ScriptValue ScriptValueV8Wrapper::call(const ScriptValue& thisObject, const Scri // V8TODO should there be a v8 try-catch here? // IsFunction check should be here probably // V8TODO I'm not sure in what format arguments are yet, backtrace will show how it is used + // V8TODO: this seems to never be used? Q_ASSERT(false); return _engine->undefinedValue(); /*v8::Local v8Function = v8::Local::Cast(_value.get()); @@ -148,7 +140,6 @@ ScriptValue ScriptValueV8Wrapper::construct(const ScriptValueList& args) { v8::HandleScope handleScope(isolate); v8::Context::Scope contextScope(_engine->getContext()); Q_ASSERT(args.length() <= Q_METAMETHOD_INVOKE_MAX_ARGS); - //V8TODO I'm not sure how else to do this since v8::Local should probably be on stack, not heap v8::Local v8Args[Q_METAMETHOD_INVOKE_MAX_ARGS]; int argIndex = 0; for (ScriptValueList::const_iterator iter = args.begin(); iter != args.end(); ++iter) { @@ -174,6 +165,7 @@ ScriptValue ScriptValueV8Wrapper::construct(const ScriptValueList& args) { } } +// V8TODO: this seems to never be used? ScriptValue ScriptValueV8Wrapper::construct(const ScriptValue& arguments) { auto isolate = _engine->getIsolate(); v8::Locker locker(isolate); @@ -287,7 +279,6 @@ ScriptValue ScriptValueV8Wrapper::property(const QString& name, const ScriptValu const v8::Local object = v8::Local::Cast(_value.constGet()); //V8TODO: Which context? if (object->Get(_engine->getContext(), key).ToLocal(&resultLocal)) { - //if (object->Get(_value.constGetContext(), key).ToLocal(&resultLocal)) { V8ScriptValue result(_engine, resultLocal); return ScriptValue(new ScriptValueV8Wrapper(_engine, std::move(result))); } else { @@ -302,15 +293,7 @@ ScriptValue ScriptValueV8Wrapper::property(const QString& name, const ScriptValu if (name == QString("x")) { printf("x"); } - //This displays too many messages during correct operation, but is useful for debugging - //qCDebug(scriptengine_v8) << "Failed to get property, parent of value: " << name << " is not a V8 object, reported type: " << QString(*v8::String::Utf8Value(isolate, _value.constGet()->TypeOf(isolate))); - //qCDebug(scriptengine_v8) << "Backtrace: " << _engine->currentContext()->backtrace(); return _engine->undefinedValue(); - /*v8::Local nullValue = v8::Null(_engine->getIsolate()); - V8ScriptValue nullScriptValue(_engine->getIsolate(), std::move(nullValue)); - return ScriptValue(new ScriptValueV8Wrapper(_engine, nullScriptValue));*/ - //V8ScriptValue result = _value.property(name, (V8ScriptValue::ResolveFlags)(int)mode); - //return ScriptValue(new ScriptValueV8Wrapper(_engine, std::move(result))); } ScriptValue ScriptValueV8Wrapper::property(quint32 arrayIndex, const ScriptValue::ResolveFlags& mode) const { @@ -343,7 +326,6 @@ void ScriptValueV8Wrapper::setData(const ScriptValue& value) { v8::HandleScope handleScope(isolate); v8::Context::Scope contextScope(_engine->getContext()); V8ScriptValue unwrapped = fullUnwrap(value); - // V8TODO Check if it uses same isolate. Would pointer check be enough? // Private properties are an experimental feature for now on V8, so we are using regular value for now if (_value.constGet()->IsNullOrUndefined()) { qCDebug(scriptengine_v8) << "ScriptValueV8Wrapper::setData() was called on a value that is null or undefined"; @@ -377,9 +359,6 @@ void ScriptValueV8Wrapper::setProperty(const QString& name, const ScriptValue& v v8::Local key = v8::String::NewFromUtf8(isolate, name.toStdString().c_str(),v8::NewStringType::kNormal).ToLocalChecked(); Q_ASSERT(_value.get()->IsObject()); auto object = v8::Local::Cast(_value.get()); - // V8TODO: What if value context and current context is different? - //v8::Maybe retVal = object->Set(_value.getContext(), key, unwrapped.constGet()); - //v8::Maybe retVal = object->Set(_engine->getContext(), key, unwrapped.constGet()); v8::Maybe retVal = object->Set(isolate->GetCurrentContext(), key, unwrapped.constGet()); if (retVal.IsJust() ? !retVal.FromJust() : true){ qCDebug(scriptengine_v8) << "Failed to set property"; @@ -413,7 +392,6 @@ void ScriptValueV8Wrapper::setProperty(quint32 arrayIndex, const ScriptValue& va auto object = v8::Local::Cast(_value.get()); //V8TODO: I don't know which context to use here v8::Maybe retVal(object->Set(_engine->getContext(), arrayIndex, unwrapped.constGet())); - //v8::Maybe retVal(object->Set(_value.getContext(), arrayIndex, unwrapped.constGet())); if (retVal.IsJust() ? !retVal.FromJust() : true){ qCDebug(scriptengine_v8) << "Failed to set property"; } @@ -440,7 +418,6 @@ void ScriptValueV8Wrapper::setPrototype(const ScriptValue& prototype) { auto object = v8::Local::Cast(_value.get()); //V8TODO: I don't know which context to use here v8::Maybe retVal = object->SetPrototype(context, unwrappedPrototype->toV8Value().constGet()); - //v8::Maybe retVal = object->SetPrototype(_value.getContext(), unwrappedPrototype->toV8Value().constGet()); if (retVal.IsJust() ? !retVal.FromJust() : true){ qCDebug(scriptengine_v8) << "Failed to assign prototype"; } @@ -604,23 +581,15 @@ bool ScriptValueV8Wrapper::equals(const ScriptValue& other) const { v8::HandleScope handleScope(isolate); v8::Context::Scope contextScope(_engine->getContext()); ScriptValueV8Wrapper* unwrappedOther = unwrap(other); - //V8TODO: is this used with different isolates? - // in such case conversion will probably be necessary Q_ASSERT(_engine->getIsolate() == unwrappedOther->_engine->getIsolate()); if (!unwrappedOther) { return false; }else{ - // V8TODO: which context needs to be used here? if (_value.constGet()->Equals(_engine->getContext(), unwrappedOther->toV8Value().constGet()).IsNothing()) { return false; } else { return _value.constGet()->Equals(_engine->getContext(), unwrappedOther->toV8Value().constGet()).FromJust(); } - /*if (_value.constGet()->Equals(unwrappedOther->toV8Value().constGetContext(), unwrappedOther->toV8Value().constGet()).IsNothing()) { - return false; - } else { - return _value.constGet()->Equals(unwrappedOther->toV8Value().constGetContext(), unwrappedOther->toV8Value().constGet()).FromJust(); - }*/ } } @@ -719,7 +688,7 @@ bool ScriptValueV8Wrapper::isValid() const { } bool ScriptValueV8Wrapper::isVariant() const { - //V8TODO + //V8TODO: check if it's variant proxy? I'm not sure though. auto isolate = _engine->getIsolate(); v8::Locker locker(isolate); v8::Isolate::Scope isolateScope(isolate); diff --git a/libraries/script-engine/src/v8/TypedArrayPrototype.cpp b/libraries/script-engine/src/v8/TypedArrayPrototype.cpp index 9a6f199cb0..08eaf059f2 100644 --- a/libraries/script-engine/src/v8/TypedArrayPrototype.cpp +++ b/libraries/script-engine/src/v8/TypedArrayPrototype.cpp @@ -17,7 +17,7 @@ #include "TypedArrays.h" -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*Q_DECLARE_METATYPE(QByteArray*) TypedArrayPrototype::TypedArrayPrototype(QObject* parent) : QObject(parent) { diff --git a/libraries/script-engine/src/v8/TypedArrayPrototype.h b/libraries/script-engine/src/v8/TypedArrayPrototype.h index d0418f2066..3c19935f0c 100644 --- a/libraries/script-engine/src/v8/TypedArrayPrototype.h +++ b/libraries/script-engine/src/v8/TypedArrayPrototype.h @@ -22,7 +22,7 @@ #include "V8Types.h" #include "../Scriptable.h" -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /// [V8] The javascript functions associated with a TypedArray instance prototype /*class TypedArrayPrototype : public QObject, public Scriptable { Q_OBJECT diff --git a/libraries/script-engine/src/v8/TypedArrays.cpp b/libraries/script-engine/src/v8/TypedArrays.cpp index 5de23ef7b4..ba7e0fe8cc 100644 --- a/libraries/script-engine/src/v8/TypedArrays.cpp +++ b/libraries/script-engine/src/v8/TypedArrays.cpp @@ -23,7 +23,7 @@ #include "ScriptEngineV8.h" #include "TypedArrayPrototype.h" -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*Q_DECLARE_METATYPE(QByteArray*) TypedArray::TypedArray(ScriptEngineV8* scriptEngine, QString name) : ArrayBufferViewClass(scriptEngine) { diff --git a/libraries/script-engine/src/v8/TypedArrays.h b/libraries/script-engine/src/v8/TypedArrays.h index 38787eb46c..4350eefe58 100644 --- a/libraries/script-engine/src/v8/TypedArrays.h +++ b/libraries/script-engine/src/v8/TypedArrays.h @@ -17,7 +17,7 @@ #ifndef hifi_TypedArrays_h #define hifi_TypedArrays_h -// V8TODO +// V8TODO Do not remove yet, this will be useful in later PRs /*#include "ArrayBufferViewClass.h" static const QString BYTES_PER_ELEMENT_PROPERTY_NAME = "BYTES_PER_ELEMENT"; diff --git a/libraries/script-engine/src/v8/V8Lambda.h b/libraries/script-engine/src/v8/V8Lambda.h index 52c56df627..ad38095365 100644 --- a/libraries/script-engine/src/v8/V8Lambda.h +++ b/libraries/script-engine/src/v8/V8Lambda.h @@ -22,6 +22,7 @@ // Lambda helps create callable V8ScriptValues out of std::functions: // (just meant for use from within the script engine itself) +// V8TODO: this looks like it can be safely removed? class Lambda : public QObject { Q_OBJECT public: diff --git a/libraries/script-engine/src/v8/V8Types.h b/libraries/script-engine/src/v8/V8Types.h index 2c52175c9d..499fc6ee21 100644 --- a/libraries/script-engine/src/v8/V8Types.h +++ b/libraries/script-engine/src/v8/V8Types.h @@ -25,28 +25,11 @@ class V8ScriptValueTemplate { public: V8ScriptValueTemplate() = delete; - /*enum class Ownership { - Script = 0, - Value = 1 - };*/ - //V8ScriptValueTemplate(v8::Isolate *isolate, v8::Local value) : _isolate(isolate) { - //_value.reset(v8::UniquePersistent::New(_isolate, value)); - //}; - // V8TODO: Persistent handles need to be constructed and destructed in the same thread - // Adding asserts might be a good idea V8ScriptValueTemplate(ScriptEngineV8 *engine, const v8::Local value/*, V8ScriptValueTemplate::Ownership ownership*/) : _engine(engine) { v8::Locker locker(_engine->getIsolate()); v8::Isolate::Scope isolateScope(_engine->getIsolate()); v8::HandleScope handleScope(_engine->getIsolate()); v8::Context::Scope(_engine->getContext()); - /*if (ownership == V8ScriptValueTemplate::Ownership::Script) { - _value.reset(new v8::WeakPersistent(_engine->getIsolate(), value)); - return; - } else if (ownership == V8ScriptValueTemplate::Ownership::Value) { - _value.reset(new v8::UniquePersistent(_engine->getIsolate(), value)); - return; - } - Q_ASSERT(false);*/ #ifdef OVERTE_V8_MEMORY_DEBUG _engine->incrementScriptValueCounter(); #endif @@ -60,7 +43,6 @@ public: v8::Context::Scope(_engine->getContext()); _engine = source.getEngine(); _value.reset(new v8::UniquePersistent(_engine->getIsolate(), source.constGet())); - //_value.reset(new v8::UniquePersistent(_engine->getIsolate(), source.constGet())); return *this; }; @@ -69,7 +51,6 @@ public: v8::Isolate::Scope isolateScope(_engine->getIsolate()); v8::HandleScope handleScope(_engine->getIsolate()); v8::Context::Scope(_engine->getContext()); - //_value.reset(new v8::UniquePersistent(_engine->getIsolate(), v8::Local())); #ifdef OVERTE_V8_MEMORY_DEBUG _engine->incrementScriptValueCounter(); #endif @@ -81,7 +62,6 @@ public: v8::Isolate::Scope isolateScope(_engine->getIsolate()); v8::HandleScope handleScope(_engine->getIsolate()); v8::Context::Scope(_engine->getContext()); - //_value.reset(new v8::UniquePersistent(_engine->getIsolate(), copied.constGet())); #ifdef OVERTE_V8_MEMORY_DEBUG _engine->incrementScriptValueCounter(); #endif @@ -100,31 +80,21 @@ public: return handleScope.Escape(_value.get()->Get(_engine->getIsolate())); }; - /*V8ScriptValueTemplate&& copy() const { - Q_ASSERT(_engine->getIsolate()->IsCurrent()); - v8::HandleScope handleScope(_engine->getIsolate()); - return new V8ScriptValueTemplate(_engine->getIsolate(), v8::Local::New(_engine->getIsolate(), constGet())); - };*/ - const v8::Local constGetContext() const { Q_ASSERT(_engine->getIsolate()->IsCurrent()); v8::EscapableHandleScope handleScope(_engine->getIsolate()); return handleScope.Escape(_engine->getIsolate()->GetCurrentContext()); - //return handleScope.Escape(_context.Get(_isolate)); }; const v8::Isolate* constGetIsolate() const { return _engine->getIsolate(); }; v8::Isolate* getIsolate() { return _engine->getIsolate();}; - //v8::Persistent>& getContext() { return _context;}; - ScriptEngineV8* getEngine() const { return _engine; }; v8::Local getContext() { Q_ASSERT(_engine->getIsolate()->IsCurrent()); v8::EscapableHandleScope handleScope(_engine->getIsolate()); return handleScope.Escape(_engine->getIsolate()->GetCurrentContext()); - //return handleScope.Escape(_context.Get(_isolate)); }; void reset(v8::Isolate *isolate, v8::Local) { @@ -144,19 +114,9 @@ public: private: std::shared_ptr> _value; - // V8TODO: maybe make it weak - // does it need , CopyablePersistentTraits? - // V8TODO: is context needed at all? - //v8::Isolate *_isolate; ScriptEngineV8 *_engine; - //v8::Persistent> _context; }; -//typedef V8ScriptValueTemplate V8ScriptValue; -//typedef V8ScriptValueTemplate V8ScriptProgram; -// V8TODO: Maybe weak? -//typedef v8::Persistent V8ScriptContext; - class V8ScriptString : public V8ScriptValueTemplate { public: V8ScriptString() = delete; @@ -165,7 +125,7 @@ public: v8::Locker locker(getEngine()->getIsolate()); v8::Isolate::Scope isolateScope(getEngine()->getIsolate()); v8::HandleScope handleScope(getEngine()->getIsolate()); - v8::Context::Scope(getEngine()->getContext()); + v8::Context::Scope contextScope(getEngine()->getContext()); Q_ASSERT(constGet()->IsString()); return QString(*v8::String::Utf8Value(const_cast(constGetIsolate()), constGet())); }; @@ -173,7 +133,7 @@ public: v8::Locker locker(getEngine()->getIsolate()); v8::Isolate::Scope isolateScope(getEngine()->getIsolate()); v8::HandleScope handleScope(getEngine()->getIsolate()); - v8::Context::Scope(getEngine()->getContext()); + v8::Context::Scope contextScope(getEngine()->getContext()); Q_ASSERT(constGet()->IsString()); return constGet()->StringEquals(string.constGet()); }