Small changes to support chromium-based apps like the ones based on Electron or NW.js.

[galvesribeiro]

Just to support #18

[msftclas]

Hi @galvesribeiro, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

[galvesribeiro]

Don't know if it is the perfect solution but worked well on both Electron and NW.js... It will unblock lots of folks which aren't using VSCode for those apps due to those filters.

Those were the configs I tested with on launch.json:

{
            "name": "Electron",
            "type": "chrome",
            "request": "launch",
            "runtimeExecutable": "${workspaceRoot}/node_modules/electron-prebuilt/dist/electron.exe",
            "runtimeArgs": [
                "${workspaceRoot}",
                "--enable-logging",
                "--remote-debugging-port=9222"
            ],
            "sourceMaps": true,
            "diagnosticLogging": true
        },
        {
            "name": "NW.js",
            "type": "chrome",
            "request": "launch",
            "runtimeExecutable": "nw",
            "webRoot": "${workspaceRoot}",
            "runtimeArgs": [
                "${workspaceRoot}",
                "--mixed-context",
                "--remote-debugging-port=9222"
            ],
            "sourceMaps": true,
            "diagnosticLogging": true
        },
        {
            "name": "Attach NW.js",
            "type": "chrome",
            "request": "attach",
            "port": 9222,
            "sourceMaps": true,
            "webRoot": "${workspaceRoot}",
            "diagnosticLogging": true
        }

[roblourens]

I'd like to leave this in place because it filters out the extension targets, and sometimes we can't positively identify the page target and have to just pick. But in your example, looks like target.type === 'app', so how about adding || that? You don't need to attach to the background_script one right?

[galvesribeiro]

In case of nw.js, we can attach to both, its the developer call on which context he wants to debug. Should I add background_page and app as a target?

[roblourens]

Can you actually debug the background_page with the Chrome extension? Is it not a Node process? I don't know much about nwjs.

[galvesribeiro]

Yes. NW.js allow you to run node code from the HTML on the client. That background page is a bridge between those worlds so it doesn't have to attach 2 debuggers one for node and other for chrome.

[roblourens]

Ok. Hm... it looks exactly like the thing I want to filter out when attaching to Chrome. I'm trying to figure out how to unblock nw.js debugging without impacting the experience for users attaching to Chrome, which is the only "officially supported" scenario. Let me think about it for a bit.

[galvesribeiro]

Yes I agree. But keep in mind that even with pages, attach to a wrong tab happens all the time so will do with extensions... That is something on chrome debugger defined as "by design" that you can't avoid... People using other tools suffer with the same today even developing pages... Many other tools ask for the user to close all chrome instances before start the debug... You have the "url" setting on launch.json which should avoid to close chrome if already open... Lets just emphasize on the docs that url must be used to find the tab...

[galvesribeiro]

Did a simple test. If I use the sample launch.json I posted in the PR, yes, sometimes(1 from 20 times running "launch" request) if chrome is already open, the debugger print that it picked up the first tab which probably will not be the right target to debug, however, if I add the "url": "//nochadnphaoigfjnaclpecjelllocobm/dashboard" to the launch.json, the problem is gone... Again, use urlkill the problem...

[roblourens]

We'll figure this out but in the meantime I'm not going to do anything that could break someone's working config. I don't want to tell users to add a new "url" prop when they didn't need it before.

[galvesribeiro]

Nop, maybe I not explained myself... Current users(page developers) don't need to change anything. The new app developers that uses nw.js and want to debug the node code need to add the URL.

What I'm trying to say is that page developers already have the problem to put the "url" prop if they what to use chrome while developing pages otherwise, they risk the debugger to attach to the first tab. This behavior exist, and that filter will not avoid that.

[galvesribeiro]

@roblourens just for the sake of information:

http://docs.nwjs.io/en/latest/For%20Users/Debugging%20with%20DevTools/

Look at the "Node.js Modules Debugging" section that show what I'm saying.

[galvesribeiro]

@roblourens just found another problem...

https://github.com/Microsoft/vscode-chrome-debug-core/blob/master/src/chrome/chromeConnection.ts#L151

This line has 127.0.0.1 hardcoded... That blocks developers who are working on apps that target remote devices which is a supported scenario on Chrome Dev Tools... For instance in our case, we had NW.js running on an ARM device. As long as the device exposes the http://device_ip_address:9222/json (or whatever port you pass to --remote-debugging-port=), Chrome Dev Tools can access it... I'll look thru the code and check where the configuration from launch.json is consumed, add a supported chromeWSUrl tag and default it on code to 127.0.0.1 so that setting would be totally optional for that cases... Any concerns about that?

[galvesribeiro]

Just an important note; the page url that you use to find the right tab, maybe not the same that host the chrome debugger websocket...

[auchenberg]

Idea: We could move the "target filtering" out of this core debugger and into the Chrome specific debugger? This would enable us to ship a vscode-nwjs-debugger that has a simpler config?

[galvesribeiro]

@auchenberg that could work.

The point is, the debugger (IMHO) is not for the Chrome browser... It is for the Chrome Debugger Protocol(CDP) if I understood its purpose correctly... By not tying the debug experience to Browser itself, and stick to the protocol, other browsers which are based on CDP will just work with that... The Chrome Dev Tools by itself can be used for any target as long as it has implemented CDP correctly even nother browsers, and even if they don't use the Chromium rendering engine or V8 at all, but just implement the CDP instead... I don't see a reason to split on 2(or many more) debuggers if all of them will have the same protocol underneath and exactly same possible combinations of configuration...

[auchenberg]

This project, the vscode-chrome-debug-core is intended as a generic Chrome Debugging Protocol compliant debugger for VS Code. It's being used in vscode-chrome-debug and vscode-edge-debug which each has specific launching/config logic for the runtime they are targeting, but relying on this core debugger for the debugging itself.

By making a vscode-electron-debug and vscode-nwjs-debug extension we could remove the runtimeExecutable and runtimeArgs properties from the launch.json configs (simplification) and add the necessary target filtering logic, as the target types seems to be different cross regular Chrome, NWJS and Electron.

[galvesribeiro]

AHHH ok... Sorry, I was thinking that it was a single thing... Understood now. BTW, didn't know Edge is using the chrome-debug-core since its JS engine is Chakra and not V8.

So @auchenberg in any case, we have to (1)remove that hardcoded 127.0.0.1 and (2) create the new project for other debuggers that have those different dicovery settings...

Should we close/discard this PR and start with a new one?

[roblourens]

Remote debugging is another thing - I know Node supports it now, I have a task to look at that, I don't know what it will take. Go ahead if you know how to do it. Use the same property name that Node does.

@auchenberg is right, removing this filtering which is only applicable to browsers and creating a new extension for the different target is what we should be moving towards. Just a matter of prioritizing that vs other work.

In the meantime here's another possible low-impact workaround - if the 'url' property is set, and there's a match before the filters are applied, use that match. Otherwise, apply the filters as usual.

[galvesribeiro]

@roblourens don't need to match node debug... I mean, the current code already work. Local or remote the only thing the debugger cares is to have the IP address and the port used by --remote-debugging-port. The only thing is that 127.0.0.1 hardcoded. If that is took from a parameter and have default value as 127.0.0.1 you get both remote and local debug working.

Will have a good look on the project structure and see how can we implement those points we discussed here in another PR. Just keep this one open for the record. When I push the other I'll close this one.

Btw, if you guys want to talk more about that ping me on gitter. I'm on the VSCode channel.

Thanks!

[roblourens]

Gotcha. Thanks for the contribution!

[nojvek]

Oh I just saw this. It seems remote debugging should work now after #50. Using the same param as node. I verified it by running chrome on a virtual machine and connecting to it.

[galvesribeiro]

@nojvek that is good! So I can get hid of my hack and just set the address field on launch.json for the chrome target and it will attach to it? Does it already got pushed to the latest release?

[nojvek]

I'm not sure about electron and nw.js. This is a question more for
@auchenberg and @roblou whether its in the scope of this repo.

I'm pretty new here, so I'm not sure when a new version will get published.
Hopefully soon.

On Tue, Jun 21, 2016 at 3:52 PM, Gutemberg Ribeiro <notifications@github.com

wrote:

@nojvek https://github.com/nojvek that is good! So I can get hid of my
hack and just set the address field on launch.json for the chrome target
and it will attach to it? Does it already got pushed to the latest release?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#20 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA-JVA2-XLX53i5F2GgtBe2Y8E-cQHQGks5qOGtDgaJpZM4Isvg1
.

[auchenberg]

We won't be adding NW.js and electron support in this debugger. That should happen in new separate debuggers as explained above, but the core-debugger is now made more generic and is ready if the community want to build such debuggers.

[galvesribeiro]

@nojvek I meant, if remote debug for Chrome is something that is working. As you said looks like.

@auchenberg I understand that. I still don't see the point on have NW.js and Electron as diff debuggers than Chrome and Node since they still chrome and node under the hood.

Anyway, I'll stick with the hack now since we are in a emergency here to get it working asap. Later on, I'll investigate how to use core-debugger and submit a better debugger experience which is focused on support both chrome and node protocols which is what people want, and not just web pages.

Worth mention that any other IDE/Editor which support those protocols works OOTB with Electron and NW.js like Atom Editor, WebStorm, alm.tools etc... none require extra (or different) debugger/plugin.

I'm closing this PR since it is pointless to keep discussing this if that is the decision you guys made regarding the product.

Thanks for all support so far.

[nojvek]

To be honest I support the view. I'm personally not a fan of having
separate debuggers which in essence speak the same chrome remote debug
protocol. Too much fragmentation.

I guess the separation is because launching involves work. But visual
studio usually attaches, and so do other IDE's. I would argue that
launching is the work of tasks.

I would propose a single debugger (vscode-js-debug) that can attach to any
address:port that speaks the chrome debug protocol. Hell even being able to
attach to multiple ports simultaneously since VScode debug adapter api has
some support for threads. Being able to debug browser and node at the same
time is a very common scenario. Or being able to debug the different parts
of an electron / nwjs app at the same time. This becomes even more useful
once you're able to live edit the entire stack without a restart.

my 2 cents.

On Saturday, June 25, 2016, Gutemberg Ribeiro notifications@github.com
wrote:

Closed #20 #20
.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#20 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA-JVEcVoQY0UeJMm_hxZZgORPYNVnraks5qPZQYgaJpZM4Isvg1
.

[auchenberg]

Ideally we would be able to ship "one unified debugger" that would be able to target any given runtime that supports the CDP and the API's are we using. That's the end-goal, but with the current architecture for VSCode Debuggers we need to have runtime-specific logic within each debugger, so VS Code know's how to launch Chrome or Electron with remote debugging enabled, and that's the basis for shipping runtime-specific debuggers as their own extensions.

vscode-chrome-debug is a Google Chrome specific debugger for VS Code, and in order to keep the configuration to the simplest possible we are gonna keep the debugger specific to Chrome. Frankly the configuration is already too complicated, and we need to spend time reducing configuration further, so we don't end up with a configuration nightmare like it has happened in other projects like WebPack.

Until we get a unified way to enable remote debugging in browser runtimes or get a better way to encapsulate runtime launching in VS Code, I don't see a way for us to provide a unified debugger without complicating configuration to annoyance for most of our users.

This project vscode-chrome-debug-core provides a generic CDP debugger that easily can be used to make runtime specific debuggers, so as previously mentioned you are more than welcome to make vscode-electron-debug extension, that would contain logic to detect Electron, OS specifc paths, etc.

We have added support for address on the config and the target filtering can now be modified, so everything is in place to make a good Electron debugger.

[nojvek]

When chrome-debug-core is updated, do all the dependent shim debugger
extensions also pick up the update or do they need independent publishes as
well?

On Saturday, June 25, 2016, Kenneth Auchenberg notifications@github.com
wrote:

Ideally we would be able to ship "one unified debugger" that would be able
to target any given runtime that supports the CDP and the API's are we
using. That's the end-goal, but with the current architecture for VSCode
Debuggers we need to have runtime-specific logic within each debugger, so
VS Code know's how to launch Chrome or Electron with remote debugging
enabled, and that's the basis for shipping runtime-specific debuggers as
their own extensions.

vscode-chrome-debug is a Google Chrome specific debugger for VS Code, and
in order to keep the configuration to the simplest possible we are gonna
keep the debugger specific to Chrome. Frankly the configuration is already
too complicated, and we need to spend time reducing configuration further,
so we don't end up with a configuration nightmare like it has happened in
other projects like WebPack.

Until we get a unified way to enable remote debugging in browser runtimes
or get a better way to encapsulate runtime launching in VS Code, I don't
see a way for us to provide a unified debugger without complicating
configuration to annoyance for most of our users.

This project vscode-chrome-debug-core provides a generic CDP debugger
that easily can be used to make runtime specific debuggers, so as
previously mentioned you are more than welcome to make
vscode-electron-debug extension, that would contain logic to detect
Electron, OS specifc paths, etc.

We have added support for address on the config and the target filtering
can now be modified, so everything is in place to make a good Electron
debugger.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#20 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA-JVOIPga3kMGuzmtWx1S7bYIBicTBqks5qPfungaJpZM4Isvg1
.

[galvesribeiro]

@nojvek I agree with you. Debug chrome and node at the same time is something required with today`s movement of those web apps... I think we will probably need to go and start a debugger on our own if we want it or, just move to another editor.

One of the major complains I've heard is the fact that even pointing to a single Chrome window, you can only debug JS and have no live-edit of it, so people will need to attach tons of other node modules in order to make that happen or even worse. If you are debugging a JS and you want to edit the DOM(just change a css class!), you have to deatach, and launch dev tools to do it, which will kill the VSCode session so instead of keeping 2 tools open, people rather prefer to just use DevTools due to this lack of tooling on VSCode.

With tools like http://alm.tools/ people are less inclined to use VSCode due to those many limitations...