Use SetAlternateDesktop(false) for service process.

Use SetAlternateDesktop(false) for service process.
Service process may run as windows service and it fails to create a window station.

BUG=410237
Committed: https://crrev.com/13db9e4e90bf7809af656f04a7d5bda626cdc893
Cr-Commit-Position: refs/heads/master@{#293391}

[Vitaly Buka (NO REVIEWS), 2014-09-03 19:14:21]

vitalybuka@chromium.org changed reviewers:
+ jschuh@chromium.org

[jschuh, 2014-09-03 19:16:51]

lgtm. just verify locally that the service launches properly.

[Vitaly Buka (NO REVIEWS), 2014-09-03 22:19:36]

vitalybuka@chromium.org changed reviewers:
+ jam@chromium.org

[Vitaly Buka (NO REVIEWS), 2014-09-03 22:19:36]

+jam for switches

[jschuh, 2014-09-03 22:22:50]

jschuh@chromium.org changed reviewers:
+ cpu@chromium.org
- jschuh@chromium.org

[jschuh, 2014-09-03 22:23:21]

jschuh@chromium.org changed reviewers:
+ jschuh@chromium.org

[jschuh, 2014-09-03 22:23:21]

Adding cpu@, since I'm out.

[jam, 2014-09-03 22:25:08]

not lgtm,

see http://www.chromium.org/developers/content-module/. service process is a
chrome feature; content shouldn't know about it.

This should be changed by SandboxedProcessLauncherDelegate.

In ServiceSandboxedProcessLauncherDelegate, set *disable_default_policy=true

[Vitaly Buka (NO REVIEWS), 2014-09-03 22:26:39]

On 2014/09/03 22:23:21, jschuh (out thru 20-10-2014) wrote:
> Adding cpu@, since I'm out.

so if it's not straight forward, I'll walk through with debugger and share my
findings

[Vitaly Buka (NO REVIEWS), 2014-09-04 00:03:12]

Patchset #1 (id:1) has been deleted

[Vitaly Buka (NO REVIEWS), 2014-09-04 00:03:17]

Patchset #1 (id:20001) has been deleted

[Vitaly Buka (NO REVIEWS), 2014-09-04 00:03:25]

Patchset #2 (id:60001) has been deleted

[Vitaly Buka (NO REVIEWS), 2014-09-04 00:37:06]

Carlos, Please take a look.

patch #3 is the same as #1, reviewed by Justin, but with addressed John's
comment.
I have confused Justin saying that #1 didn't work. I probably was running broken
local build. Now it works fine.

However I just realized maybe the best option is patch #4?
according comment PolicyBase::GetAlternateDesktop(), (use_alternate_desktop_ &&
!alternate_desktop_handle_) is expected there.

https://codereview.chromium.org/534413002/diff/120001/sandbox/win/src/sandbox...
File sandbox/win/src/sandbox_policy_base.cc (right):

https://codereview.chromium.org/534413002/diff/120001/sandbox/win/src/sandbox...
sandbox/win/src/sandbox_policy_base.cc:531: if (use_alternate_desktop_ &&
alternate_desktop_handle_ &&
this use-case seems should be supported, check comment on line 216

[jam, 2014-09-04 16:10:29]

I don't understand why you need to change sandbox.

if your patchset 2 was correct, then as i mentioned above an equivalent way of
doing this is setting *disable_default_polic=true in
ServiceSandboxedProcessLauncherDelegate

[Vitaly Buka (NO REVIEWS), 2014-09-04 17:50:27]

On 2014/09/04 16:10:29, jam wrote:
> I don't understand why you need to change sandbox.

I don't have. Both #2 and #3 works.
#4 is the simplest and consistent with other functions in the code.

> 
> if your patchset 2 was correct, then as i mentioned above an equivalent way of
> doing this is setting *disable_default_polic=true in
> ServiceSandboxedProcessLauncherDelegate

[Vitaly Buka (NO REVIEWS), 2014-09-04 17:52:52]

On 2014/09/04 17:50:27, Vitaly Buka wrote:
> On 2014/09/04 16:10:29, jam wrote:
> > I don't understand why you need to change sandbox.
> 
> I don't have. Both #2 and #3 works.
> #4 is the simplest and consistent with other functions in the code.
> 
> > 
> > if your patchset 2 was correct, then as i mentioned above an equivalent way
of
> > doing this is setting *disable_default_polic=true in
> > ServiceSandboxedProcessLauncherDelegate

#2 is not a good option. Probably I would need to move default polity to service
and duplicate code.
So I'd recommend to choose between #3 and $4.

[Vitaly Buka (NO REVIEWS), 2014-09-04 17:57:26]

On 2014/09/04 17:52:52, Vitaly Buka wrote:
> On 2014/09/04 17:50:27, Vitaly Buka wrote:
> > On 2014/09/04 16:10:29, jam wrote:
> > > I don't understand why you need to change sandbox.
> > 
> > I don't have. Both #2 and #3 works.
> > #4 is the simplest and consistent with other functions in the code.
> > 
> > > 
> > > if your patchset 2 was correct, then as i mentioned above an equivalent
way
> of
> > > doing this is setting *disable_default_polic=true in
> > > ServiceSandboxedProcessLauncherDelegate
> 
> #2 is not a good option. Probably I would need to move default polity to
service
> and duplicate code.
> So I'd recommend to choose between #3 and $4.

Actually we need merge this into Beta branch. Let's commit #3 to avoid
unexpected serious consequences. 
I'll offer #4 to Justin after he come back.

[jam, 2014-09-04 18:06:38]

lgtm

https://codereview.chromium.org/534413002/diff/140001/chrome/service/service_...
File chrome/service/service_utility_process_host.cc (right):

https://codereview.chromium.org/534413002/diff/140001/chrome/service/service_...
chrome/service/service_utility_process_host.cc:28: 
nit: no change needed

[Vitaly Buka (NO REVIEWS), 2014-09-04 18:19:39]

https://codereview.chromium.org/534413002/diff/140001/chrome/service/service_...
File chrome/service/service_utility_process_host.cc (right):

https://codereview.chromium.org/534413002/diff/140001/chrome/service/service_...
chrome/service/service_utility_process_host.cc:28: 
On 2014/09/04 18:06:38, jam wrote:
> nit: no change needed

Done.

[cpu_(ooo_6.6-7.5), 2014-09-04 19:04:23]

lgtm

https://codereview.chromium.org/534413002/diff/160001/chrome/service/service_...
File chrome/service/service_utility_process_host.cc (right):

https://codereview.chromium.org/534413002/diff/160001/chrome/service/service_...
chrome/service/service_utility_process_host.cc:56: // ::CreateWindowStation.
.. and it fails to create a window station.

[Vitaly Buka (NO REVIEWS), 2014-09-04 21:15:10]

https://codereview.chromium.org/534413002/diff/160001/chrome/service/service_...
File chrome/service/service_utility_process_host.cc (right):

https://codereview.chromium.org/534413002/diff/160001/chrome/service/service_...
chrome/service/service_utility_process_host.cc:56: // ::CreateWindowStation.
On 2014/09/04 19:04:23, cpu wrote:
> .. and it fails to create a window station.

Done.

[Vitaly Buka (NO REVIEWS), 2014-09-04 21:15:29]

The CQ bit was checked by vitalybuka@chromium.org

[Vitaly Buka (NO REVIEWS), 2014-09-04 21:16:18]

The CQ bit was unchecked by vitalybuka@chromium.org

[Vitaly Buka (NO REVIEWS), 2014-09-04 21:17:05]

Patchset #8 (id:200001) has been deleted

[Vitaly Buka (NO REVIEWS), 2014-09-04 21:22:49]

The CQ bit was checked by vitalybuka@chromium.org

[commit-bot: I haz the power, 2014-09-04 21:23:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/534413002/180001

[commit-bot: I haz the power, 2014-09-05 01:25:37]

Committed patchset #7 (id:180001) as dbed36a1133f4b0258ab3f3d1fc7d4dba407a373

[commit-bot: I haz the power, 2014-09-10 03:34:47]

Patchset 7 (id:??) landed as
https://crrev.com/13db9e4e90bf7809af656f04a7d5bda626cdc893
Cr-Commit-Position: refs/heads/master@{#293391}