From 473297046cd8eca24bada519c10c9be49fbd5301 Mon Sep 17 00:00:00 2001 From: be_ Date: Thu, 7 Apr 2016 16:46:51 -0500 Subject: [PATCH 1/7] call MIDI input script functions with appropriate "this" object --- src/controllers/controllerengine.cpp | 3 ++- src/controllers/controllerengine.h | 1 + src/controllers/midi/midicontroller.cpp | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/controllers/controllerengine.cpp b/src/controllers/controllerengine.cpp index de7aa379dae0..960830a029ec 100644 --- a/src/controllers/controllerengine.cpp +++ b/src/controllers/controllerengine.cpp @@ -379,6 +379,7 @@ bool ControllerEngine::internalExecute(QScriptValue thisObject, QScriptValue fun } bool ControllerEngine::execute(QScriptValue functionObject, + QScriptValue thisObject, unsigned char channel, unsigned char control, unsigned char value, @@ -395,7 +396,7 @@ bool ControllerEngine::execute(QScriptValue functionObject, args << QScriptValue(value); args << QScriptValue(status); args << QScriptValue(group); - return internalExecute(m_pEngine->globalObject(), functionObject, args); + return internalExecute(thisObject, functionObject, args); } bool ControllerEngine::execute(QScriptValue function, const QByteArray data, diff --git a/src/controllers/controllerengine.h b/src/controllers/controllerengine.h index 8b5a17c95d41..d9188d0da779 100644 --- a/src/controllers/controllerengine.h +++ b/src/controllers/controllerengine.h @@ -121,6 +121,7 @@ class ControllerEngine : public QObject { // Execute a basic MIDI message callback. bool execute(QScriptValue function, + QScriptValue thisObject, unsigned char channel, unsigned char control, unsigned char value, diff --git a/src/controllers/midi/midicontroller.cpp b/src/controllers/midi/midicontroller.cpp index e4d33a1beb3f..ca84ce660d21 100644 --- a/src/controllers/midi/midicontroller.cpp +++ b/src/controllers/midi/midicontroller.cpp @@ -289,7 +289,8 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, } QScriptValue function = pEngine->resolveFunction(mapping.control.item); - if (!pEngine->execute(function, channel, control, value, status, + QScriptValue thisObject = pEngine->resolveFunction(mapping.control.item.section(".", 0, -2)); + if (!pEngine->execute(function, thisObject, channel, control, value, status, mapping.control.group, timestamp)) { qDebug() << "MidiController: Invalid script function" << mapping.control.item; From 131ed5f897978448b1c660d1c1cdbe82f1094f08 Mon Sep 17 00:00:00 2001 From: be_ Date: Sun, 10 Apr 2016 14:26:00 -0500 Subject: [PATCH 2/7] Revert "call MIDI input script functions with appropriate "this" object" This reverts commit 473297046cd8eca24bada519c10c9be49fbd5301. --- src/controllers/controllerengine.cpp | 3 +-- src/controllers/controllerengine.h | 1 - src/controllers/midi/midicontroller.cpp | 3 +-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/controllers/controllerengine.cpp b/src/controllers/controllerengine.cpp index 960830a029ec..de7aa379dae0 100644 --- a/src/controllers/controllerengine.cpp +++ b/src/controllers/controllerengine.cpp @@ -379,7 +379,6 @@ bool ControllerEngine::internalExecute(QScriptValue thisObject, QScriptValue fun } bool ControllerEngine::execute(QScriptValue functionObject, - QScriptValue thisObject, unsigned char channel, unsigned char control, unsigned char value, @@ -396,7 +395,7 @@ bool ControllerEngine::execute(QScriptValue functionObject, args << QScriptValue(value); args << QScriptValue(status); args << QScriptValue(group); - return internalExecute(thisObject, functionObject, args); + return internalExecute(m_pEngine->globalObject(), functionObject, args); } bool ControllerEngine::execute(QScriptValue function, const QByteArray data, diff --git a/src/controllers/controllerengine.h b/src/controllers/controllerengine.h index d9188d0da779..8b5a17c95d41 100644 --- a/src/controllers/controllerengine.h +++ b/src/controllers/controllerengine.h @@ -121,7 +121,6 @@ class ControllerEngine : public QObject { // Execute a basic MIDI message callback. bool execute(QScriptValue function, - QScriptValue thisObject, unsigned char channel, unsigned char control, unsigned char value, diff --git a/src/controllers/midi/midicontroller.cpp b/src/controllers/midi/midicontroller.cpp index ca84ce660d21..e4d33a1beb3f 100644 --- a/src/controllers/midi/midicontroller.cpp +++ b/src/controllers/midi/midicontroller.cpp @@ -289,8 +289,7 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, } QScriptValue function = pEngine->resolveFunction(mapping.control.item); - QScriptValue thisObject = pEngine->resolveFunction(mapping.control.item.section(".", 0, -2)); - if (!pEngine->execute(function, thisObject, channel, control, value, status, + if (!pEngine->execute(function, channel, control, value, status, mapping.control.group, timestamp)) { qDebug() << "MidiController: Invalid script function" << mapping.control.item; From 0db778cc347c90db4b7b372d7fe5a34f7007df88 Mon Sep 17 00:00:00 2001 From: be_ Date: Sun, 10 Apr 2016 19:32:43 -0500 Subject: [PATCH 3/7] treat fields in MIDI mapping XML files for script functions as JS code by wrapping in anonymous functions. This resolves these bugs: https://bugs.launchpad.net/mixxx/+bug/1567203 https://bugs.launchpad.net/mixxx/+bug/1565377 --- src/controllers/controller.cpp | 2 +- src/controllers/controllerengine.cpp | 91 ++++++++++++++++--------- src/controllers/controllerengine.h | 7 +- src/controllers/midi/midicontroller.cpp | 4 +- src/test/controllerengine_test.cpp | 2 +- 5 files changed, 67 insertions(+), 39 deletions(-) diff --git a/src/controllers/controller.cpp b/src/controllers/controller.cpp index 325277e1e9d0..823ad98f0f2a 100644 --- a/src/controllers/controller.cpp +++ b/src/controllers/controller.cpp @@ -121,7 +121,7 @@ void Controller::receive(const QByteArray data, mixxx::Duration timestamp) { continue; } function.append(".incomingData"); - QScriptValue incomingData = m_pEngine->resolveFunction(function); + QScriptValue incomingData = m_pEngine->wrapFunctionCode(function); if (!m_pEngine->execute(incomingData, data, timestamp)) { qWarning() << "Controller: Invalid script function" << function; } diff --git a/src/controllers/controllerengine.cpp b/src/controllers/controllerengine.cpp index de7aa379dae0..fd08dc1ae1ef 100644 --- a/src/controllers/controllerengine.cpp +++ b/src/controllers/controllerengine.cpp @@ -98,30 +98,44 @@ void ControllerEngine::callFunctionOnObjects(QList scriptFunctionPrefix } } -/* -------- ------------------------------------------------------ -Purpose: Resolves a function name to a QScriptValue including - OBJECT.Function calls -Input: - -Output: - --------- ------------------------------------------------------ */ -QScriptValue ControllerEngine::resolveFunction(const QString& function) const { +/* ------------------------------------------------------------------ +Purpose: Turn a snippet of JS into a QScriptValue function. + Wrapping it in an anonymous function allows any JS that + evaluates to a function to be used in MIDI mapping XML files + and ensures the function is executed with the correct + 'this' object. +Input: QString snippet of JS that evaluates to a function +Output: QScriptValue of JS snippet wrapped in an anonymous function +------------------------------------------------------------------- */ +QScriptValue ControllerEngine::wrapFunctionCode(const QString& codeSnippet) { + QScriptValue wrappedFunction; + QHash::const_iterator i = - m_scriptValueCache.find(function); - if (i != m_scriptValueCache.end()) { - return i.value(); - } + m_scriptWrappedFunctionCache.find(codeSnippet); - QScriptValue object = m_pEngine->globalObject(); - QStringList parts = function.split("."); + if (i != m_scriptWrappedFunctionCache.end()) { + wrappedFunction = i.value(); + } else { + QScriptValue function = m_pEngine->evaluate(codeSnippet); + if (!syntaxIsValid(codeSnippet)) { + return m_pEngine->evaluate("(function () {})"); + } + if (checkException()) { + return m_pEngine->evaluate("(function () {})"); + } - for (int i = 0; i < parts.size(); i++) { - object = object.property(parts.at(i)); - if (!object.isValid()) { - break; + QString wrappedCode = "(function (channel, control, value, status, group) { (" + + codeSnippet + ")(channel, control, value, status, group); })"; + wrappedFunction = m_pEngine->evaluate(wrappedCode); + + if (checkException()) { + // There will be an exception if the codeSnippet didn't evaluate to a function + return m_pEngine->evaluate("(function () {})"); } + + m_scriptWrappedFunctionCache[codeSnippet] = wrappedFunction; } - m_scriptValueCache[function] = object; - return object; + return wrappedFunction; } /* -------- ------------------------------------------------------ @@ -158,8 +172,8 @@ void ControllerEngine::gracefulShutdown() { } } - // Clear the Script Value cache - m_scriptValueCache.clear(); + // Clear the cache of function wrappers + m_scriptWrappedFunctionCache.clear(); // Free all the control object threads QList keys = m_controlCache.keys(); @@ -297,19 +311,12 @@ bool ControllerEngine::evaluate(const QString& filepath) { return ret; } -/* -------- ------------------------------------------------------ -Purpose: Evaluate & run script code -Input: 'this' object if applicable, Code string -Output: false if an exception --------- ------------------------------------------------------ */ -bool ControllerEngine::internalExecute(QScriptValue thisObject, - const QString& scriptCode) { - // A special version of safeExecute since we're evaluating strings, not actual functions - // (execute() would print an error that it's not a function every time a timer fires.) - if (m_pEngine == nullptr) + +bool ControllerEngine::syntaxIsValid(const QString& scriptCode) { + if (m_pEngine == nullptr) { return false; + } - // Check syntax QScriptSyntaxCheckResult result = m_pEngine->checkSyntax(scriptCode); QString error = ""; switch (result.state()) { @@ -330,6 +337,25 @@ bool ControllerEngine::internalExecute(QScriptValue thisObject, scriptCode); scriptErrorDialog(error); + m_pEngine->clearExceptions(); + return false; + } + return true; +} + +/* -------- ------------------------------------------------------ +Purpose: Evaluate & run script code +Input: 'this' object if applicable, Code string +Output: false if an exception +-------- ------------------------------------------------------ */ +bool ControllerEngine::internalExecute(QScriptValue thisObject, + const QString& scriptCode) { + // A special version of safeExecute since we're evaluating strings, not actual functions + // (execute() would print an error that it's not a function every time a timer fires.) + if (m_pEngine == nullptr) + return false; + + if (!syntaxIsValid(scriptCode)) { return false; } @@ -441,6 +467,7 @@ bool ControllerEngine::checkException() { scriptErrorDialog(ControllerDebug::enabled() ? QString("%1\nBacktrace:\n%2") .arg(errorText, backtrace.join("\n")) : errorText); + m_pEngine->clearExceptions(); return true; } return false; diff --git a/src/controllers/controllerengine.h b/src/controllers/controllerengine.h index 8b5a17c95d41..ce8d27287ea7 100644 --- a/src/controllers/controllerengine.h +++ b/src/controllers/controllerengine.h @@ -77,8 +77,8 @@ class ControllerEngine : public QObject { m_bPopups = bPopups; } - // Resolve a function name to a QScriptValue. - QScriptValue resolveFunction(const QString& function) const; + // Wrap a snippet of JS code in an anonymous function + QScriptValue wrapFunctionCode(const QString& codeSnippet); // Look up registered script function prefixes const QList& getScriptFunctionPrefixes() { return m_scriptFunctionPrefixes; }; @@ -148,6 +148,7 @@ class ControllerEngine : public QObject { void errorDialogButton(const QString& key, QMessageBox::StandardButton button); private: + bool syntaxIsValid(const QString& scriptCode); bool evaluate(const QString& scriptName, QList scriptPaths); bool internalExecute(QScriptValue thisObject, const QString& scriptCode); bool internalExecute(QScriptValue thisObject, QScriptValue functionObject, @@ -193,7 +194,7 @@ class ControllerEngine : public QObject { QVarLengthArray m_ramp, m_brakeActive; QVarLengthArray m_scratchFilters; QHash m_scratchTimers; - mutable QHash m_scriptValueCache; + QHash m_scriptWrappedFunctionCache; // Filesystem watcher for script auto-reload QFileSystemWatcher m_scriptWatcher; QList m_lastScriptPaths; diff --git a/src/controllers/midi/midicontroller.cpp b/src/controllers/midi/midicontroller.cpp index e4d33a1beb3f..2de7a99dd136 100644 --- a/src/controllers/midi/midicontroller.cpp +++ b/src/controllers/midi/midicontroller.cpp @@ -288,7 +288,7 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, return; } - QScriptValue function = pEngine->resolveFunction(mapping.control.item); + QScriptValue function = pEngine->wrapFunctionCode(mapping.control.item); if (!pEngine->execute(function, channel, control, value, status, mapping.control.group, timestamp)) { qDebug() << "MidiController: Invalid script function" @@ -547,7 +547,7 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, if (pEngine == NULL) { return; } - QScriptValue function = pEngine->resolveFunction(mapping.control.item); + QScriptValue function = pEngine->wrapFunctionCode(mapping.control.item); if (!pEngine->execute(function, data, timestamp)) { qDebug() << "MidiController: Invalid script function" << mapping.control.item; diff --git a/src/test/controllerengine_test.cpp b/src/test/controllerengine_test.cpp index ab50cca3d5ad..9817afdfd863 100644 --- a/src/test/controllerengine_test.cpp +++ b/src/test/controllerengine_test.cpp @@ -30,7 +30,7 @@ class ControllerEngineTest : public MixxxTest { } bool execute(const QString& functionName) { - QScriptValue function = cEngine->resolveFunction(functionName); + QScriptValue function = cEngine->wrapFunctionCode(functionName); return cEngine->internalExecute(QScriptValue(), function, QScriptValueList()); } From 7ffefe02ec3c679613d0b6288bc5f1706edc4609 Mon Sep 17 00:00:00 2001 From: be_ Date: Sun, 10 Apr 2016 21:12:56 -0500 Subject: [PATCH 4/7] add numberOfArgs argument to ControllerEngine::wrapFunctionCode --- src/controllers/controller.cpp | 2 +- src/controllers/controllerengine.cpp | 17 ++++++++++++----- src/controllers/controllerengine.h | 2 +- src/controllers/midi/midicontroller.cpp | 4 ++-- src/test/controllerengine_test.cpp | 2 +- 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/controllers/controller.cpp b/src/controllers/controller.cpp index 823ad98f0f2a..a174167e61b5 100644 --- a/src/controllers/controller.cpp +++ b/src/controllers/controller.cpp @@ -121,7 +121,7 @@ void Controller::receive(const QByteArray data, mixxx::Duration timestamp) { continue; } function.append(".incomingData"); - QScriptValue incomingData = m_pEngine->wrapFunctionCode(function); + QScriptValue incomingData = m_pEngine->wrapFunctionCode(function, 2); if (!m_pEngine->execute(incomingData, data, timestamp)) { qWarning() << "Controller: Invalid script function" << function; } diff --git a/src/controllers/controllerengine.cpp b/src/controllers/controllerengine.cpp index fd08dc1ae1ef..6376b283d7e8 100644 --- a/src/controllers/controllerengine.cpp +++ b/src/controllers/controllerengine.cpp @@ -104,10 +104,12 @@ Purpose: Turn a snippet of JS into a QScriptValue function. evaluates to a function to be used in MIDI mapping XML files and ensures the function is executed with the correct 'this' object. -Input: QString snippet of JS that evaluates to a function +Input: QString snippet of JS that evaluates to a function, + int number of arguments that the function takes Output: QScriptValue of JS snippet wrapped in an anonymous function ------------------------------------------------------------------- */ -QScriptValue ControllerEngine::wrapFunctionCode(const QString& codeSnippet) { +QScriptValue ControllerEngine::wrapFunctionCode(const QString& codeSnippet, + int numberOfArgs) { QScriptValue wrappedFunction; QHash::const_iterator i = @@ -116,16 +118,21 @@ QScriptValue ControllerEngine::wrapFunctionCode(const QString& codeSnippet) { if (i != m_scriptWrappedFunctionCache.end()) { wrappedFunction = i.value(); } else { - QScriptValue function = m_pEngine->evaluate(codeSnippet); if (!syntaxIsValid(codeSnippet)) { return m_pEngine->evaluate("(function () {})"); } + QScriptValue function = m_pEngine->evaluate(codeSnippet); if (checkException()) { return m_pEngine->evaluate("(function () {})"); } - QString wrappedCode = "(function (channel, control, value, status, group) { (" + - codeSnippet + ")(channel, control, value, status, group); })"; + QStringList wrapperArgList; + for (int i = 1; i <= numberOfArgs; i++) { + wrapperArgList << QString("arg%1").arg(i); + } + QString wrapperArgs = wrapperArgList.join(","); + QString wrappedCode = "(function (" + wrapperArgs + ") { (" + + codeSnippet + ")(" + wrapperArgs + "); })"; wrappedFunction = m_pEngine->evaluate(wrappedCode); if (checkException()) { diff --git a/src/controllers/controllerengine.h b/src/controllers/controllerengine.h index ce8d27287ea7..549ae34ba951 100644 --- a/src/controllers/controllerengine.h +++ b/src/controllers/controllerengine.h @@ -78,7 +78,7 @@ class ControllerEngine : public QObject { } // Wrap a snippet of JS code in an anonymous function - QScriptValue wrapFunctionCode(const QString& codeSnippet); + QScriptValue wrapFunctionCode(const QString& codeSnippet, int numberOfArgs); // Look up registered script function prefixes const QList& getScriptFunctionPrefixes() { return m_scriptFunctionPrefixes; }; diff --git a/src/controllers/midi/midicontroller.cpp b/src/controllers/midi/midicontroller.cpp index 2de7a99dd136..5701f3d62c2b 100644 --- a/src/controllers/midi/midicontroller.cpp +++ b/src/controllers/midi/midicontroller.cpp @@ -288,7 +288,7 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, return; } - QScriptValue function = pEngine->wrapFunctionCode(mapping.control.item); + QScriptValue function = pEngine->wrapFunctionCode(mapping.control.item, 5); if (!pEngine->execute(function, channel, control, value, status, mapping.control.group, timestamp)) { qDebug() << "MidiController: Invalid script function" @@ -547,7 +547,7 @@ void MidiController::processInputMapping(const MidiInputMapping& mapping, if (pEngine == NULL) { return; } - QScriptValue function = pEngine->wrapFunctionCode(mapping.control.item); + QScriptValue function = pEngine->wrapFunctionCode(mapping.control.item, 2); if (!pEngine->execute(function, data, timestamp)) { qDebug() << "MidiController: Invalid script function" << mapping.control.item; diff --git a/src/test/controllerengine_test.cpp b/src/test/controllerengine_test.cpp index 9817afdfd863..5e52744d5164 100644 --- a/src/test/controllerengine_test.cpp +++ b/src/test/controllerengine_test.cpp @@ -30,7 +30,7 @@ class ControllerEngineTest : public MixxxTest { } bool execute(const QString& functionName) { - QScriptValue function = cEngine->wrapFunctionCode(functionName); + QScriptValue function = cEngine->wrapFunctionCode(functionName, 0); return cEngine->internalExecute(QScriptValue(), function, QScriptValueList()); } From 9d3df5171374c08b5329c3bbbeedc47de375aee7 Mon Sep 17 00:00:00 2001 From: be_ Date: Mon, 11 Apr 2016 00:54:53 -0500 Subject: [PATCH 5/7] always cache function in ControllerEngine::wrapFunctionCode Also, print error messages through JS functions --- src/controllers/controllerengine.cpp | 37 +++++++++++++--------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/controllers/controllerengine.cpp b/src/controllers/controllerengine.cpp index 6376b283d7e8..7faaa113a1ea 100644 --- a/src/controllers/controllerengine.cpp +++ b/src/controllers/controllerengine.cpp @@ -118,28 +118,25 @@ QScriptValue ControllerEngine::wrapFunctionCode(const QString& codeSnippet, if (i != m_scriptWrappedFunctionCache.end()) { wrappedFunction = i.value(); } else { - if (!syntaxIsValid(codeSnippet)) { - return m_pEngine->evaluate("(function () {})"); - } QScriptValue function = m_pEngine->evaluate(codeSnippet); - if (checkException()) { - return m_pEngine->evaluate("(function () {})"); - } - - QStringList wrapperArgList; - for (int i = 1; i <= numberOfArgs; i++) { - wrapperArgList << QString("arg%1").arg(i); - } - QString wrapperArgs = wrapperArgList.join(","); - QString wrappedCode = "(function (" + wrapperArgs + ") { (" + - codeSnippet + ")(" + wrapperArgs + "); })"; - wrappedFunction = m_pEngine->evaluate(wrappedCode); - - if (checkException()) { - // There will be an exception if the codeSnippet didn't evaluate to a function - return m_pEngine->evaluate("(function () {})"); + if (!syntaxIsValid(codeSnippet)) { + wrappedFunction = m_pEngine->evaluate( + "(function () { print('Syntax error in: " + + codeSnippet + "'); })"); + } else if (checkException()) { + wrappedFunction = m_pEngine->evaluate( + "(function () { print('Exception occured evaluating: " + + codeSnippet + "'); })"); + } else { + QStringList wrapperArgList; + for (int i = 1; i <= numberOfArgs; i++) { + wrapperArgList << QString("arg%1").arg(i); + } + QString wrapperArgs = wrapperArgList.join(","); + QString wrappedCode = "(function (" + wrapperArgs + ") { (" + + codeSnippet + ")(" + wrapperArgs + "); })"; + wrappedFunction = m_pEngine->evaluate(wrappedCode); } - m_scriptWrappedFunctionCache[codeSnippet] = wrappedFunction; } return wrappedFunction; From d49025acdcc22bad77a04830e588a84adc77b11b Mon Sep 17 00:00:00 2001 From: be_ Date: Mon, 11 Apr 2016 16:20:21 -0500 Subject: [PATCH 6/7] return QScriptValue Error objects from ControllerEngine::wrapFunctionCode() instead of returning a JS function that prints an error. This avoids having to deal with escaping JS within JS within C++. --- src/controllers/controllerengine.cpp | 30 +++++++++++++++++++--------- src/controllers/controllerengine.h | 1 + 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/controllers/controllerengine.cpp b/src/controllers/controllerengine.cpp index 7faaa113a1ea..2aea09d85af5 100644 --- a/src/controllers/controllerengine.cpp +++ b/src/controllers/controllerengine.cpp @@ -120,13 +120,9 @@ QScriptValue ControllerEngine::wrapFunctionCode(const QString& codeSnippet, } else { QScriptValue function = m_pEngine->evaluate(codeSnippet); if (!syntaxIsValid(codeSnippet)) { - wrappedFunction = m_pEngine->evaluate( - "(function () { print('Syntax error in: " - + codeSnippet + "'); })"); + wrappedFunction = m_scriptEvaluationException; } else if (checkException()) { - wrappedFunction = m_pEngine->evaluate( - "(function () { print('Exception occured evaluating: " - + codeSnippet + "'); })"); + wrappedFunction = m_scriptEvaluationException; } else { QStringList wrapperArgList; for (int i = 1; i <= numberOfArgs; i++) { @@ -315,7 +311,6 @@ bool ControllerEngine::evaluate(const QString& filepath) { return ret; } - bool ControllerEngine::syntaxIsValid(const QString& scriptCode) { if (m_pEngine == nullptr) { return false; @@ -341,7 +336,15 @@ bool ControllerEngine::syntaxIsValid(const QString& scriptCode) { scriptCode); scriptErrorDialog(error); - m_pEngine->clearExceptions(); + + if (m_pEngine->hasUncaughtException()) { + // The error message in the Error object is a generic + // "Parse error", so replace it with the more helpful + // error constructed here. + m_scriptEvaluationException = m_pEngine->uncaughtException(); + m_scriptEvaluationException.setProperty("message", error); + m_pEngine->clearExceptions(); + } return false; } return true; @@ -356,8 +359,9 @@ bool ControllerEngine::internalExecute(QScriptValue thisObject, const QString& scriptCode) { // A special version of safeExecute since we're evaluating strings, not actual functions // (execute() would print an error that it's not a function every time a timer fires.) - if (m_pEngine == nullptr) + if (m_pEngine == nullptr) { return false; + } if (!syntaxIsValid(scriptCode)) { return false; @@ -390,6 +394,12 @@ bool ControllerEngine::internalExecute(QScriptValue thisObject, QScriptValue fun return false; } + if (functionObject.isError()) { + qDebug() << "ControllerEngine::internalExecute:" + << functionObject.toString(); + return false; + } + // If it's not a function, we're done. if (!functionObject.isFunction()) { qDebug() << "ControllerEngine::internalExecute:" @@ -471,6 +481,8 @@ bool ControllerEngine::checkException() { scriptErrorDialog(ControllerDebug::enabled() ? QString("%1\nBacktrace:\n%2") .arg(errorText, backtrace.join("\n")) : errorText); + + m_scriptEvaluationException = exception; m_pEngine->clearExceptions(); return true; } diff --git a/src/controllers/controllerengine.h b/src/controllers/controllerengine.h index 549ae34ba951..99c2d94df5c3 100644 --- a/src/controllers/controllerengine.h +++ b/src/controllers/controllerengine.h @@ -195,6 +195,7 @@ class ControllerEngine : public QObject { QVarLengthArray m_scratchFilters; QHash m_scratchTimers; QHash m_scriptWrappedFunctionCache; + QScriptValue m_scriptEvaluationException; // Filesystem watcher for script auto-reload QFileSystemWatcher m_scriptWatcher; QList m_lastScriptPaths; From 66bc5dcb0f4717128603ddba599858015b73fb75 Mon Sep 17 00:00:00 2001 From: be_ Date: Tue, 12 Apr 2016 23:40:31 -0500 Subject: [PATCH 7/7] only evaluate codeSnippet once in ControllerEngine::wrapFunctionCode --- src/controllers/controllerengine.cpp | 32 +++++++--------------------- src/controllers/controllerengine.h | 1 - 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/src/controllers/controllerengine.cpp b/src/controllers/controllerengine.cpp index 2aea09d85af5..40c70281b859 100644 --- a/src/controllers/controllerengine.cpp +++ b/src/controllers/controllerengine.cpp @@ -118,21 +118,15 @@ QScriptValue ControllerEngine::wrapFunctionCode(const QString& codeSnippet, if (i != m_scriptWrappedFunctionCache.end()) { wrappedFunction = i.value(); } else { - QScriptValue function = m_pEngine->evaluate(codeSnippet); - if (!syntaxIsValid(codeSnippet)) { - wrappedFunction = m_scriptEvaluationException; - } else if (checkException()) { - wrappedFunction = m_scriptEvaluationException; - } else { - QStringList wrapperArgList; - for (int i = 1; i <= numberOfArgs; i++) { - wrapperArgList << QString("arg%1").arg(i); - } - QString wrapperArgs = wrapperArgList.join(","); - QString wrappedCode = "(function (" + wrapperArgs + ") { (" + - codeSnippet + ")(" + wrapperArgs + "); })"; - wrappedFunction = m_pEngine->evaluate(wrappedCode); + QStringList wrapperArgList; + for (int i = 1; i <= numberOfArgs; i++) { + wrapperArgList << QString("arg%1").arg(i); } + QString wrapperArgs = wrapperArgList.join(","); + QString wrappedCode = "(function (" + wrapperArgs + ") { (" + + codeSnippet + ")(" + wrapperArgs + "); })"; + wrappedFunction = m_pEngine->evaluate(wrappedCode); + checkException(); m_scriptWrappedFunctionCache[codeSnippet] = wrappedFunction; } return wrappedFunction; @@ -336,15 +330,6 @@ bool ControllerEngine::syntaxIsValid(const QString& scriptCode) { scriptCode); scriptErrorDialog(error); - - if (m_pEngine->hasUncaughtException()) { - // The error message in the Error object is a generic - // "Parse error", so replace it with the more helpful - // error constructed here. - m_scriptEvaluationException = m_pEngine->uncaughtException(); - m_scriptEvaluationException.setProperty("message", error); - m_pEngine->clearExceptions(); - } return false; } return true; @@ -482,7 +467,6 @@ bool ControllerEngine::checkException() { QString("%1\nBacktrace:\n%2") .arg(errorText, backtrace.join("\n")) : errorText); - m_scriptEvaluationException = exception; m_pEngine->clearExceptions(); return true; } diff --git a/src/controllers/controllerengine.h b/src/controllers/controllerengine.h index 99c2d94df5c3..549ae34ba951 100644 --- a/src/controllers/controllerengine.h +++ b/src/controllers/controllerengine.h @@ -195,7 +195,6 @@ class ControllerEngine : public QObject { QVarLengthArray m_scratchFilters; QHash m_scratchTimers; QHash m_scriptWrappedFunctionCache; - QScriptValue m_scriptEvaluationException; // Filesystem watcher for script auto-reload QFileSystemWatcher m_scriptWatcher; QList m_lastScriptPaths;