Port parts of chrome/common to Linux....

Port parts of chrome/common to Linux.
This probably needs some bigger adjustments. Some things need to be decided,
like where to store profile files on Linux/Mac.

BUG=3649

[please use my chromium address, 2008-10-23 17:39:15]

*ping*

[dank, 2008-10-23 17:55:00]

OK except for needing to use xdg-user-dirs to get directory name for Desktop

http://codereview.chromium.org/8059/diff/1/3
File chrome/common/chrome_paths.cc (right):

http://codereview.chromium.org/8059/diff/1/3#newcode103
Line 103: #endif
Not sure what DIR_USER_DOCUMENTS is used for.
Potentially we'd want to use the xdg spec for locating ~/Documents...

http://codereview.chromium.org/8059/diff/1/3#newcode123
Line 123: file_util::AppendToPath(&cur, L"Desktop");
Again, "Desktop" might not always be right.
That xdg spec is documented here:
http://www.freedesktop.org/wiki/Software/xdg-user-dirs

[Evan Martin, 2008-10-23 19:43:14]

I think it'd be ok to punt on special directory locations (e.g.
NOTIMPLEMENTED()) until we figure out what they're used for.

For example, finding the user's desktop may only be used on Windows for stuffing
an icon there, which we wouldn't want to do on Linux.

[please use my chromium address, 2008-10-23 20:07:59]

On 2008/10/23 19:43:14, Evan Martin wrote:
> I think it'd be ok to punt on special directory locations (e.g.
> NOTIMPLEMENTED()) until we figure out what they're used for.

Okay, so NOTIMPLEMENTED() for now. I also added some comments about that.

[Mark Mentovai, 2008-10-23 20:53:44]

LG with this change

http://codereview.chromium.org/8059/diff/206/9
File chrome/common/chrome_paths.cc (right):

http://codereview.chromium.org/8059/diff/206/9#newcode70
Line 70: #if defined(OS_WIN)
This is weird because system_buffer is only used in the DIR_USER_DESKTOP case. 
Can you move the declaration down there and give it a {scope}?  That would
minimize #ifdefs.

[please use my chromium address, 2008-10-24 16:54:58]

On 2008/10/23 20:53:44, Mark Mentovai wrote:
> LG with this change
> 
> http://codereview.chromium.org/8059/diff/206/9
> File chrome/common/chrome_paths.cc (right):
> 
> http://codereview.chromium.org/8059/diff/206/9#newcode70
> Line 70: #if defined(OS_WIN)
> This is weird because system_buffer is only used in the DIR_USER_DESKTOP case.

> Can you move the declaration down there and give it a {scope}?  That would
> minimize #ifdefs.

Done. Actually except the {} scope part. There is a similar path_buf declaration
few lines above, which doesn't have {} scope. If you wish I will add {} around
appropriate block containing system_buffer, or maybe let system_buffer be used
at both places.

[Mark Mentovai, 2008-10-24 17:07:35]

Oh, I didn't catch that the same thing applied to path_buf being moved.  Yes, I
think that both stack buffers should be narrowly {scoped} to their cases.

[please use my chromium address, 2008-10-24 17:14:29]

On 2008/10/24 17:07:35, Mark Mentovai wrote:
> Oh, I didn't catch that the same thing applied to path_buf being moved.  Yes,
I
> think that both stack buffers should be narrowly {scoped} to their cases.

Done. Hopefully I got formatting right.

[Mark Mentovai, 2008-10-24 18:32:38]

Checked in r3932 with the changes noted.

http://codereview.chromium.org/8059/diff/211/30
File chrome/common/chrome_paths.cc (right):

http://codereview.chromium.org/8059/diff/211/30#newcode87
Line 87: cur->assign(path_buf);
This should be cur.assign(path_buf);.

http://codereview.chromium.org/8059/diff/211/30#newcode118
Line 118: cur = system_buffer;
I changed this to cur.assign(system_buffer) too.

http://codereview.chromium.org/8059/diff/211/31
File chrome/common/logging_chrome.cc (right):

http://codereview.chromium.org/8059/diff/211/31#newcode97
Line 97: const wchar_t* log_file_name = GetLogFileName().c_str()
Needs a ; at the end of the line.

[jar (doing other things), 2008-10-24 22:28:07]

http://codereview.chromium.org/8059/diff/211/31
File chrome/common/logging_chrome.cc (right):

http://codereview.chromium.org/8059/diff/211/31#newcode97
Line 97: const wchar_t* log_file_name = GetLogFileName().c_str()
I don't think the lifetime of the wchar_t* is guaranteed beyond the ; at the end
of this line.

It was ok in the original code, but you can't save the temp this way (safely) I
think.

[please use my chromium address, 2008-10-25 07:52:21]

On 2008/10/24 22:28:07, jar wrote:
> http://codereview.chromium.org/8059/diff/211/31
> File chrome/common/logging_chrome.cc (right):
> 
> http://codereview.chromium.org/8059/diff/211/31#newcode97
> Line 97: const wchar_t* log_file_name = GetLogFileName().c_str()
> I don't think the lifetime of the wchar_t* is guaranteed beyond the ; at the
end
> of this line.
> 
> It was ok in the original code, but you can't save the temp this way (safely)
I
> think.

Oops. Indeed. Changed to std::{w,}string and extract c_str later. That looks
safe to me.

I also incorporated Mark's changes so applying patch next time will be easier.

[please use my chromium address, 2008-10-30 14:54:50]

On 2008/10/25 07:52:21, phajdan.jr wrote:
> On 2008/10/24 22:28:07, jar wrote:
> > http://codereview.chromium.org/8059/diff/211/31
> > File chrome/common/logging_chrome.cc (right):
> > 
> > http://codereview.chromium.org/8059/diff/211/31#newcode97
> > Line 97: const wchar_t* log_file_name = GetLogFileName().c_str()
> > I don't think the lifetime of the wchar_t* is guaranteed beyond the ; at the
> end
> > of this line.
> > 
> > It was ok in the original code, but you can't save the temp this way
(safely)
> I
> > think.
> 
> Oops. Indeed. Changed to std::{w,}string and extract c_str later. That looks
> safe to me.
> 
> I also incorporated Mark's changes so applying patch next time will be easier.

*ping* Please take a look. It should be ok now, but for sure a testbot test
would be nice.

[please use my chromium address, 2008-11-05 16:51:28]

Patch updated.

[Evan Martin, 2008-11-07 20:06:27]

r5012