Fix base::CopyDirectory() and JumpListIcons(Old) folders' operation logic

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

[grt (UTC plus 2), 2016/12/15 09:23:55]

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?

+  // result in a false negative, e.g. on C:\bar\Foo versus C:\bar\FooOld.
   if (real_to_path.value().size() >= real_from_path.value().size() &&

[grt (UTC plus 2), 2016/12/15 09:23:55]

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.

-      real_to_path.value().compare(0, real_from_path.value().size(),
-                                   real_from_path.value()) == 0) {
+      real_from_path.IsParent(real_to_path)) {

[grt (UTC plus 2), 2016/12/15 09:23:55]

please make this same fix in file_util_posix

     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