inspector: add inspector console support for native Console

[alexkozy]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

console

Description of change

When node is running with --inspect flag, default console.log,
console.warn and other methods call inspector console methods in
addition to current behaviour (dump formatted message to stderr
and stdout). Inspector console methods forward message to DevTools and
show its in DevTools Console with DevTools formatters. Inspector console
methods not presented on Node console will be added into it.

Only own methods on global.console object will be changed while
debugging session. User still able to redefine it, use console.Console
or change original methods on Console.prototype.

[alexkozy]

/cc @ofrobots@eugeneo

[jasnell]

@bnoordhuis@ofrobots@trevnorris ...

[eugeneo]

I would like to suggest moving this block (and 2 functions above) into inspector_agent.{h,cc} - this way there will be less conditional compilation to maintain.

[alexkozy]

/cc @dgozman

[mscdex]

Wouldn't it be possible to instead store copies of the original console functions, which would allow us to avoid having to modify console.js at all? Having to make those console.js changes just for inspector makes me cringe.

[alexkozy]

I addressed your comments and moved all changes into inspector_agent.cc and bootstrap_node.js.
Please take another look!

[dgozman]

Let's check that arguments are functions here.

[alexkozy]

Addressed all comments, please take another look!

[bnoordhuis]

Still a s/sequentally/sequentially/ typo here.

[bnoordhuis]

Nit: operator should go on previous line.

[alexkozy]

addressed comments, please take another look.

[bnoordhuis]

Nit: arguments should line up.

[bnoordhuis]

LGTM with a suggestion and a nit. CI: https://ci.nodejs.org/job/node-test-pull-request/3559/

[alexkozy]

Thanks! All done. It looks like freebsd test failure doesn't relate to this pull request.

[jasnell]

LGTM

[ofrobots]

I would prefer if inspector wasn't a public property on process (except during startup). Could you get away with deleting process.inspector here?

[alexkozy]

I can remove process.inspector here. Done.

[alexkozy]

All done. Please take another look!

[ofrobots]

LGTM. New CI: https://ci.nodejs.org/job/node-test-pull-request/3580/.

[trevnorris]

grammar nit: "then wrap these two methods..."

[trevnorris]

Also, "will preserve the original stack"

[alexkozy]

Thanks for review.
Fixed grammar.

[jasnell]

Would prefer the error message to be a bit more descriptive

[jasnell]

LGTM with a nit

[alexkozy]

Done.

[ofrobots]

One more CI: https://ci.nodejs.org/job/node-test-pull-request/3616/

[alexkozy]

This time it was failed on https://ci.nodejs.org/job/node-compile-windows/3631/label=win-vs2015/console .
I compare output with previous successful build and it looks like difference at the end of output:
c:\workspace\node-compile-windows\label\win-vs2015>exit 0
FATAL: java.io.IOException: Connection aborted: org.jenkinsci.remoting.nio.NioChannelHub$MonoNioTransport@6034828e[name=test-rackspace-win2012r2-x64-6] ...

It looks like it doesn't relate to my change, does it?

[ofrobots]

That seems to be infrastructure related. Windows did pass on the previous build. I think this is good to land. I will do so tomorrow.

[alexkozy]

I've rebased this pull request to resolve conflicts.

[ofrobots]

Thanks, landed as f6070a1.

[ofrobots]

Looks great, btw:

// ...http.get('http://www.google.com/index.html',(res)=>{console.log(res);res.resume();});