Return the thread handle when launching a process in a different session via the execution server o…

Return the thread handle when launching a process in a different session via the execution server on Windows XP.

BUG=134694
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=160495

[alexeypa (please no reviews), 2012-10-04 18:10:27]

PTAL.

[simonmorris, 2012-10-04 23:32:29]

On 2012/10/04 18:10:27, alexeypa wrote:
> PTAL.

lgtm with optional suggestions.

[simonmorris, 2012-10-04 23:32:44]

lgtm with optional suggestions.

http://codereview.chromium.org/11065016/diff/1/remoting/host/win/launch_proce...
File remoting/host/win/launch_process_with_token.cc (right):

http://codereview.chromium.org/11065016/diff/1/remoting/host/win/launch_proce...
remoting/host/win/launch_process_with_token.cc:210: // exition too soon and
OpenProcess() call below.
exition -> exiting

http://codereview.chromium.org/11065016/diff/1/remoting/host/win/launch_proce...
remoting/host/win/launch_process_with_token.cc:270: if
(response.process_information.hProcess == NULL) {
Maybe change this to !response..., for consistency.

http://codereview.chromium.org/11065016/diff/1/remoting/host/win/launch_proce...
remoting/host/win/launch_process_with_token.cc:346: }
Consider moving the preceding 7 lines to a separate method, and replacing each
"success = false;" by "CloseHandles(response); return false;". Then you don't
need |success|, and I think the logic is easier to follow.

[alexeypa (please no reviews), 2012-10-05 18:29:59]

Splitting CreateRemoteSessionProcess() into several functions resulted in a
large diff. Please take a look just to make sure.

https://chromiumcodereview.appspot.com/11065016/diff/1/remoting/host/win/laun...
File remoting/host/win/launch_process_with_token.cc (right):

https://chromiumcodereview.appspot.com/11065016/diff/1/remoting/host/win/laun...
remoting/host/win/launch_process_with_token.cc:210: // exition too soon and
OpenProcess() call below.
On 2012/10/04 23:32:44, simonmorris wrote:
> exition -> exiting

Done.

https://chromiumcodereview.appspot.com/11065016/diff/1/remoting/host/win/laun...
remoting/host/win/launch_process_with_token.cc:270: if
(response.process_information.hProcess == NULL) {
On 2012/10/04 23:32:44, simonmorris wrote:
> Maybe change this to !response..., for consistency.

Done.

https://chromiumcodereview.appspot.com/11065016/diff/1/remoting/host/win/laun...
remoting/host/win/launch_process_with_token.cc:346: }
On 2012/10/04 23:32:44, simonmorris wrote:
> Consider moving the preceding 7 lines to a separate method, and replacing each
> "success = false;" by "CloseHandles(response); return false;". Then you don't
> need |success|, and I think the logic is easier to follow.

Done. I've split CreateRemoteSessionProcess() into several functions.

[simonmorris, 2012-10-05 18:53:02]

lgtm

[commit-bot: I haz the power, 2012-10-05 20:13:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/11065016/7001

[commit-bot: I haz the power, 2012-10-05 22:07:26]

Change committed as 160495