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

Commit

Permalink
chakrashim: Add 'Debug' object only when needed
Browse files Browse the repository at this point in the history
If `--debug` is specified, the in-built 'Debug' object is exposed by
chakra.dll that has some [APIs](https://msdn.microsoft.com/en-us/library/bs12a9wf(v=vs.94).aspx) that node-uwp relies on. With my change in #155,
I had overriden 'Debug' global object and hence certain Debug APIs stopped
working with `--debug` switch. The original intent of adding `Debug` object
was that `util.js` fetches this object in Debugging context. In absense of
`--debug` flag this object is unavailable and hence `util.js` throws TypeError.

The fix is to expose `Debug` object only in the DebugContext (called from
`util.js`). If ran with `--debug`, by the time `utill.js` code executes, engine
would have already expose in-built `Debug` object and we won't overwrite it.
Without `--debug` we would override `Debug` object.

Thanks @agarwal-sandeep for helping debugging this issue.

Fixes : #175
  • Loading branch information
kunalspathak committed Jan 20, 2016
1 parent e300485 commit d69e2c8
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 8 deletions.
5 changes: 0 additions & 5 deletions deps/chakrashim/lib/chakra_shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@
// IN THE SOFTWARE.
'use strict';

// CHAKRA-TODO doesn't implement the debugger. So add a dummy 'Debug' on
// global object for now.
Object.defineProperty(this, 'Debug',
{ value: {}, enumerable: false, configurable: false, writable: false });

(function() {
// Save original builtIns
var
Expand Down
17 changes: 17 additions & 0 deletions deps/chakrashim/src/v8debug.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,23 @@ Local<Context> Debug::GetDebugContext() {
HandleScope scope(isolate);
g_debugContext = *Context::New(isolate);
JsAddRef(g_debugContext, nullptr);

// CHAKRA-TODO: Chakra doesn't fully implement the debugger without
// --debug flag. Add a dummy 'Debug' on global object if it doesn't
// already exist.
{
Context::Scope context_scope(g_debugContext);
wchar_t * debugScript = L""
"if(this['Debug'] == undefined) { "
"Object.defineProperty(this, 'Debug', { value: {}, "
"enumerable : false, configurable : false, writable : false "
"}); "
"}";
JsValueRef result;
if (JsRunScript(debugScript, JS_SOURCE_CONTEXT_NONE, L"", &result) != JsNoError) {
return Local<Context>();
}
}
}

return static_cast<Context*>(g_debugContext);
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-cli-syntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ var syntaxArgs = [

// stderr should have a syntax error message
var match = c.stderr.match(common.engineSpecificMessage({
v8: /^SyntaxError: Unexpected identifier$/m,
chakracore: /^SyntaxError: Expected ';'$/m})
);
v8: /^SyntaxError: Unexpected identifier$/m,
chakracore: /^SyntaxError: Expected ';'$/m})
);
assert(match, 'stderr incorrect');

assert.equal(c.status, 1, 'code == ' + c.status);
Expand Down

0 comments on commit d69e2c8

Please sign in to comment.