Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

debug: allow debugging the renderer process #100242

Merged
merged 6 commits into from
Jun 25, 2020

Conversation

connor4312
Copy link
Member

@connor4312 connor4312 commented Jun 15, 2020

This adds a new parameter to the launchVSCode request --
debugRenderer. If set to true and we're in Electron, the main process
will attempt to stand up a CDP-speaking server and respond with the port
the debugger can connect to in order to handle debug events.

Note: this only works when webviews are iframe-based. It's possible
to do the same treatment to Electron webviews with greater complexity. I didn't
implement that today, and maybe we say since iframes are coming
eventually (and they can be toggled on with a user setting that we could
have in the starter...) we don't need do add handling for that.

This does not work when debugging in web because there's no way to
force the browser to enter debug mode.

Code in this PR is still rough, I will keep this in PR until
I have js-debug working end to end and iron out any kinks.

Works towards #96626

This adds a new parameter to the `launchVSCode` request --
`debugRenderer`. If set to true and we're in Electron, the main process
will attempt to stand up a CDP-speaking server and respond with the port
the debugger can connect to in order to handle debug events.

Note: this only works when webviews are iframe-based. It's _possible_
to do the same treatment to webviews with greater complexity. I didn't
implement that today, and maybe we say since iframes are coming
eventually (and they can be toggled on with a user setting that we could
have in the starter...) we don't need do add handling for that.

This does not work when debugging in web because there's no way to
force the browser to enter debug mode.

Code in this PR is still rough, I will keep this in PR until
I have js-debug working end to end and iron out any kinks.
@connor4312 connor4312 requested a review from weinand June 15, 2020 23:14
@weinand weinand added the debug Debug viewlet, configurations, breakpoints, adapter issues label Jun 17, 2020
@weinand weinand added this to the June 2020 milestone Jun 17, 2020
@connor4312 connor4312 force-pushed the connor4312/expose-webview-cdp branch from 6ce01c3 to 1513428 Compare June 24, 2020 00:48
@connor4312
Copy link
Member Author

connor4312 commented Jun 24, 2020

@weinand this is ready for review now; I've got the scenario working end to end now, pending polish on the js-debug side of things.

@connor4312 connor4312 self-assigned this Jun 24, 2020
connor4312 added a commit to microsoft/vscode-js-debug that referenced this pull request Jun 24, 2020
- soft dependency on microsoft/vscode#100242
- fixes microsoft/vscode#100242

With these changes, you can now add `debugWebviews: true` (or a Chrome
configuration object) to an extension host configuration to attach
and debug webviews. It works.

![](https://memes.peet.io/img/20-06-787960a9-f2de-4879-bee3-d7088738cea0.png)

Todo:

- As mentioned, this only works with iframe-based webviews. This is where
  VS Code is moving towards and Electron webviews are a different beast
	that I don't think it's worth dealing with.
- Currently, it attaches to all webviews in Code instances
- The name of the webview debug sessions is a long URL

This can be most easily fixed by exposing some data in the webview
title, cc @mjbvz. Alternatively, we could have some messaging in a
custom message that the main process can send over to js-debug to tell
it about webviews.
Copy link
Contributor

@weinand weinand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes look good to me but I could not get the web UI ("yarn web") to work with your branch (even after merging with the latest).

This is the error I was running into:
2020-06-25_11-38-27

Extension debugging worked fine in the desktop app.

@connor4312
Copy link
Member Author

I updated the IExtensionHostDebugService and I think will need to make the corresponding change in the server. I'll do so and merge.

The `purpose` can be used for notebooks instead of the extension ID,
since there's no extension associated with the renderer view.

The renderer URL now looks like:

```
https://<guid>.vscode-webview-test.com/<random>/index.html?id=<guid>&extensionId=&purpose=notebookRenderer
```
And a renderer is:

```
https://<guid>.vscode-webview-test.com/<random>/index.html?id=<guid>&extensionId=connor4312.vsix-viewer&purpose=undefined
```
I wanted to put this in the page title, but unfortunately this is hard
due to https://bugs.chromium.org/p/chromium/issues/detail?id=1058108.
Instead, add it to the url which _is_ available and given in
the first targetInfoChanged event.
@connor4312
Copy link
Member Author

yarn web and web debugging actually do work for me with these changes (webviews can't be debugging in web because browsers, but that's expected). I thought I had to change something in the server but it looks like that's not the case. The error you're running into looks unrelated most likely.

Copy link
Collaborator

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Webview changes look good to me

@connor4312 connor4312 merged commit 08ed2b6 into master Jun 25, 2020
@connor4312 connor4312 deleted the connor4312/expose-webview-cdp branch June 25, 2020 18:54
connor4312 added a commit to microsoft/vscode-js-debug that referenced this pull request Jun 26, 2020
* fix: increase repl character budget significantly

Fixes microsoft/vscode#100786

* feat: implement webview debugging

- soft dependency on microsoft/vscode#100242
- fixes microsoft/vscode#100242

With these changes, you can now add `debugWebviews: true` (or a Chrome
configuration object) to an extension host configuration to attach
and debug webviews. It works.

![](https://memes.peet.io/img/20-06-787960a9-f2de-4879-bee3-d7088738cea0.png)

Todo:

- As mentioned, this only works with iframe-based webviews. This is where
  VS Code is moving towards and Electron webviews are a different beast
	that I don't think it's worth dealing with.
- Currently, it attaches to all webviews in Code instances
- The name of the webview debug sessions is a long URL

This can be most easily fixed by exposing some data in the webview
title, cc @mjbvz. Alternatively, we could have some messaging in a
custom message that the main process can send over to js-debug to tell
it about webviews.

* fix: tests

* feat: name webviews better, support url filtering
@github-actions github-actions bot locked and limited conversation to collaborators Aug 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants