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: #356 Reviewed-By: Jimmy Thomson <jithomso@microsoft.com>
nodejs · Aug 10, 2017 · 3e93b74 · 3e93b74
1 parent 8d74ab1
commit 3e93b74
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 29 deletions.
diff --git a/deps/chakrashim/include/v8.h b/deps/chakrashim/include/v8.h
@@ -2897,7 +2897,7 @@ bool PersistentBase<T>::IsNearDeath() const {
 
 template <class T>
 bool PersistentBase<T>::IsWeak() const {
-  return static_cast<bool>(_weakWrapper);
+  return _weakWrapper != nullptr;
 }
 
 template <class T>

diff --git 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
@@ -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
@@ -29,21 +29,15 @@
  */
 
 #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"
 
 namespace v8_inspector {
 
 using jsrt::InspectorHelpers;
 
-JavaScriptCallFrame::JavaScriptCallFrame(v8::Local<v8::Context> 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<v8::Object> JavaScriptCallFrame::details() const {
 }
 
 v8::MaybeLocal<v8::Value> JavaScriptCallFrame::evaluate(
-    v8::Local<v8::Value> expression, bool* isError) {
+    v8::Local<v8::Value> expression, bool returnByValue, bool* isError) {
   return jsrt::InspectorHelpers::EvaluateOnCallFrame(
-      m_callFrame, reinterpret_cast<JsValueRef>(*expression), false, isError);
+      m_callFrame, reinterpret_cast<JsValueRef>(*expression), returnByValue,
+      isError);
 }
 
 v8::MaybeLocal<v8::Value> JavaScriptCallFrame::restart() {

diff --git 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<JavaScriptCallFrame> create(
-      v8::Local<v8::Context> debuggerContext, JsValueRef callFrame) {
-    return wrapUnique(new JavaScriptCallFrame(debuggerContext, callFrame));
+  static std::unique_ptr<JavaScriptCallFrame> create(JsValueRef callFrame) {
+    return wrapUnique(new JavaScriptCallFrame(callFrame));
   }
   ~JavaScriptCallFrame();
 
@@ -57,18 +56,15 @@ class JavaScriptCallFrame {
   v8::Local<v8::Object> details() const;
 
   v8::MaybeLocal<v8::Value> evaluate(v8::Local<v8::Value> expression,
-                                     bool* isError);
+                                     bool returnByValue, bool* isError);
   v8::MaybeLocal<v8::Value> restart();
   v8::MaybeLocal<v8::Value> setVariableValue(int scopeNumber,
                                              v8::Local<v8::Value> variableName,
                                              v8::Local<v8::Value> newValue);
 
  private:
-  JavaScriptCallFrame(v8::Local<v8::Context> debuggerContext,
-                      JsValueRef callFrame);
+  explicit JavaScriptCallFrame(JsValueRef callFrame);
 
-  v8::Isolate* m_isolate;
-  v8::Global<v8::Context> 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
@@ -716,7 +716,8 @@ void V8DebuggerAgentImpl::evaluateOnCallFrame(
   bool isError = false;
   v8::MaybeLocal<v8::Value> 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
@@ -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
@@ -100,8 +100,8 @@ void V8RuntimeAgentImpl::evaluate(
   }
 
   v8::Local<v8::Value> 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";