Get rid of kPathSeparator on windows.

Get rid of kPathSeparator on windows. Port some wstring function to take FilePaths. Re-enable relevant posix unit tests.

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

[tony, 2008-12-04 08:45:12]

http://codereview.chromium.org/12893/diff/261/281
File base/file_util.cc (right):

http://codereview.chromium.org/12893/diff/261/281#newcode21
Line 21: const FilePath::CharType kExtensionSeparator = FILE_PATH_LITERAL('.');
If this is only used in this file, perhaps it shouldn't be in the file_util
namespace.  Maybe just put it in an anonymous namespace?

http://codereview.chromium.org/12893/diff/261/280
File base/file_util.h (right):

http://codereview.chromium.org/12893/diff/261/280#newcode110
Line 110: void InsertBeforeExtension(FilePath* path, const FilePath::StringType
suffix);
There's a missing & for the suffix.

http://codereview.chromium.org/12893/diff/261/282
File base/file_util_unittest.cc (right):

http://codereview.chromium.org/12893/diff/261/282#newcode202
Line 202: FilePath path =
FilePath::FromWStringHack(kInsertBeforeExtension[i].path);
Would it be possible to use FILE_PATH_LITERAL above for path and result in
struct InsertBeforeExtensionCase so you don't have to use FromWStringHack and
ToWStringHack below?

http://codereview.chromium.org/12893/diff/261/282#newcode813
Line 813: FilePath path =
FilePath::FromWStringHack(kReplaceExtension[i].file_name);
Same comment as above.

http://codereview.chromium.org/12893/diff/261/282#newcode828
Line 828: EXPECT_EQ(path.ToWStringHack() + L".baz",
result_path.ToWStringHack());
And again.

http://codereview.chromium.org/12893/diff/261/264
File chrome/browser/visitedlink_perftest.cc (right):

http://codereview.chromium.org/12893/diff/261/264#newcode44
Line 44: wcscpy_s(db_name, MAX_PATH, db_path.ToWStringHack().c_str());
This wcscpy_s won't be portable.  Maybe this method should take a FilePath* as
the out param?  You can add a TODO for this for now.

[Evan Stade, 2008-12-04 19:19:25]

http://codereview.chromium.org/12893/diff/261/281
File base/file_util.cc (right):

http://codereview.chromium.org/12893/diff/261/281#newcode21
Line 21: const FilePath::CharType kExtensionSeparator = FILE_PATH_LITERAL('.');
On 2008/12/04 08:45:12, tony wrote:
> If this is only used in this file, perhaps it shouldn't be in the file_util
> namespace.  Maybe just put it in an anonymous namespace?

Done.

http://codereview.chromium.org/12893/diff/261/280
File base/file_util.h (right):

http://codereview.chromium.org/12893/diff/261/280#newcode110
Line 110: void InsertBeforeExtension(FilePath* path, const FilePath::StringType
suffix);
On 2008/12/04 08:45:12, tony wrote:
> There's a missing & for the suffix.

Done.

http://codereview.chromium.org/12893/diff/261/282
File base/file_util_unittest.cc (right):

http://codereview.chromium.org/12893/diff/261/282#newcode202
Line 202: FilePath path =
FilePath::FromWStringHack(kInsertBeforeExtension[i].path);
On 2008/12/04 08:45:12, tony wrote:
> Would it be possible to use FILE_PATH_LITERAL above for path and result in
> struct InsertBeforeExtensionCase so you don't have to use FromWStringHack and
> ToWStringHack below?

yes the Ls should change to FILE_PATH_LITERALs. But that can be said of almost
every L-prefixed constant in this file. The only way to live with this file is
to accept the use of *Hack or deprecated functions.

I'll still change it if you want though. Just not sure where you draw the line.

[tony, 2008-12-04 19:30:36]

The plan is to remove the *Hack and deprecated functions entirely.  If you want,
you can put a TODO into the file that this needs to happen, but it would be more
awesome if you just went ahead and did it while you're there.

With either change, LGTM.

On 2008/12/04 19:19:25, Evan Stade wrote:
> On 2008/12/04 08:45:12, tony wrote:
> > Would it be possible to use FILE_PATH_LITERAL above for path and result in
> > struct InsertBeforeExtensionCase so you don't have to use FromWStringHack
and
> > ToWStringHack below?
> 
> yes the Ls should change to FILE_PATH_LITERALs. But that can be said of almost
> every L-prefixed constant in this file. The only way to live with this file is
> to accept the use of *Hack or deprecated functions.
> 
> I'll still change it if you want though. Just not sure where you draw the
line.