A follow-up nit fix for jumplist file operations

base/files/file_util_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/files/file_util.h"
 
 #include <windows.h>
 #include <io.h>
 #include <psapi.h>
 #include <shellapi.h>
@@ skipping 159 matching lines @@
     if (real_to_path.empty())
       return false;
   } else {
     real_to_path = MakeAbsoluteFilePath(real_to_path.DirName());
     if (real_to_path.empty())
       return false;
   }
   FilePath real_from_path = MakeAbsoluteFilePath(from_path);
   if (real_from_path.empty())
     return false;
-  // Note: it's important to use IsParent() here as string comparison would
-  // result in a false negative, e.g. on C:\bar\Foo versus C:\bar\FooOld.

[chengx, 2016/12/15 19:51:54]

grt@ 
nit: i think this comment is unnecessary. the FilePath API should be used
because it's the right thing. otherwise, why is this one use of IsParent so
important that this comment is justified in comparison to every other use of
IsParent in the codebase?

-  if (real_to_path.value().size() >= real_from_path.value().size() &&
-      real_from_path.IsParent(real_to_path)) {

[chengx, 2016/12/15 19:51:54]

grt@ 
this "optimization" seems wrong to me -- either a FilePath is treated like a
string (as was with the old code), or it's treated as a FilePath (as it is with
the call to IsParent). i think it's a mistake to mix the two like this.

+  if (real_to_path == real_from_path || real_from_path.IsParent(real_to_path)) {

[grt (UTC plus 2), 2016/12/15 21:28:32]

nit: no braces

     return false;
   }
 
   int traverse_type = FileEnumerator::FILES;
   if (recursive)
     traverse_type |= FileEnumerator::DIRECTORIES;
   FileEnumerator traversal(from_path, recursive, traverse_type);
 
   if (!PathExists(from_path)) {
     DLOG(ERROR) << "CopyDirectory() couldn't stat source directory: "
@@ skipping 630 matching lines @@
     // Like Move, this function is not transactional, so we just
     // leave the copied bits behind if deleting from_path fails.
     // If to_path exists previously then we have already overwritten
     // it by now, we don't get better off by deleting the new bits.
   }
   return false;
 }
 
 }  // namespace internal
 }  // namespace base