file_util: Convert the wstring version of IsDirectoryEmpty to FilePath.

file_util: Convert the wstring version of IsDirectoryEmpty to FilePath.

BUG=24672
TEST=compiles

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

[tfarina, 2010-05-24 15:46:15]

Hi Tony, could you take a look?

[tony, 2010-05-25 00:11:42]

The change looks fine.  Do you think you could write a unittest for this
function?

http://codereview.chromium.org/2153002/diff/1005/2004
File chrome/installer/setup/uninstall.cc (right):

http://codereview.chromium.org/2153002/diff/1005/2004#newcode153
chrome/installer/setup/uninstall.cc:153:
file_util::IsDirectoryEmpty(parent_dir)) {
Nit: I think you can put this on the same line as the 'if' now.

[tfarina, 2010-05-25 01:33:33]

On 2010/05/25 00:11:42, tony wrote:
> The change looks fine.  Do you think you could write a unittest for this
> function?
> 
Unittests added.

> http://codereview.chromium.org/2153002/diff/1005/2004
> File chrome/installer/setup/uninstall.cc (right):
> 
> http://codereview.chromium.org/2153002/diff/1005/2004#newcode153
> chrome/installer/setup/uninstall.cc:153:
> file_util::IsDirectoryEmpty(parent_dir)) {
> Nit: I think you can put this on the same line as the 'if' now.

Done.

Rietveld is showing a strange diff of file_util_unittest.cc (maybe due to the
recent failures of 500 error?).

[tony, 2010-05-25 01:51:53]

LGTM

http://codereview.chromium.org/2153002/diff/4006/12003
File base/file_util_unittest.cc (right):

http://codereview.chromium.org/2153002/diff/4006/12003#newcode1628
base/file_util_unittest.cc:1628: if (file_util::PathExists(empty_dir))
Nit: Why would the directory already exist?  Can't we make this an ASSERT_FALSE
instead?

[tfarina, 2010-05-25 02:08:57]

On 2010/05/25 01:51:53, tony wrote:
> LGTM
> 
> http://codereview.chromium.org/2153002/diff/4006/12003
> File base/file_util_unittest.cc (right):
> 
> http://codereview.chromium.org/2153002/diff/4006/12003#newcode1628
> base/file_util_unittest.cc:1628: if (file_util::PathExists(empty_dir))
> Nit: Why would the directory already exist?  Can't we make this an
ASSERT_FALSE
> instead?

I think this is more safe with the trybots scenario (sometimes they are flaky),
but I changed to ASSERT_FALSE.