From 3e93b74b61015ff365d16520a8159d0ebf1f8433 Mon Sep 17 00:00:00 2001 From: Kyle Farnung Date: Wed, 2 Aug 2017 15:49:12 -0700 Subject: [PATCH] chakrashim: fix console autocomplete in VS Code The autocomplete for VS Code (and probably Chrome) sends a blob of JavaScript to the engine which enumerates/filters the properties and returns them. The code wasn't respecting the `returnByValue` parameter and VS Code was unable to get the length of the undefined `value` field in the response. While I was in there I also removed some unneeded fields from the `JavaScriptCallFrame` class and fixed a build warning (C4800) in v8.h. PR-URL: https://github.com/nodejs/node-chakracore/pull/356 Reviewed-By: Jimmy Thomson --- deps/chakrashim/include/v8.h | 2 +- deps/chakrashim/lib/chakra_inspector.js | 3 ++- deps/chakrashim/lib/chakra_shim.js | 10 ++++++---- .../src/inspector/java-script-call-frame.cc | 15 +++++---------- .../src/inspector/java-script-call-frame.h | 12 ++++-------- .../src/inspector/v8-debugger-agent-impl.cc | 3 ++- deps/chakrashim/src/inspector/v8-debugger.cc | 3 +-- .../src/inspector/v8-runtime-agent-impl.cc | 4 ++-- 8 files changed, 23 insertions(+), 29 deletions(-) diff --git a/deps/chakrashim/include/v8.h b/deps/chakrashim/include/v8.h index 1b5be278f99..e57259f9eee 100644 --- a/deps/chakrashim/include/v8.h +++ b/deps/chakrashim/include/v8.h @@ -2897,7 +2897,7 @@ bool PersistentBase::IsNearDeath() const { template bool PersistentBase::IsWeak() const { - return static_cast(_weakWrapper); + return _weakWrapper != nullptr; } template diff --git a/deps/chakrashim/lib/chakra_inspector.js b/deps/chakrashim/lib/chakra_inspector.js index 2d38db5e8ef..f36e62d3530 100644 --- a/deps/chakrashim/lib/chakra_inspector.js +++ b/deps/chakrashim/lib/chakra_inspector.js @@ -92,7 +92,8 @@ var bpLine = funcInfo.firstStatementLine + line; var bpColumn = funcInfo.firstStatementColumn + column; var v8Breakpoint = new V8Breakpoint(Debug.ScriptBreakPointType.ScriptId, - funcInfo.scriptId, scriptObj, bpLine, bpColumn); + funcInfo.scriptId, scriptObj, bpLine, + bpColumn); if (v8Breakpoint.Set()) { bpId = v8Breakpoint.Id(); } diff --git a/deps/chakrashim/lib/chakra_shim.js b/deps/chakrashim/lib/chakra_shim.js index c601fba8795..7ec253489f7 100644 --- a/deps/chakrashim/lib/chakra_shim.js +++ b/deps/chakrashim/lib/chakra_shim.js @@ -166,8 +166,8 @@ var lineNumber = fileDetails[1] ? fileDetails[1] : 0; var columnNumber = fileDetails[3] ? fileDetails[3] : 0; - errstack.push( - new StackFrame(func, funcName, fileName, lineNumber, columnNumber)); + errstack.push(new StackFrame(func, funcName, fileName, lineNumber, + columnNumber)); } return errstack; } @@ -330,7 +330,8 @@ originalMapMethods.forEach(function(pair) { Map.prototype[pair[0]] = function() { var result = pair[1].apply(this); - Object_defineProperty(result, mapIteratorProperty, + Object_defineProperty( + result, mapIteratorProperty, { value: true, enumerable: false, writable: false }); return result; }; @@ -346,7 +347,8 @@ originalSetMethods.forEach(function(pair) { Set.prototype[pair[0]] = function() { var result = pair[1].apply(this); - Object_defineProperty(result, setIteratorProperty, + Object_defineProperty( + result, setIteratorProperty, { value: true, enumerable: false, writable: false }); return result; }; diff --git a/deps/chakrashim/src/inspector/java-script-call-frame.cc b/deps/chakrashim/src/inspector/java-script-call-frame.cc index 0c597b3705e..6de0528960e 100644 --- a/deps/chakrashim/src/inspector/java-script-call-frame.cc +++ b/deps/chakrashim/src/inspector/java-script-call-frame.cc @@ -29,9 +29,6 @@ */ #include "src/inspector/java-script-call-frame.h" - -#include "src/inspector/string-util.h" -#include "include/v8-debug.h" #include "src/jsrtinspectorhelpers.h" #include "src/jsrtutils.h" @@ -39,11 +36,8 @@ namespace v8_inspector { using jsrt::InspectorHelpers; -JavaScriptCallFrame::JavaScriptCallFrame(v8::Local debuggerContext, - JsValueRef callFrame) - : m_isolate(debuggerContext->GetIsolate()), - m_debuggerContext(m_isolate, debuggerContext), - m_callFrame(callFrame) { +JavaScriptCallFrame::JavaScriptCallFrame(JsValueRef callFrame) + : m_callFrame(callFrame) { JsAddRef(m_callFrame, nullptr); } @@ -115,9 +109,10 @@ v8::Local JavaScriptCallFrame::details() const { } v8::MaybeLocal JavaScriptCallFrame::evaluate( - v8::Local expression, bool* isError) { + v8::Local expression, bool returnByValue, bool* isError) { return jsrt::InspectorHelpers::EvaluateOnCallFrame( - m_callFrame, reinterpret_cast(*expression), false, isError); + m_callFrame, reinterpret_cast(*expression), returnByValue, + isError); } v8::MaybeLocal JavaScriptCallFrame::restart() { diff --git a/deps/chakrashim/src/inspector/java-script-call-frame.h b/deps/chakrashim/src/inspector/java-script-call-frame.h index 21396c3d717..5ee47793271 100644 --- a/deps/chakrashim/src/inspector/java-script-call-frame.h +++ b/deps/chakrashim/src/inspector/java-script-call-frame.h @@ -42,9 +42,8 @@ namespace v8_inspector { class JavaScriptCallFrame { public: - static std::unique_ptr create( - v8::Local debuggerContext, JsValueRef callFrame) { - return wrapUnique(new JavaScriptCallFrame(debuggerContext, callFrame)); + static std::unique_ptr create(JsValueRef callFrame) { + return wrapUnique(new JavaScriptCallFrame(callFrame)); } ~JavaScriptCallFrame(); @@ -57,18 +56,15 @@ class JavaScriptCallFrame { v8::Local details() const; v8::MaybeLocal evaluate(v8::Local expression, - bool* isError); + bool returnByValue, bool* isError); v8::MaybeLocal restart(); v8::MaybeLocal setVariableValue(int scopeNumber, v8::Local variableName, v8::Local newValue); private: - JavaScriptCallFrame(v8::Local debuggerContext, - JsValueRef callFrame); + explicit JavaScriptCallFrame(JsValueRef callFrame); - v8::Isolate* m_isolate; - v8::Global m_debuggerContext; JsValueRef const m_callFrame; DISALLOW_COPY_AND_ASSIGN(JavaScriptCallFrame); diff --git a/deps/chakrashim/src/inspector/v8-debugger-agent-impl.cc b/deps/chakrashim/src/inspector/v8-debugger-agent-impl.cc index 05edf7f8340..c3e851f4caf 100644 --- a/deps/chakrashim/src/inspector/v8-debugger-agent-impl.cc +++ b/deps/chakrashim/src/inspector/v8-debugger-agent-impl.cc @@ -716,7 +716,8 @@ void V8DebuggerAgentImpl::evaluateOnCallFrame( bool isError = false; v8::MaybeLocal maybeResultValue = m_pausedCallFrames[ordinal]->evaluate( - toV8String(m_isolate, expression), &isError); + toV8String(m_isolate, expression), returnByValue.fromMaybe(false), + &isError); if (maybeResultValue.IsEmpty()) { *errorString = "Failed to evaluate expression"; diff --git a/deps/chakrashim/src/inspector/v8-debugger.cc b/deps/chakrashim/src/inspector/v8-debugger.cc index adf3ec592ed..946ebd0b5e6 100644 --- a/deps/chakrashim/src/inspector/v8-debugger.cc +++ b/deps/chakrashim/src/inspector/v8-debugger.cc @@ -293,8 +293,7 @@ JavaScriptCallFrames V8Debugger::currentCallFrames(int limit) { CHAKRA_VERIFY_NOERROR(jsrt::GetIndexedProperty(stackTrace, i, &callFrameValue)); - callFrames.push_back(JavaScriptCallFrame::create( - debuggerContext(), callFrameValue)); + callFrames.push_back(JavaScriptCallFrame::create(callFrameValue)); } return callFrames; diff --git a/deps/chakrashim/src/inspector/v8-runtime-agent-impl.cc b/deps/chakrashim/src/inspector/v8-runtime-agent-impl.cc index 09d91af4074..127cab4241c 100644 --- a/deps/chakrashim/src/inspector/v8-runtime-agent-impl.cc +++ b/deps/chakrashim/src/inspector/v8-runtime-agent-impl.cc @@ -100,8 +100,8 @@ void V8RuntimeAgentImpl::evaluate( } v8::Local evalResult = - jsrt::InspectorHelpers::EvaluateOnCallFrame(/* ordinal */ 0, expStr, - /* returnByValue */ true); + jsrt::InspectorHelpers::EvaluateOnCallFrame( + /* ordinal */ 0, expStr, returnByValue.fromMaybe(false)); if (evalResult.IsEmpty()) { errorString = "Failed to evaluate expression";