Clean up the strings for sync code:

- Use FilePath for real file paths.
- Use std::string for PathString on Windows as well

TODO in a separate CL:
- Get rid of PathString typedef completely and directly use std::string everywhere.
- Use wchar_t in syncapi.h/.cc and get rid of sync_char16.

TEST=Existing tests are sufficient since this CL does a lot of code refactoring.
BUG=26342

Review URL: http://codereview.chromium.org/340055

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30970 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 81 insertions, 228 deletions

diff --git a/chrome/browser/sync/syncable/directory_backing_store.cc b/chrome/browser/sync/syncable/directory_backing_store.cc
index 26577d6..173dc0b 100644
--- a/chrome/browser/sync/syncable/directory_backing_store.cc
+++ b/chrome/browser/sync/syncable/directory_backing_store.cc
@@ -71,11 +71,7 @@ static void PathNameMatch16WithEscape(sqlite3_context* context,
 #endif
 
 static void RegisterPathNameCollate(sqlite3* dbhandle) {
-#if defined(OS_WIN)
-  const int collate = SQLITE_UTF16;
-#else
   const int collate = SQLITE_UTF8;
-#endif
   CHECK(SQLITE_OK == sqlite3_create_collation(dbhandle, "PATHNAME", collate,
       NULL, &ComparePathNames16));
 }
@@ -246,7 +242,7 @@ static string ComposeCreateTableColumnSpecs(const ColumnSpec* begin,
 // DirectoryBackingStore implementation.
 
 DirectoryBackingStore::DirectoryBackingStore(const PathString& dir_name,
-    const PathString& backing_filepath)
+                                             const FilePath& backing_filepath)
     : load_dbhandle_(NULL),
       save_dbhandle_(NULL),
       dir_name_(dir_name),
@@ -266,7 +262,7 @@ DirectoryBackingStore::~DirectoryBackingStore() {
 
 bool DirectoryBackingStore::OpenAndConfigureHandleHelper(
     sqlite3** handle) const {
-  if (SQLITE_OK == SqliteOpen(backing_filepath_.c_str(), handle)) {
+  if (SQLITE_OK == SqliteOpen(backing_filepath_, handle)) {
     sqlite3_busy_timeout(*handle, kDirectoryBackingStoreBusyTimeoutMs);
     RegisterPathNameCollate(*handle);
     RegisterPathNameMatch(*handle);
diff --git a/chrome/browser/sync/syncable/directory_backing_store.h b/chrome/browser/sync/syncable/directory_backing_store.h
index a0bf8b1..d12bde7 100644
--- a/chrome/browser/sync/syncable/directory_backing_store.h
+++ b/chrome/browser/sync/syncable/directory_backing_store.h
@@ -7,6 +7,7 @@
 
 #include <set>
 
+#include "base/file_path.h"
 #include "chrome/browser/sync/syncable/dir_open_result.h"
 #include "chrome/browser/sync/syncable/syncable.h"
 
@@ -45,7 +46,7 @@ typedef Directory::MetahandlesIndex MetahandlesIndex;
 class DirectoryBackingStore {
  public:
   DirectoryBackingStore(const PathString& dir_name,
-                        const PathString& backing_filepath);
+                        const FilePath& backing_filepath);
 
   virtual ~DirectoryBackingStore();
 
@@ -113,7 +114,7 @@ class DirectoryBackingStore {
   sqlite3* save_dbhandle_;
 
   PathString dir_name_;
-  PathString backing_filepath_;
+  FilePath backing_filepath_;
 
   DISALLOW_COPY_AND_ASSIGN(DirectoryBackingStore);
 };
diff --git a/chrome/browser/sync/syncable/directory_manager.cc b/chrome/browser/sync/syncable/directory_manager.cc
index edce2e27c..d934f94 100644
--- a/chrome/browser/sync/syncable/directory_manager.cc
+++ b/chrome/browser/sync/syncable/directory_manager.cc
@@ -10,13 +10,15 @@
 
 #include "base/logging.h"
 #include "base/port.h"
+#include "base/scoped_ptr.h"
 #include "chrome/browser/sync/syncable/syncable.h"
 #include "chrome/browser/sync/util/event_sys-inl.h"
 #include "chrome/browser/sync/util/path_helpers.h"
 
 namespace syncable {
 
-static const PSTR_CHAR kSyncDataDatabaseFilename[] = PSTR("SyncData.sqlite3");
+static const FilePath::CharType kSyncDataDatabaseFilename[] =
+    FILE_PATH_LITERAL("SyncData.sqlite3");
 
 DirectoryManagerEvent DirectoryManagerShutdownEvent() {
   DirectoryManagerEvent event;
@@ -25,18 +27,16 @@ DirectoryManagerEvent DirectoryManagerShutdownEvent() {
 }
 
 // static
-const PathString DirectoryManager::GetSyncDataDatabaseFilename() {
-  return PathString(kSyncDataDatabaseFilename);
+const FilePath DirectoryManager::GetSyncDataDatabaseFilename() {
+  return FilePath(kSyncDataDatabaseFilename);
 }
 
-const PathString DirectoryManager::GetSyncDataDatabasePath() const {
-  PathString path(root_path_);
-  path.append(kSyncDataDatabaseFilename);
-  return path;
+const FilePath DirectoryManager::GetSyncDataDatabasePath() const {
+  return root_path_.Append(GetSyncDataDatabaseFilename());
 }
 
-DirectoryManager::DirectoryManager(const PathString& path)
-    : root_path_(AppendSlash(path)),
+DirectoryManager::DirectoryManager(const FilePath& path)
+    : root_path_(path),
       managed_directory_(NULL),
       channel_(new Channel(DirectoryManagerShutdownEvent())) {
 }
@@ -68,7 +68,7 @@ bool DirectoryManager::Open(const PathString& name) {
 
 // Opens a directory.  Returns false on error.
 DirOpenResult DirectoryManager::OpenImpl(const PathString& name,
-                                         const PathString& path,
+                                         const FilePath& path,
                                          bool* was_open) {
   bool opened = false;
   {
@@ -85,13 +85,11 @@ DirOpenResult DirectoryManager::OpenImpl(const PathString& name,
     return syncable::OPENED;
   // Otherwise, open it.
 
-  Directory* dir = new Directory;
+  scoped_ptr<Directory> dir(new Directory);
   const DirOpenResult result = dir->Open(path, name);
   if (syncable::OPENED == result) {
     AutoLock lock(lock_);
-    managed_directory_ = dir;
-  } else {
-    delete dir;
+    managed_directory_ = dir.release();
   }
   return result;
 }
diff --git a/chrome/browser/sync/syncable/directory_manager.h b/chrome/browser/sync/syncable/directory_manager.h
index f059257..5d9a106 100644
--- a/chrome/browser/sync/syncable/directory_manager.h
+++ b/chrome/browser/sync/syncable/directory_manager.h
@@ -15,8 +15,9 @@
 #include <vector>
 
 #include "base/atomicops.h"
-#include "base/lock.h"
 #include "base/basictypes.h"
+#include "base/file_path.h"
+#include "base/lock.h"
 #include "chrome/browser/sync/syncable/dir_open_result.h"
 #include "chrome/browser/sync/syncable/path_name_cmp.h"
 #include "chrome/browser/sync/syncable/syncable.h"
@@ -50,11 +51,11 @@ class DirectoryManager {
   typedef EventChannel<DirectoryManagerEvent> Channel;
 
   // root_path specifies where db is stored.
-  explicit DirectoryManager(const PathString& root_path);
+  explicit DirectoryManager(const FilePath& root_path);
   ~DirectoryManager();
 
-  static const PathString GetSyncDataDatabaseFilename();
-  const PathString GetSyncDataDatabasePath() const;
+  static const FilePath GetSyncDataDatabaseFilename();
+  const FilePath GetSyncDataDatabasePath() const;
 
   // Opens a directory.  Returns false on error.
   // Name parameter is the the user's login,
@@ -75,13 +76,13 @@ class DirectoryManager {
   Channel* channel() const { return channel_; }
 
  protected:
-  DirOpenResult OpenImpl(const PathString& name, const PathString& path,
+  DirOpenResult OpenImpl(const PathString& name, const FilePath& path,
                          bool* was_open);
 
   // Helpers for friend class ScopedDirLookup:
   friend class ScopedDirLookup;
 
-  const PathString root_path_;
+  const FilePath root_path_;
 
   // protects managed_directory_
   Lock lock_;
diff --git a/chrome/browser/sync/syncable/syncable.cc b/chrome/browser/sync/syncable/syncable.cc
index 550c495..0ba9dab 100644
--- a/chrome/browser/sync/syncable/syncable.cc
+++ b/chrome/browser/sync/syncable/syncable.cc
@@ -26,9 +26,11 @@
 #include <string>
 
 #include "base/hash_tables.h"
+#include "base/file_util.h"
 #include "base/logging.h"
 #include "base/perftimer.h"
 #include "base/scoped_ptr.h"
+#include "base/string_util.h"
 #include "base/time.h"
 #include "chrome/browser/sync/engine/syncer.h"
 #include "chrome/browser/sync/engine/syncer_util.h"
@@ -38,8 +40,6 @@
 #include "chrome/browser/sync/syncable/syncable-inl.h"
 #include "chrome/browser/sync/syncable/syncable_changes_version.h"
 #include "chrome/browser/sync/syncable/syncable_columns.h"
-#include "chrome/browser/sync/util/character_set_converters.h"
-#include "chrome/browser/sync/util/compat_file.h"
 #include "chrome/browser/sync/util/crypto_helpers.h"
 #include "chrome/browser/sync/util/event_sys-inl.h"
 #include "chrome/browser/sync/util/fast_dump.h"
@@ -90,15 +90,6 @@ int64 Now() {
 // Callback for sqlite3
 int ComparePathNames16(void*, int a_bytes, const void* a, int b_bytes,
                        const void* b) {
-#if defined(OS_WIN)
-  DCHECK_EQ(0, a_bytes % 2);
-  DCHECK_EQ(0, b_bytes % 2);
-  int result = CompareString(LOCALE_INVARIANT, NORM_IGNORECASE,
-    static_cast<const PathChar*>(a), a_bytes / 2,
-    static_cast<const PathChar*>(b), b_bytes / 2);
-  CHECK(0 != result) << "Error comparing strings: " << GetLastError();
-  return result - 2;  // Convert to -1, 0, 1
-#elif defined(OS_LINUX)
   int result = base::strncasecmp(reinterpret_cast<const char *>(a),
                                  reinterpret_cast<const char *>(b),
                                  std::min(a_bytes, b_bytes));
@@ -107,21 +98,6 @@ int ComparePathNames16(void*, int a_bytes, const void* a, int b_bytes,
   } else {
     return a_bytes > b_bytes ? 1 : b_bytes > a_bytes ? -1 : 0;
   }
-#elif defined(OS_MACOSX)
-  CFStringRef a_str;
-  CFStringRef b_str;
-  a_str = CFStringCreateWithBytes(NULL, reinterpret_cast<const UInt8*>(a),
-                                  a_bytes, kCFStringEncodingUTF8, FALSE);
-  b_str = CFStringCreateWithBytes(NULL, reinterpret_cast<const UInt8*>(b),
-                                  b_bytes, kCFStringEncodingUTF8, FALSE);
-  CFComparisonResult res;
-  res = CFStringCompare(a_str, b_str, kCFCompareCaseInsensitive);
-  CFRelease(a_str);
-  CFRelease(b_str);
-  return res;
-#else
-#error no ComparePathNames16() for your OS
-#endif
 }
 
 template <Int64Field field_index>
@@ -177,7 +153,7 @@ Name Name::FromEntryKernel(EntryKernel* kernel) {
 static const DirectoryChangeEvent kShutdownChangesEvent =
     { DirectoryChangeEvent::SHUTDOWN, 0, 0 };
 
-Directory::Kernel::Kernel(const PathString& db_path,
+Directory::Kernel::Kernel(const FilePath& db_path,
                           const PathString& name,
                           const KernelLoadInfo& info)
 : db_path(db_path),
@@ -247,7 +223,7 @@ BOOL PathNameMatch(const PathString& pathname, const PathString& pathspec) {
      ++pathname_ptr, ++pathspec_ptr;
 
   // If we have more inital spaces in the pathspec than in the pathname then the
-  // result from PathMatchSpec will be erronous.
+  // result from PathMatchSpec will be erroneous.
   if (*pathspec_ptr == ' ')
     return FALSE;
 
@@ -257,10 +233,10 @@ BOOL PathNameMatch(const PathString& pathname, const PathString& pathspec) {
   // substituting ';' with ':' which is illegal character in file name and
   // we're not going to see it there. With ':' in path name and spec
   // PathMatchSpec works fine.
-  if ((NULL == wcschr(pathname_ptr, L';')) &&
-      (NULL == wcschr(pathspec_ptr, L';'))) {
+  if ((NULL == strchr(pathname_ptr, ';')) &&
+      (NULL == strchr(pathspec_ptr, ';'))) {
     // No ';' in file name and in spec. Just pass it as it is.
-    return ::PathMatchSpec(pathname_ptr, pathspec_ptr);
+    return ::PathMatchSpecA(pathname_ptr, pathspec_ptr);
   }
 
   // We need to subst ';' with ':' in both, name and spec.
@@ -269,23 +245,23 @@ BOOL PathNameMatch(const PathString& pathname, const PathString& pathspec) {
 
   PathString::size_type index = name_subst.find(L';');
   while (PathString::npos != index) {
-    name_subst[index] = L':';
-    index = name_subst.find(L';', index + 1);
+    name_subst[index] = ':';
+    index = name_subst.find(';', index + 1);
   }
 
   index = spec_subst.find(L';');
   while (PathString::npos != index) {
-    spec_subst[index] = L':';
-    index = spec_subst.find(L';', index + 1);
+    spec_subst[index] = ':';
+    index = spec_subst.find(';', index + 1);
   }
 
-  return ::PathMatchSpec(name_subst.c_str(), spec_subst.c_str());
+  return ::PathMatchSpecA(name_subst.c_str(), spec_subst.c_str());
 #else
   return 0 == ComparePathNames(pathname, pathspec);
 #endif
 }
 
-DirOpenResult Directory::Open(const PathString& file_path,
+DirOpenResult Directory::Open(const FilePath& file_path,
                               const PathString& name) {
   const DirOpenResult result = OpenImpl(file_path, name);
   if (OPENED != result)
@@ -308,18 +284,19 @@ void Directory::InitializeIndices() {
 }
 
 DirectoryBackingStore* Directory::CreateBackingStore(
-    const PathString& dir_name, const PathString& backing_filepath) {
+    const PathString& dir_name, const FilePath& backing_filepath) {
   return new DirectoryBackingStore(dir_name, backing_filepath);
 }
 
-DirOpenResult Directory::OpenImpl(const PathString& file_path,
+DirOpenResult Directory::OpenImpl(const FilePath& file_path,
                                   const PathString& name) {
   DCHECK_EQ(static_cast<DirectoryBackingStore*>(NULL), store_);
-  const PathString db_path = ::GetFullPath(file_path);
+  FilePath db_path(file_path);
+  file_util::AbsolutePath(&db_path);
   store_ = CreateBackingStore(name, db_path);
 
   KernelLoadInfo info;
-  // Temporary indicies before kernel_ initialized in case Load fails. We 0(1)
+  // Temporary indices before kernel_ initialized in case Load fails. We 0(1)
   // swap these later.
   MetahandlesIndex metas_bucket;
   ExtendedAttributes xattrs_bucket;
@@ -506,62 +483,6 @@ struct ExactPathMatcher : public PathMatcher {
   const PathString pathspec_;
 };
 
-// Matches a pathspec with wildcards.
-struct PartialPathMatcher : public PathMatcher {
-  PartialPathMatcher(const PathString& pathspec,
-                     PathString::size_type wildcard, const Id& parent_id)
-    : PathMatcher(parent_id), pathspec_(pathspec) {
-    if (0 == wildcard)
-      return;
-    lesser_.assign(pathspec_.data(), wildcard);
-    greater_.assign(pathspec_.data(), wildcard);
-    // Increment the last letter of greater so we can then less than
-    // compare to it.
-    PathString::size_type i = greater_.size() - 1;
-    do {
-    if (greater_[i] == std::numeric_limits<PathString::value_type>::max()) {
-        greater_.resize(i);  // Try the preceding character.
-        if (0 == i--)
-          break;
-      } else {
-        greater_[i] += 1;
-      }
-      // Yes, there are cases where incrementing a character
-      // actually decreases its position in the sort.  Example: 9 -> :
-    } while (ComparePathNames(lesser_, greater_) >= 0);
-  }
-
-  virtual MatchType PathMatches(const PathString& path) {
-    return PathNameMatch(path, pathspec_) ? MATCH : NO_MATCH;
-  }
-
-  virtual Index::iterator lower_bound(Index* index) {
-    needle_.ref(PARENT_ID) = parent_id_;
-    needle_.ref(NAME) = lesser_;
-    return index->lower_bound(&needle_);
-  }
-  virtual Index::iterator upper_bound(Index* index) {
-    if (greater_.empty()) {
-      needle_.ref(PARENT_ID) = parent_id_;
-      needle_.ref(NAME).clear();
-      Index::iterator i = index->upper_bound(&needle_),
-        end = index->end();
-      while (i != end && (*i)->ref(PARENT_ID) == parent_id_)
-        ++i;
-      return i;
-    } else {
-      needle_.ref(PARENT_ID) = parent_id_;
-      needle_.ref(NAME) = greater_;
-      return index->lower_bound(&needle_);
-    }
-  }
-
-  const PathString pathspec_;
-  PathString lesser_;
-  PathString greater_;
-};
-
-
 void Directory::GetChildHandles(BaseTransaction* trans, const Id& parent_id,
                                 Directory::ChildHandles* result) {
   AllPathsMatcher matcher(parent_id);
@@ -1658,25 +1579,7 @@ static int PathStringToInteger(PathString s) {
     if (PathString::npos == PathString(PSTR("0123456789")).find(*i))
       return -1;
   }
-  return
-#if !PATHSTRING_IS_STD_STRING
-  _wtoi
-#else
-  atoi
-#endif
-  (s.c_str());
-}
-
-static PathString IntegerToPathString(int i) {
-  const size_t kBufSize = 25;
-  PathChar buf[kBufSize];
-#if !PATHSTRING_IS_STD_STRING
-  const int radix = 10;
-  _itow(i, buf, radix);
-#else
-  snprintf(buf, kBufSize, "%d", i);
-#endif
-  return buf;
+  return atoi(s.c_str());
 }
 
 // appends ~1 to the end of 's' unless there is already ~#, in which case
@@ -1690,7 +1593,7 @@ static PathString FixBasenameInCollision(const PathString s) {
   if ((n = PathStringToInteger(s.substr(last_tilde + 1))) != -1) {
     n++;
     PathString pre_number = s.substr(0, last_tilde + 1);
-    return pre_number + IntegerToPathString(n);
+    return pre_number + IntToString(n);
   } else {
     // we have a ~, but not a number following it, so we'll add another
     // ~ and this time, a number
@@ -1724,31 +1627,6 @@ void DBName::MakeNoncollidingForEntry(BaseTransaction* trans,
   swap(new_value);
 }
 
-PathString GetFullPath(BaseTransaction* trans, const Entry& e) {
-  PathString result;
-#if defined(COMPILER_MSVC)
-  result.reserve(MAX_PATH);
-#endif
-  ReverseAppend(e.Get(NAME), &result);
-  Id id = e.Get(PARENT_ID);
-  while (!id.IsRoot()) {
-    result.push_back(kPathSeparator[0]);
-    Entry ancestor(trans, GET_BY_ID, id);
-    if (!ancestor.good()) {
-      // This can happen if the parent folder got deleted before the entry.
-      LOG(WARNING) << "Cannot get full path of " << e
-                   << "\nbecause an ancestor folder has been deleted.";
-      result.clear();
-      return result;
-    }
-    ReverseAppend(ancestor.Get(NAME), &result);
-    id = ancestor.Get(PARENT_ID);
-  }
-  result.push_back(kPathSeparator[0]);
-  reverse(result.begin(), result.end());
-  return result;
-}
-
 const Blob* GetExtendedAttributeValue(const Entry& e,
                                       const PathString& attribute_name) {
   ExtendedAttributeKey key(e.Get(META_HANDLE), attribute_name);
@@ -1788,7 +1666,6 @@ inline FastDump& operator<<(FastDump& dump, const DumpColon&) {
 std::ostream& operator<<(std::ostream& stream, const syncable::Entry& entry) {
   // Using ostreams directly here is dreadfully slow, because a mutex is
   // acquired for every <<.  Users noticed it spiking CPU.
-  using browser_sync::ToUTF8;
   using syncable::BitField;
   using syncable::BitTemp;
   using syncable::BlobField;
@@ -1823,8 +1700,8 @@ std::ostream& operator<<(std::ostream& stream, const syncable::Entry& entry) {
       s << g_metas_columns[i].name << separator;
   }
   for ( ; i < STRING_FIELDS_END; ++i) {
-    ToUTF8 field(kernel->ref(static_cast<StringField>(i)));
-    s << g_metas_columns[i].name << colon << field.get_string() << separator;
+    const PathString& field = kernel->ref(static_cast<StringField>(i));
+    s << g_metas_columns[i].name << colon << field << separator;
   }
   for ( ; i < BLOB_FIELDS_END; ++i) {
     s << g_metas_columns[i].name << colon
diff --git a/chrome/browser/sync/syncable/syncable.h b/chrome/browser/sync/syncable/syncable.h
index 5c6195e..c92ea7b 100644
--- a/chrome/browser/sync/syncable/syncable.h
+++ b/chrome/browser/sync/syncable/syncable.h
@@ -15,6 +15,7 @@
 
 #include "base/atomicops.h"
 #include "base/basictypes.h"
+#include "base/file_path.h"
 #include "base/lock.h"
 #include "base/time.h"
 #include "chrome/browser/sync/syncable/blob.h"
@@ -22,7 +23,6 @@
 #include "chrome/browser/sync/syncable/directory_event.h"
 #include "chrome/browser/sync/syncable/path_name_cmp.h"
 #include "chrome/browser/sync/syncable/syncable_id.h"
-#include "chrome/browser/sync/util/compat_file.h"
 #include "chrome/browser/sync/util/dbgq.h"
 #include "chrome/browser/sync/util/event_sys.h"
 #include "chrome/browser/sync/util/path_helpers.h"
@@ -319,6 +319,9 @@ class SyncName {
   PathString& value() { return value_; }
   const PathString& non_unique_value() const { return non_unique_value_; }
   PathString& non_unique_value() { return non_unique_value_; }
+  void set_non_unique_value(const PathString& value) {
+    non_unique_value_ = value;
+  }
 
   inline bool operator==(const SyncName& right_hand_side) const {
     return value_ == right_hand_side.value_ &&
@@ -886,7 +889,7 @@ class Directory {
   Directory();
   virtual ~Directory();
 
-  DirOpenResult Open(const PathString& file_path, const PathString& name);
+  DirOpenResult Open(const FilePath& file_path, const PathString& name);
 
   void Close();
 
@@ -895,7 +898,7 @@ class Directory {
   // by the server only.
   Id NextId();
 
-  PathString file_path() const { return kernel_->db_path; }
+  const FilePath& file_path() const { return kernel_->db_path; }
   bool good() const { return NULL != store_; }
 
   // The sync timestamp is an index into the list of changes for an account.
@@ -938,7 +941,7 @@ class Directory {
   // Overridden by tests.
   virtual DirectoryBackingStore* CreateBackingStore(
       const PathString& dir_name,
-      const PathString& backing_filepath);
+      const FilePath& backing_filepath);
 
  private:
   // These private versions expect the kernel lock to already be held
@@ -951,7 +954,7 @@ class Directory {
                                     const PathString& name,
                                     ScopedKernelLock* const lock);
 
-  DirOpenResult OpenImpl(const PathString& file_path, const PathString& name);
+  DirOpenResult OpenImpl(const FilePath& file_path, const PathString& name);
 
   struct DirectoryEventTraits {
     typedef DirectoryEvent EventType;
@@ -1086,12 +1089,12 @@ class Directory {
  private:
 
   struct Kernel {
-    Kernel(const PathString& db_path, const PathString& name,
+    Kernel(const FilePath& db_path, const PathString& name,
            const KernelLoadInfo& info);
 
     ~Kernel();
 
-    PathString const db_path;
+    FilePath const db_path;
     // TODO(timsteele): audit use of the member and remove if possible
     volatile base::subtle::AtomicWord refcount;
     void AddRef();  // For convenience.
@@ -1254,12 +1257,6 @@ int64 Now();
 // Does wildcard processing.
 BOOL PathNameMatch(const PathString& pathname, const PathString& pathspec);
 
-PathString GetFullPath(BaseTransaction* trans, const Entry& e);
-
-inline void ReverseAppend(const PathString& s, PathString* target) {
-  target->append(s.rbegin(), s.rend());
-}
-
 class ExtendedAttribute {
  public:
   ExtendedAttribute(BaseTransaction* trans, GetByHandle,
diff --git a/chrome/browser/sync/syncable/syncable_unittest.cc b/chrome/browser/sync/syncable/syncable_unittest.cc
index db5bc08..d5bcd8c 100644
--- a/chrome/browser/sync/syncable/syncable_unittest.cc
+++ b/chrome/browser/sync/syncable/syncable_unittest.cc
@@ -30,13 +30,14 @@
 #endif  // !defined(OS_WIN)
 
 #include "base/at_exit.h"
+#include "base/file_path.h"
+#include "base/file_util.h"
 #include "base/logging.h"
 #include "base/platform_thread.h"
 #include "base/scoped_ptr.h"
 #include "chrome/browser/sync/syncable/directory_backing_store.h"
 #include "chrome/browser/sync/syncable/directory_manager.h"
 #include "chrome/browser/sync/util/closure.h"
-#include "chrome/browser/sync/util/compat_file.h"
 #include "chrome/browser/sync/util/event_sys-inl.h"
 #include "chrome/browser/sync/util/path_helpers.h"
 #include "chrome/browser/sync/util/query_helpers.h"
@@ -78,7 +79,8 @@ void ExpectDataFromExtendedAttributeEquals(BaseTransaction* trans,
 TEST(Syncable, General) {
   remove("SimpleTest.sqlite3");
   Directory dir;
-  dir.Open(PSTR("SimpleTest.sqlite3"), PSTR("SimpleTest"));
+  FilePath test_db(FILE_PATH_LITERAL("SimpleTest.sqlite3"));
+  dir.Open(test_db, PSTR("SimpleTest"));
   bool entry_exists = false;
   int64 metahandle;
   const Id id = TestIdFactory::FromNumber(99);
@@ -176,14 +178,14 @@ class TestUnsaveableDirectory : public Directory {
   class UnsaveableBackingStore : public DirectoryBackingStore {
    public:
      UnsaveableBackingStore(const PathString& dir_name,
-                            const PathString& backing_filepath)
+                            const FilePath& backing_filepath)
          : DirectoryBackingStore(dir_name, backing_filepath) { }
      virtual bool SaveChanges(const Directory::SaveChangesSnapshot& snapshot) {
        return false;
      }
   };
   virtual DirectoryBackingStore* CreateBackingStore(
-      const PathString& dir_name, const PathString& backing_filepath) {
+      const PathString& dir_name, const FilePath& backing_filepath) {
     return new UnsaveableBackingStore(dir_name, backing_filepath);
   }
 };
@@ -191,18 +193,18 @@ class TestUnsaveableDirectory : public Directory {
 // Test suite for syncable::Directory.
 class SyncableDirectoryTest : public testing::Test {
  protected:
-  static const PathString kFilePath;
+  static const FilePath::CharType kFilePath[];
   static const PathString kName;
-  static const PathChar* kSqlite3File;
   static const Id kId;
 
   // SetUp() is called before each test case is run.
   // The sqlite3 DB is deleted before each test is run.
   virtual void SetUp() {
-    PathRemove(PathString(kSqlite3File));
+    file_path_ = FilePath(kFilePath);
+    file_util::Delete(file_path_, true);
     dir_.reset(new Directory());
     ASSERT_TRUE(dir_.get());
-    ASSERT_TRUE(OPENED == dir_->Open(kFilePath, kName));
+    ASSERT_TRUE(OPENED == dir_->Open(file_path_, kName));
     ASSERT_TRUE(dir_->good());
   }
 
@@ -210,10 +212,11 @@ class SyncableDirectoryTest : public testing::Test {
     // This also closes file handles.
     dir_->SaveChanges();
     dir_.reset();
-    PathRemove(PathString(kSqlite3File));
+    file_util::Delete(file_path_, true);
   }
 
   scoped_ptr<Directory> dir_;
+  FilePath file_path_;
 
   // Creates an empty entry and sets the ID field to the default kId.
   void CreateEntry(const PathString &entryname) {
@@ -239,8 +242,8 @@ class SyncableDirectoryTest : public testing::Test {
       bool set_server_fields, bool is_dir, bool add_to_lru, int64 *meta_handle);
 };
 
-const PathString SyncableDirectoryTest::kFilePath(PSTR("Test.sqlite3"));
-const PathChar* SyncableDirectoryTest::kSqlite3File(PSTR("Test.sqlite3"));
+const FilePath::CharType SyncableDirectoryTest::kFilePath[] =
+    FILE_PATH_LITERAL("Test.sqlite3");
 const PathString SyncableDirectoryTest::kName(PSTR("Foo"));
 const Id SyncableDirectoryTest::kId(TestIdFactory::FromNumber(-99));
 
@@ -291,27 +294,6 @@ TEST_F(SyncableDirectoryTest, TestDelete) {
   ASSERT_TRUE(e1.Put(IS_DEL, true));
 }
 
-TEST_F(SyncableDirectoryTest, TestGetFullPathNeverCrashes) {
-  PathString dirname = PSTR("honey"),
-      childname = PSTR("jelly");
-  WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
-  MutableEntry e1(&trans, CREATE, trans.root_id(), dirname);
-  ASSERT_TRUE(e1.good());
-  ASSERT_TRUE(e1.Put(IS_DIR, true));
-  MutableEntry e2(&trans, CREATE, e1.Get(ID), childname);
-  ASSERT_TRUE(e2.good());
-  PathString path = GetFullPath(&trans, e2);
-  ASSERT_FALSE(path.empty());
-  // Give the child a parent that doesn't exist.
-  e2.Put(PARENT_ID, TestIdFactory::FromNumber(42));
-  path = GetFullPath(&trans, e2);
-  ASSERT_TRUE(path.empty());
-  // Done testing, make sure CheckTreeInvariants doesn't choke.
-  e2.Put(PARENT_ID, e1.Get(ID));
-  e2.Put(IS_DEL, true);
-  e1.Put(IS_DEL, true);
-}
-
 TEST_F(SyncableDirectoryTest, TestGetUnsynced) {
   Directory::UnsyncedMetaHandles handles;
   int64 handle1, handle2;
@@ -723,7 +705,7 @@ TEST_F(SyncableDirectoryTest, TestSaveChangesFailure) {
 
   dir_.reset(new TestUnsaveableDirectory());
   ASSERT_TRUE(dir_.get());
-  ASSERT_TRUE(OPENED == dir_->Open(kFilePath, kName));
+  ASSERT_TRUE(OPENED == dir_->Open(FilePath(kFilePath), kName));
   ASSERT_TRUE(dir_->good());
   int64 handle2 = 0;
   {
@@ -775,13 +757,13 @@ void SyncableDirectoryTest::ValidateEntry(BaseTransaction* trans, int64 id,
 }
 
 TEST(SyncableDirectoryManager, TestFileRelease) {
-  DirectoryManager dm(PSTR("."));
+  DirectoryManager dm(FilePath(FILE_PATH_LITERAL(".")));
   ASSERT_TRUE(dm.Open(PSTR("ScopeTest")));
   {
     ScopedDirLookup(&dm, PSTR("ScopeTest"));
   }
   dm.Close(PSTR("ScopeTest"));
-  ASSERT_TRUE(0 == PathRemove(dm.GetSyncDataDatabasePath()));
+  ASSERT_TRUE(file_util::Delete(dm.GetSyncDataDatabasePath(), true));
 }
 
 class ThreadOpenTestDelegate : public PlatformThread::Delegate {
@@ -800,7 +782,7 @@ class ThreadOpenTestDelegate : public PlatformThread::Delegate {
 };
 
 TEST(SyncableDirectoryManager, ThreadOpenTest) {
-  DirectoryManager dm(PSTR("."));
+  DirectoryManager dm(FilePath(FILE_PATH_LITERAL(".")));
   PlatformThreadHandle thread_handle;
   ThreadOpenTestDelegate test_delegate(&dm);
   ASSERT_TRUE(PlatformThread::Create(0, &test_delegate, &thread_handle));
@@ -881,7 +863,7 @@ class ThreadBugDelegate : public PlatformThread::Delegate {
 TEST(SyncableDirectoryManager, ThreadBug1) {
   Step step;
   step.number = 0;
-  DirectoryManager dirman(PSTR("."));
+  DirectoryManager dirman(FilePath(FILE_PATH_LITERAL(".")));
   ThreadBugDelegate thread_delegate_1(0, &step, &dirman);
   ThreadBugDelegate thread_delegate_2(1, &step, &dirman);
 
@@ -918,7 +900,8 @@ class DirectoryKernelStalenessBugDelegate : public ThreadBugDelegate {
       case 0:
         {
           // Clean up remnants of earlier test runs.
-          PathRemove(directory_manager_->GetSyncDataDatabasePath());
+          file_util::Delete(directory_manager_->GetSyncDataDatabasePath(),
+                            true);
           // Test.
           directory_manager_->Open(dirname);
           ScopedDirLookup dir(directory_manager_, dirname);
@@ -974,7 +957,7 @@ class DirectoryKernelStalenessBugDelegate : public ThreadBugDelegate {
 TEST(SyncableDirectoryManager, DirectoryKernelStalenessBug) {
   Step step;
 
-  DirectoryManager dirman(PSTR("."));
+  DirectoryManager dirman(FilePath(FILE_PATH_LITERAL(".")));
   DirectoryKernelStalenessBugDelegate thread_delegate_1(0, &step, &dirman);
   DirectoryKernelStalenessBugDelegate thread_delegate_2(1, &step, &dirman);
 
@@ -1034,9 +1017,9 @@ class StressTransactionsDelegate : public PlatformThread::Delegate {
 };
 
 TEST(SyncableDirectory, StressTransactions) {
-  DirectoryManager dirman(PSTR("."));
+  DirectoryManager dirman(FilePath(FILE_PATH_LITERAL(".")));
   PathString dirname = PSTR("stress");
-  PathRemove(dirman.GetSyncDataDatabasePath());
+  file_util::Delete(dirman.GetSyncDataDatabasePath(), true);
   dirman.Open(dirname);
 
   const int kThreadCount = 7;
@@ -1055,7 +1038,7 @@ TEST(SyncableDirectory, StressTransactions) {
   }
 
   dirman.Close(dirname);
-  PathRemove(dirman.GetSyncDataDatabasePath());
+  file_util::Delete(dirman.GetSyncDataDatabasePath(), true);
 }
 
 TEST(Syncable, ComparePathNames) {