Launch Chrome in unelevated state on Windows platform by using wmic call create.
#619
Conversation
| @@ -38,7 +38,8 @@ const watchedSources = [ | |||
| ]; | |||
|
|
|||
| const scripts = [ | |||
| 'src/terminateProcess.sh' | |||
| 'src/terminateProcess.sh', | |||
There was a problem hiding this comment.
Is "teminatePRocess.sh" still needed?
There was a problem hiding this comment.
Yes as far as I know. It's mac and Linux only
There was a problem hiding this comment.
I can't find src/terminateProcess.sh in the code base. How does it get injected for mac and Linux cases?
There was a problem hiding this comment.
| @@ -38,7 +38,8 @@ const watchedSources = [ | |||
| ]; | |||
|
|
|||
| const scripts = [ | |||
| 'src/terminateProcess.sh' | |||
| 'src/terminateProcess.sh', | |||
There was a problem hiding this comment.
Yes as far as I know. It's mac and Linux only
| @@ -38,7 +38,8 @@ const watchedSources = [ | |||
| ]; | |||
|
|
|||
| const scripts = [ | |||
| 'src/terminateProcess.sh' | |||
| 'src/terminateProcess.sh', | |||
| 'src/launchUnelevated.js' | |||
There was a problem hiding this comment.
I think this will have to be added to the csproj for signing
There was a problem hiding this comment.
Looking at the current state of the csproj, when it uses wildcard of ..\out\src\**\* and $(OutDir)\**\*.js, it should include this file too.
| @@ -222,7 +225,7 @@ export class ChromeDebugAdapter extends CoreDebugAdapter { | |||
| // Disconnect before killing Chrome, because running "taskkill" when it's paused sometimes doesn't kill it | |||
| super.disconnect(args); | |||
|
|
|||
| if (this._chromeProc && !hadTerminated) { | |||
| if ( (this._chromeProc || (!this._chromeProc && this._chromePID)) && !hadTerminated) { | |||
There was a problem hiding this comment.
Shouldn't happen with the current code, but since this can go through when this._chromeProc is null, add a null check below where we call .kill on it.
There was a problem hiding this comment.
Good catch.
Done.
|
|
||
| telemetry.telemetry.reportEvent('error', telemetryProperties); | ||
|
|
||
| return null; |
There was a problem hiding this comment.
Is this supposed to just keep going and try to attach if it can't find the Chrome pid? Or should it throw and fail the launch?
There was a problem hiding this comment.
Yes, after evaluated two approaches, we reached to a conclusion that we should let the debugging session go as much as it can. Not being able to find process id doesn't necessary mean Chrome is not launched nor non-debuggable. One known side-effect is merely that Chrome can't be closed at the end of the session. Due to current multi-debugger-client design of Chrome, that is not an issue any more.
| @@ -0,0 +1,28 @@ | |||
| var objShell = new ActiveXObject("shell.application"); | |||
There was a problem hiding this comment.
I'm just going to trust that this is all correct :)
There was a problem hiding this comment.
Maybe use let instead of var.
There was a problem hiding this comment.
Yes, and I tested it as much as I can. It works.
Unfortunately the Windows Script Host (cscript.exe) is nowhere near any modern ECMAScript standard. let is not supported.
|
Guess the build error is already fixed now. So please disregard the above. :) |
changsi-an
force-pushed
the
master
branch
2 times, most recently
from
7ad7e0b to
5ca41a7
Compare
This change is needed because when VS is started in elevated mode, DA will be started in elevated mode too. Any processes launched by DA, in our case Chrome, will be started in elevated mode as well. Chrome is known to have issues with elevated mode.
When DA received
shouldLaunchChromeUnelevatedlaunch parameter, it will callcscript.exeto run a customized Windows Script Host program (launchUnelevated.js) to launch chrome process in unelevated mode.launchUnelevated.jswill dump the process id information to a temporary file negotiated with DA, so after the WSH process is finished, DA will poll the content of the file and find out the PID of the launched Chrome process.