Make sure only the main browser process and service proceses are allowed to create the profile dire…

Make sure only the main browser process and service proceses are allowed to create the profile directory.

This patch lets Chrome start with profile located on a network share on
Windows Vista and newer.

BUG=120388
TEST=Start Chrome with --user-data-dir pointing to a network share location and try to navigate to a web page. This should not lead to a hang of the renderer.


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

[pastarmovj, 2012-05-02 15:25:49]

Hi Carlos, Will,
Can you please review this CL. It is the second of the rows of CL that should
bring the support for profiles on network shares on Windows at least as far as
it is for Linux now. This one makes sure only the main process is allowed to
create the profile dir.

@Carlos: Please review the CL as a whole.
@Will: Please review as an OWNER of base/*

[jam, 2012-05-02 17:29:52]

http://codereview.chromium.org/10306009/diff/7001/chrome/common/chrome_paths.cc
File chrome/common/chrome_paths.cc (right):

http://codereview.chromium.org/10306009/diff/7001/chrome/common/chrome_paths....
chrome/common/chrome_paths.cc:78: // switches makes all hell break loose on us
when linking.
I don't understand this comment. Many switches in
content/public/common/content_switches are used in chrome. they just need
CONTENT_EXPORT, which kProcessType has.

[cpu_(ooo_6.6-7.5), 2012-05-02 17:33:48]

lgtm, but please wait for john about the line 79 issue.

http://codereview.chromium.org/10306009/diff/7001/chrome/common/chrome_paths.cc
File chrome/common/chrome_paths.cc (right):

http://codereview.chromium.org/10306009/diff/7001/chrome/common/chrome_paths....
chrome/common/chrome_paths.cc:79: const char kProcessTypeSwitch[] = "type";
asking john (jam) what is the way to go here.

http://codereview.chromium.org/10306009/diff/7001/chrome/common/chrome_paths....
chrome/common/chrome_paths.cc:144: kProcessTypeSwitch)));
thanks! no sandboxed process should me trafficking with our profile dir.

[Elliot Glaysher, 2012-05-02 17:37:14]

lgtm

[willchan no longer on Chromium, 2012-05-02 17:42:34]

lgtm

http://codereview.chromium.org/10306009/diff/7001/base/path_service.h
File base/path_service.h (right):

http://codereview.chromium.org/10306009/diff/7001/base/path_service.h#newcode43
base/path_service.h:43: // Overrides the path to a special directory or file. 
This cannot be used to
Rather than copying the comment from above, how about just saying it's the same
except for the |create| stuff, which you should explain?

Also, it seems like this API subsumes the other one. Maybe we should eliminate
the overlap and just create a OverrideButDontCreate()? Sounds ugly. I guess I
prefer what you currently have. Was just a thought.

[pastarmovj, 2012-05-03 16:48:23]

This version does not need the symbol duplication any more.

http://codereview.chromium.org/10306009/diff/7001/base/path_service.h
File base/path_service.h (right):

http://codereview.chromium.org/10306009/diff/7001/base/path_service.h#newcode43
base/path_service.h:43: // Overrides the path to a special directory or file. 
This cannot be used to
On 2012/05/02 17:42:34, willchan wrote:
> Rather than copying the comment from above, how about just saying it's the
same
> except for the |create| stuff, which you should explain?
> 
> Also, it seems like this API subsumes the other one. Maybe we should eliminate
> the overlap and just create a OverrideButDontCreate()? Sounds ugly. I guess I
> prefer what you currently have. Was just a thought.

Done.

http://codereview.chromium.org/10306009/diff/7001/chrome/common/chrome_paths.cc
File chrome/common/chrome_paths.cc (right):

http://codereview.chromium.org/10306009/diff/7001/chrome/common/chrome_paths....
chrome/common/chrome_paths.cc:78: // switches makes all hell break loose on us
when linking.
On 2012/05/02 17:29:52, John Abd-El-Malek wrote:
> I don't understand this comment. Many switches in
> content/public/common/content_switches are used in chrome. they just need
> CONTENT_EXPORT, which kProcessType has. 

After our offline discussion I came up with new version which fixes the broken
project crash_service_win64 in a pattern that other projects have already used.

http://codereview.chromium.org/10306009/diff/7001/chrome/common/chrome_paths....
chrome/common/chrome_paths.cc:144: kProcessTypeSwitch)));
On 2012/05/02 17:33:48, cpu wrote:
> thanks! no sandboxed process should me trafficking with our profile dir.

I am glad to be useful :)

[jam, 2012-05-03 16:54:11]

lgtm

[commit-bot: I haz the power, 2012-05-03 17:09:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pastarmovj@chromium.org/10306009/17003

[commit-bot: I haz the power, 2012-05-03 17:14:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pastarmovj@chromium.org/10306009/21004

[commit-bot: I haz the power, 2012-05-03 19:48:47]

Try job failure for 10306009-21004 (retry) on win_rel for step "ui_tests"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-04 09:26:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pastarmovj@chromium.org/10306009/21004

[commit-bot: I haz the power, 2012-05-04 11:27:31]

Try job failure for 10306009-21004 (retry) on win_rel for step "ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-04 11:40:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pastarmovj@chromium.org/10306009/37001

[commit-bot: I haz the power, 2012-05-04 13:04:16]

Change committed as 135321