Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Commit

Permalink
chakrashim: cleanup inspector code
Browse files Browse the repository at this point in the history
* Use cached prop IDs throughout
* Use helpers throughout
* Replace nullptr with JS_INVALID_REFERENCE where appropriate
* Shorten long lines
* Cleaned up error handling

PR-URL: #287
Reviewed-By: Sandeep Agarwal <saagarwa@microsoft.com>
  • Loading branch information
kfarnung committed Jun 13, 2017
1 parent d33c32f commit d94d248
Show file tree
Hide file tree
Showing 15 changed files with 1,015 additions and 1,110 deletions.
59 changes: 23 additions & 36 deletions deps/chakrashim/src/inspector/java-script-call-frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,11 @@
*/

#include "src/inspector/java-script-call-frame.h"
#include <assert.h>

#include "src/inspector/string-util.h"

#include "include/v8-debug.h"
#include "src/jsrtinspectorhelpers.h"
#include "src/jsrtutils.h"

namespace v8_inspector {

Expand All @@ -54,30 +53,27 @@ JavaScriptCallFrame::~JavaScriptCallFrame() {

int JavaScriptCallFrame::sourceID() const {
int scriptId;
if (InspectorHelpers::GetIntProperty(m_callFrame, "scriptId", &scriptId) != JsNoError) {
assert(false);
return 0;
}
CHAKRA_VERIFY_NOERROR(jsrt::GetProperty(m_callFrame,
jsrt::CachedPropertyIdRef::scriptId,
&scriptId));

return scriptId;
}

int JavaScriptCallFrame::line() const {
int line;
if (InspectorHelpers::GetIntProperty(m_callFrame, "line", &line) != JsNoError) {
assert(false);
return 0;
}
CHAKRA_VERIFY_NOERROR(jsrt::GetProperty(m_callFrame,
jsrt::CachedPropertyIdRef::line,
&line));

return line;
}

int JavaScriptCallFrame::column() const {
int column;
if (InspectorHelpers::GetIntProperty(m_callFrame, "column", &column) != JsNoError) {
assert(false);
return 0;
}
CHAKRA_VERIFY_NOERROR(jsrt::GetProperty(m_callFrame,
jsrt::CachedPropertyIdRef::column,
&column));

return column;
}
Expand All @@ -88,37 +84,28 @@ int JavaScriptCallFrame::contextId() const {

bool JavaScriptCallFrame::isAtReturn() const {
int index;
if (InspectorHelpers::GetIntProperty(m_callFrame, "index", &index) != JsNoError) {
assert(false);
return false;
}
CHAKRA_VERIFY_NOERROR(jsrt::GetProperty(m_callFrame,
jsrt::CachedPropertyIdRef::index,
&index));

JsValueRef properties;
if (JsDiagGetStackProperties(index, &properties) != JsNoError) {
assert(false);
return false;
}
CHAKRA_VERIFY_NOERROR(JsDiagGetStackProperties(index, &properties));

bool hasProp;
if (InspectorHelpers::HasProperty(m_callFrame, "returnValue", &hasProp) != JsNoError) {
assert(false);
return false;
}
CHAKRA_VERIFY_NOERROR(jsrt::HasProperty(
m_callFrame, jsrt::CachedPropertyIdRef::returnValue, &hasProp));

if (!hasProp) {
return false;
}

JsValueRef propVal;
if (InspectorHelpers::GetProperty(m_callFrame, "returnValue", &propVal) != JsNoError) {
assert(false);
return false;
}
CHAKRA_VERIFY_NOERROR(jsrt::GetProperty(
m_callFrame, jsrt::CachedPropertyIdRef::returnValue, &propVal));

if (InspectorHelpers::HasProperty(propVal, "handle", &hasProp) != JsNoError) {
assert(false);
return false;
}
CHAKRA_VERIFY_NOERROR(jsrt::HasProperty(propVal,
jsrt::CachedPropertyIdRef::handle,
&hasProp));

return hasProp;
}
Expand All @@ -135,15 +122,15 @@ v8::MaybeLocal<v8::Value> JavaScriptCallFrame::evaluate(

v8::MaybeLocal<v8::Value> JavaScriptCallFrame::restart() {
// CHAKRA-TODO - Figure out what to do here.
assert(false);
CHAKRA_UNIMPLEMENTED();
return v8::MaybeLocal<v8::Value>();
}

v8::MaybeLocal<v8::Value> JavaScriptCallFrame::setVariableValue(
int scopeNumber, v8::Local<v8::Value> variableName,
v8::Local<v8::Value> newValue) {
// CHAKRA-TODO - Figure out what to do here.
assert(false);
CHAKRA_UNIMPLEMENTED();
return v8::MaybeLocal<v8::Value>();
}

Expand Down
6 changes: 2 additions & 4 deletions deps/chakrashim/src/inspector/v8-console-message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

#include "src/inspector/v8-console-message.h"

#include <assert.h>

#include "src/inspector/inspected-context.h"
#include "src/inspector/protocol/Protocol.h"
#include "src/inspector/string-util.h"
Expand All @@ -14,8 +12,8 @@
#include "src/inspector/v8-inspector-session-impl.h"
#include "src/inspector/v8-runtime-agent-impl.h"
#include "src/inspector/v8-stack-trace-impl.h"

#include "include/v8-inspector.h"
#include "src/jsrtutils.h"

namespace v8_inspector {

Expand Down Expand Up @@ -245,7 +243,7 @@ V8ConsoleMessage::wrapArguments(V8InspectorSessionImpl* session,

if (m_type == ConsoleAPIType::kTable && generatePreview) {
// CHAKRA-TODO - Figure out what to do here.
assert(false);
CHAKRA_UNIMPLEMENTED();
}
else {
for (size_t i = 0; i < m_arguments.size(); ++i) {
Expand Down
12 changes: 5 additions & 7 deletions deps/chakrashim/src/inspector/v8-console.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

#include "src/inspector/v8-console.h"

#include <assert.h>

#include "src/base/macros.h"
#include "src/inspector/inspected-context.h"
#include "src/inspector/string-util.h"
Expand All @@ -17,8 +15,8 @@
#include "src/inspector/v8-runtime-agent-impl.h"
#include "src/inspector/v8-stack-trace-impl.h"
#include "src/inspector/v8-value-copier.h"

#include "include/v8-inspector.h"
#include "src/jsrtutils.h"

namespace v8_inspector {

Expand Down Expand Up @@ -578,24 +576,24 @@ void V8Console::unmonitorFunctionCallback(
void V8Console::lastEvaluationResultCallback(
const v8::FunctionCallbackInfo<v8::Value>& info) {
// CHAKRA-TODO - Figure out what to do here.
assert(false);
CHAKRA_UNIMPLEMENTED();
}

void V8Console::inspectCallback(
const v8::FunctionCallbackInfo<v8::Value>& info) {
// CHAKRA-TODO - Figure out what to do here.
assert(false);
CHAKRA_UNIMPLEMENTED();
}

void V8Console::copyCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
// CHAKRA-TODO - Figure out what to do here.
assert(false);
CHAKRA_UNIMPLEMENTED();
}

void V8Console::inspectedObject(const v8::FunctionCallbackInfo<v8::Value>& info,
unsigned num) {
// CHAKRA-TODO - Figure out what to do here.
assert(false);
CHAKRA_UNIMPLEMENTED();
}

v8::Local<v8::Object> V8Console::createConsole(
Expand Down
18 changes: 9 additions & 9 deletions deps/chakrashim/src/inspector/v8-debugger-agent-impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "src/inspector/v8-debugger-agent-impl.h"

#include <algorithm>
#include <assert.h>

#include "src/inspector/java-script-call-frame.h"
#include "src/inspector/protocol/Protocol.h"
Expand All @@ -19,10 +18,10 @@
#include "src/inspector/v8-regex.h"
#include "src/inspector/v8-runtime-agent-impl.h"
#include "src/inspector/v8-stack-trace-impl.h"

#include "include/v8-inspector.h"
#include "src/jsrtinspector.h"
#include "src/jsrtinspectorhelpers.h"
#include "src/jsrtutils.h"

namespace v8_inspector {

Expand Down Expand Up @@ -256,7 +255,7 @@ void V8DebuggerAgentImpl::disable(ErrorString*) {

void V8DebuggerAgentImpl::restore() {
// CHAKRA-TODO - Figure out what to do here
assert(false);
CHAKRA_UNIMPLEMENTED();
}

void V8DebuggerAgentImpl::setBreakpointsActive(ErrorString* errorString,
Expand Down Expand Up @@ -575,7 +574,7 @@ void V8DebuggerAgentImpl::restartFrame(
std::unique_ptr<Array<CallFrame>>* newCallFrames,
Maybe<StackTrace>* asyncStackTrace) {
// CHAKRA-TODO - Figure out what to do here
assert(false);
CHAKRA_UNIMPLEMENTED();
}

void V8DebuggerAgentImpl::getScriptSource(ErrorString* error,
Expand Down Expand Up @@ -729,9 +728,10 @@ void V8DebuggerAgentImpl::evaluateOnCallFrame(
toProtocolValue(errorString, v8::Context::GetCurrent(),
maybeResultValue.ToLocalChecked());
if (!protocolValue) {
assert(false);
CHAKRA_ASSERT(false);
return;
}

std::unique_ptr<protocol::Runtime::RemoteObject> remoteObject =
protocol::Runtime::RemoteObject::parse(protocolValue.get(), &errors);
if (!remoteObject) {
Expand Down Expand Up @@ -769,7 +769,7 @@ void V8DebuggerAgentImpl::setVariableValue(
std::unique_ptr<protocol::Runtime::CallArgument> newValueArgument,
const String16& callFrameId) {
// CHAKRA-TODO - Figure out what to do here
assert(false);
CHAKRA_UNIMPLEMENTED();
}

void V8DebuggerAgentImpl::setAsyncCallStackDepth(ErrorString* errorString,
Expand Down Expand Up @@ -861,12 +861,12 @@ void V8DebuggerAgentImpl::setBlackboxedRanges(

void V8DebuggerAgentImpl::willExecuteScript(int scriptId) {
// CHAKRA-TODO - Figure out what to do here
assert(false);
CHAKRA_UNIMPLEMENTED();
}

void V8DebuggerAgentImpl::didExecuteScript() {
// CHAKRA-TODO - Figure out what to do here
assert(false);
CHAKRA_UNIMPLEMENTED();
}

std::unique_ptr<Array<CallFrame>> V8DebuggerAgentImpl::currentCallFrames(
Expand Down Expand Up @@ -1039,7 +1039,7 @@ V8DebuggerAgentImpl::SkipPauseRequest V8DebuggerAgentImpl::didPause(
protocolValue.get(), &errors);
if (remoteObject == nullptr) {
errorString = errors.errors();
assert(false);
CHAKRA_ASSERT(false);
}
}

Expand Down
22 changes: 12 additions & 10 deletions deps/chakrashim/src/inspector/v8-debugger-script.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "src/inspector/search-util.h"

#include "src/jsrtinspectorhelpers.h"
#include "src/jsrtutils.h"

namespace v8_inspector {

Expand Down Expand Up @@ -73,12 +74,12 @@ static String16 calculateHash(const String16& str) {
}

static JsErrorCode GetNamedStringValue(JsValueRef object,
const char *propName,
jsrt::CachedPropertyIdRef cachedIdRef,
String16 *value) {
JsErrorCode err = JsNoError;

JsValueRef propValue;
err = InspectorHelpers::GetProperty(object, propName, &propValue);
err = jsrt::GetProperty(object, cachedIdRef, &propValue);
if (err != JsNoError) {
return err;
}
Expand Down Expand Up @@ -111,24 +112,25 @@ V8DebuggerScript::V8DebuggerScript(v8::Isolate* isolate,
m_isLiveEdit(false) {

int scriptId = 0;
if (InspectorHelpers::GetIntProperty(
scriptData, "scriptId", &scriptId) == JsNoError) {
if (jsrt::GetProperty(scriptData, jsrt::CachedPropertyIdRef::scriptId,
&scriptId) == JsNoError) {
m_id = String16::fromInteger(scriptId);
}

int lineCount = 0;
if (InspectorHelpers::GetIntProperty(
scriptData, "lineCount", &lineCount) == JsNoError) {
if (jsrt::GetProperty(scriptData, jsrt::CachedPropertyIdRef::lineCount,
&lineCount) == JsNoError) {
m_endLine = lineCount;
}

String16 urlValue;
if (GetNamedStringValue(
scriptData, "fileName", &urlValue) == JsNoError) {
if (GetNamedStringValue(scriptData, jsrt::CachedPropertyIdRef::fileName,
&urlValue) == JsNoError) {
m_url = urlValue;
}
else if (GetNamedStringValue(
scriptData, "scriptType", &urlValue) == JsNoError) {
else if (GetNamedStringValue(scriptData,
jsrt::CachedPropertyIdRef::scriptType,
&urlValue) == JsNoError) {
m_url = urlValue;
}

Expand Down
Loading

0 comments on commit d94d248

Please sign in to comment.