Remove unique naming from syncer! This reduces the complexity. Modify the index to store things with parent id and metahandle instead of parent id and name.

Add a template function for extracting name from proto buffers.

Refactor some unit tests to work without unique naming.
Remove path calls since they make no sense without unique names.

Remove find by parentID and names.

Remove unique name resolution from conflict resolver.

Remove syncable name class.

TEST=included unit tests

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

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

5 files changed, 208 insertions, 819 deletions

diff --git a/chrome/browser/sync/syncable/directory_backing_store.cc b/chrome/browser/sync/syncable/directory_backing_store.cc
index 173dc0b..45b72cb 100644..100755
--- a/chrome/browser/sync/syncable/directory_backing_store.cc
+++ b/chrome/browser/sync/syncable/directory_backing_store.cc
@@ -37,38 +37,7 @@ namespace syncable {
 static const string::size_type kUpdateStatementBufferSize = 2048;
 
 // Increment this version whenever updating DB tables.
-static const int32 kCurrentDBVersion = 67;
-
-#if OS_WIN
-// TODO(sync): remove
-static void PathNameMatch16(sqlite3_context* context, int argc,
-                            sqlite3_value** argv) {
-  const PathString pathspec(reinterpret_cast<const PathChar*>
-      (sqlite3_value_text16(argv[0])), sqlite3_value_bytes16(argv[0]) / 2);
-
-  const void* name_text = sqlite3_value_text16(argv[1]);
-  int name_bytes = sqlite3_value_bytes16(argv[1]);
-  // If the text is null, we need to avoid the PathString constructor.
-  if (name_text != NULL) {
-    // Have to copy to append a terminating 0 anyway.
-    const PathString name(reinterpret_cast<const PathChar*>
-        (sqlite3_value_text16(argv[1])),
-        sqlite3_value_bytes16(argv[1]) / 2);
-    sqlite3_result_int(context, PathNameMatch(name, pathspec));
-  } else {
-    sqlite3_result_int(context, PathNameMatch(PathString(), pathspec));
-  }
-}
-
-// Sqlite allows setting of the escape character in an ESCAPE clause and
-// this character is passed in as a third character to the like function.
-// See: http://www.sqlite.org/lang_expr.html
-static void PathNameMatch16WithEscape(sqlite3_context* context,
-                                      int argc, sqlite3_value** argv) {
-  // Never seen this called, but just in case.
-  LOG(FATAL) << "PathNameMatch16WithEscape() not implemented";
-}
-#endif
+static const int32 kCurrentDBVersion = 68;
 
 static void RegisterPathNameCollate(sqlite3* dbhandle) {
   const int collate = SQLITE_UTF8;
@@ -76,23 +45,6 @@ static void RegisterPathNameCollate(sqlite3* dbhandle) {
       NULL, &ComparePathNames16));
 }
 
-// Replace the LIKE operator with our own implementation that does file spec
-// matching like "*.txt".
-static void RegisterPathNameMatch(sqlite3* dbhandle) {
-  // We only register this on Windows. We use the normal sqlite
-  // matching function on mac/linux.
-  // note that the function PathNameMatch() does a simple ==
-  // comparison on mac, so that would have to be fixed if
-  // we really wanted to use PathNameMatch on mac/linux w/ the
-  // same pattern strings as we do on windows.
-#if defined(OS_WIN)
-  CHECK(SQLITE_OK == sqlite3_create_function(dbhandle, "like",
-      2, SQLITE_ANY, NULL, &PathNameMatch16, NULL, NULL));
-  CHECK(SQLITE_OK == sqlite3_create_function(dbhandle, "like",
-      3, SQLITE_ANY, NULL, &PathNameMatch16WithEscape, NULL, NULL));
-#endif  // OS_WIN
-}
-
 static inline bool IsSqliteErrorOurFault(int result) {
   switch (result) {
     case SQLITE_MISMATCH:
@@ -265,7 +217,7 @@ bool DirectoryBackingStore::OpenAndConfigureHandleHelper(
   if (SQLITE_OK == SqliteOpen(backing_filepath_, handle)) {
     sqlite3_busy_timeout(*handle, kDirectoryBackingStoreBusyTimeoutMs);
     RegisterPathNameCollate(*handle);
-    RegisterPathNameMatch(*handle);
+
     return true;
   }
   return false;
diff --git a/chrome/browser/sync/syncable/syncable.cc b/chrome/browser/sync/syncable/syncable.cc
index af2cf28..97dd525 100644..100755
--- a/chrome/browser/sync/syncable/syncable.cc
+++ b/chrome/browser/sync/syncable/syncable.cc
@@ -22,6 +22,7 @@
 #include <functional>
 #include <iomanip>
 #include <iterator>
+#include <limits>
 #include <set>
 #include <string>
 
@@ -88,6 +89,7 @@ int64 Now() {
 // Compare functions and hashes for the indices.
 
 // Callback for sqlite3
+// TODO(chron): This should be somewhere else
 int ComparePathNames16(void*, int a_bytes, const void* a, int b_bytes,
                        const void* b) {
   int result = base::strncasecmp(reinterpret_cast<const char *>(a),
@@ -118,35 +120,32 @@ class HashField {
   base::hash_set<int64> hasher_;
 };
 
-// TODO(ncarter): Rename!
+// TODO(chron): Remove this function.
 int ComparePathNames(const PathString& a, const PathString& b) {
   const size_t val_size = sizeof(PathString::value_type);
   return ComparePathNames16(NULL, a.size() * val_size, a.data(),
                                   b.size() * val_size, b.data());
 }
 
-class LessParentIdAndNames {
+class LessParentIdAndHandle {
  public:
   bool operator() (const syncable::EntryKernel* a,
                    const syncable::EntryKernel* b) const {
-    if (a->ref(PARENT_ID) != b->ref(PARENT_ID))
+    if (a->ref(PARENT_ID) != b->ref(PARENT_ID)) {
       return a->ref(PARENT_ID) < b->ref(PARENT_ID);
-    return ComparePathNames(a->ref(NAME), b->ref(NAME)) < 0;
+    }
+
+    // Meta handles are immutable per entry so this is ideal.
+    return a->ref(META_HANDLE) < b->ref(META_HANDLE);
   }
 };
 
+// TODO(chron): Remove this function.
 bool LessPathNames::operator() (const PathString& a,
                                 const PathString& b) const {
   return ComparePathNames(a, b) < 0;
 }
 
-// static
-Name Name::FromEntryKernel(EntryKernel* kernel) {
-  PathString& sync_name_ref = kernel->ref(UNSANITIZED_NAME).empty() ?
-      kernel->ref(NAME) : kernel->ref(UNSANITIZED_NAME);
-  return Name(kernel->ref(NAME), sync_name_ref, kernel->ref(NON_UNIQUE_NAME));
-}
-
 ///////////////////////////////////////////////////////////////////////////
 // Directory
 
@@ -161,7 +160,7 @@ Directory::Kernel::Kernel(const FilePath& db_path,
   name_(name),
   metahandles_index(new Directory::MetahandlesIndex),
   ids_index(new Directory::IdsIndex),
-  parent_id_and_names_index(new Directory::ParentIdAndNamesIndex),
+  parent_id_child_index(new Directory::ParentIdChildIndex),
   extended_attributes(new ExtendedAttributes),
   unapplied_update_metahandles(new MetahandleSet),
   unsynced_metahandles(new MetahandleSet),
@@ -196,7 +195,7 @@ Directory::Kernel::~Kernel() {
   delete unsynced_metahandles;
   delete unapplied_update_metahandles;
   delete extended_attributes;
-  delete parent_id_and_names_index;
+  delete parent_id_child_index;
   delete ids_index;
   for_each(metahandles_index->begin(), metahandles_index->end(), DeleteEntry);
   delete metahandles_index;
@@ -209,58 +208,6 @@ Directory::~Directory() {
   Close();
 }
 
-BOOL PathNameMatch(const PathString& pathname, const PathString& pathspec) {
-#if defined(OS_WIN)
-  // Note that if we go Vista only this is easier:
-  // http://msdn2.microsoft.com/en-us/library/ms628611.aspx
-
-  // PathMatchSpec strips spaces from the start of pathspec, so we compare those
-  // ourselves.
-  const PathChar* pathname_ptr = pathname.c_str();
-  const PathChar* pathspec_ptr = pathspec.c_str();
-
-  while (*pathname_ptr == ' ' && *pathspec_ptr == ' ')
-     ++pathname_ptr, ++pathspec_ptr;
-
-  // If we have more inital spaces in the pathspec than in the pathname then the
-  // result from PathMatchSpec will be erroneous.
-  if (*pathspec_ptr == ' ')
-    return FALSE;
-
-  // PathMatchSpec also gets "confused" when there are ';' characters in name or
-  // in spec. So, if we match (f.i.) ";" with ";" PathMatchSpec will return
-  // FALSE (which is wrong). Luckily for us, we can easily fix this by
-  // 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 == strchr(pathname_ptr, ';')) &&
-      (NULL == strchr(pathspec_ptr, ';'))) {
-    // No ';' in file name and in spec. Just pass it as it is.
-    return ::PathMatchSpecA(pathname_ptr, pathspec_ptr);
-  }
-
-  // We need to subst ';' with ':' in both, name and spec.
-  PathString name_subst(pathname_ptr);
-  PathString spec_subst(pathspec_ptr);
-
-  PathString::size_type index = name_subst.find(L';');
-  while (PathString::npos != index) {
-    name_subst[index] = ':';
-    index = name_subst.find(';', index + 1);
-  }
-
-  index = spec_subst.find(L';');
-  while (PathString::npos != index) {
-    spec_subst[index] = ':';
-    index = spec_subst.find(';', index + 1);
-  }
-
-  return ::PathMatchSpecA(name_subst.c_str(), spec_subst.c_str());
-#else
-  return 0 == ComparePathNames(pathname, pathspec);
-#endif
-}
-
 DirOpenResult Directory::Open(const FilePath& file_path,
                               const PathString& name) {
   const DirOpenResult result = OpenImpl(file_path, name);
@@ -274,7 +221,7 @@ void Directory::InitializeIndices() {
   for (; it != kernel_->metahandles_index->end(); ++it) {
     EntryKernel* entry = *it;
     if (!entry->ref(IS_DEL))
-      kernel_->parent_id_and_names_index->insert(entry);
+      kernel_->parent_id_child_index->insert(entry);
     kernel_->ids_index->insert(entry);
     if (entry->ref(IS_UNSYNCED))
       kernel_->unsynced_metahandles->insert(entry->ref(META_HANDLE));
@@ -377,61 +324,14 @@ EntryKernel* Directory::GetEntryByHandle(const int64 metahandle,
   return NULL;
 }
 
-EntryKernel* Directory::GetChildWithName(const Id& parent_id,
-                                         const PathString& name) {
-  ScopedKernelLock lock(this);
-  return GetChildWithName(parent_id, name, &lock);
-}
-
-// Will return child entry if the folder is opened,
-// otherwise it will return NULL.
-EntryKernel* Directory::GetChildWithName(const Id& parent_id,
-                                         const PathString& name,
-                                         ScopedKernelLock* const lock) {
-  PathString dbname = name;
-  EntryKernel* parent = GetEntryById(parent_id, lock);
-  if (parent == NULL)
-    return NULL;
-  return GetChildWithNameImpl(parent_id, dbname, lock);
-}
-
-// Will return child entry even when the folder is not opened. This is used by
-// syncer to apply update when folder is closed.
-EntryKernel* Directory::GetChildWithDBName(const Id& parent_id,
-                                           const PathString& name) {
-  ScopedKernelLock lock(this);
-  return GetChildWithNameImpl(parent_id, name, &lock);
-}
-
-EntryKernel* Directory::GetChildWithNameImpl(const Id& parent_id,
-                                             const PathString& name,
-                                             ScopedKernelLock* const lock) {
-  // First look up in memory:
-  kernel_->needle.ref(NAME) = name;
-  kernel_->needle.ref(PARENT_ID) = parent_id;
-  ParentIdAndNamesIndex::iterator found =
-    kernel_->parent_id_and_names_index->find(&kernel_->needle);
-  if (found != kernel_->parent_id_and_names_index->end()) {
-    // Found it in memory.  Easy.
-    return *found;
-  }
-  return NULL;
-}
-
 // An interface to specify the details of which children
 // GetChildHandles() is looking for.
+// TODO(chron): Clean this up into one function to get child handles
 struct PathMatcher {
   explicit PathMatcher(const Id& parent_id) : parent_id_(parent_id) { }
   virtual ~PathMatcher() { }
-  enum MatchType {
-    NO_MATCH,
-    MATCH,
-    // Means we found the only entry we're looking for in
-    // memory so we don't need to check the DB.
-    EXACT_MATCH
-  };
-  virtual MatchType PathMatches(const PathString& path) = 0;
-  typedef Directory::ParentIdAndNamesIndex Index;
+
+  typedef Directory::ParentIdChildIndex Index;
   virtual Index::iterator lower_bound(Index* index) = 0;
   virtual Index::iterator upper_bound(Index* index) = 0;
   const Id parent_id_;
@@ -439,48 +339,22 @@ struct PathMatcher {
 };
 
 // Matches all children.
+// TODO(chron): Unit test this by itself
 struct AllPathsMatcher : public PathMatcher {
   explicit AllPathsMatcher(const Id& parent_id) : PathMatcher(parent_id) {
   }
-  virtual MatchType PathMatches(const PathString& path) {
-    return MATCH;
-  }
-  virtual Index::iterator lower_bound(Index* index) {
-    needle_.ref(PARENT_ID) = parent_id_;
-    needle_.ref(NAME).clear();
-    return index->lower_bound(&needle_);
-  }
 
-  virtual Index::iterator upper_bound(Index* index) {
-    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;
-  }
-};
-
-// Matches an exact filename only; no wildcards.
-struct ExactPathMatcher : public PathMatcher {
-  ExactPathMatcher(const PathString& pathspec, const Id& parent_id)
-    : PathMatcher(parent_id), pathspec_(pathspec) {
-  }
-  virtual MatchType PathMatches(const PathString& path) {
-    return 0 == ComparePathNames(path, pathspec_) ? EXACT_MATCH : NO_MATCH;
-  }
   virtual Index::iterator lower_bound(Index* index) {
     needle_.ref(PARENT_ID) = parent_id_;
-    needle_.ref(NAME) = pathspec_;
+    needle_.ref(META_HANDLE) = std::numeric_limits<int64>::min();
     return index->lower_bound(&needle_);
   }
+
   virtual Index::iterator upper_bound(Index* index) {
     needle_.ref(PARENT_ID) = parent_id_;
-    needle_.ref(NAME) = pathspec_;
+    needle_.ref(META_HANDLE) = std::numeric_limits<int64>::max();
     return index->upper_bound(&needle_);
   }
-  const PathString pathspec_;
 };
 
 void Directory::GetChildHandles(BaseTransaction* trans, const Id& parent_id,
@@ -496,21 +370,17 @@ void Directory::GetChildHandlesImpl(BaseTransaction* trans, const Id& parent_id,
   result->clear();
   {
     ScopedKernelLock lock(this);
-    ParentIdAndNamesIndex* const index =
-      kernel_->parent_id_and_names_index;
-    typedef ParentIdAndNamesIndex::iterator iterator;
+
+    // This index is sorted by parent id and metahandle.
+    ParentIdChildIndex* const index = kernel_->parent_id_child_index;
+    typedef ParentIdChildIndex::iterator iterator;
     for (iterator i = matcher->lower_bound(index),
            end = matcher->upper_bound(index); i != end; ++i) {
       // root's parent_id is NULL in the db but 0 in memory, so
       // have avoid listing the root as its own child.
       if ((*i)->ref(ID) == (*i)->ref(PARENT_ID))
         continue;
-      PathMatcher::MatchType match = matcher->PathMatches((*i)->ref(NAME));
-      if (PathMatcher::NO_MATCH == match)
-        continue;
       result->push_back((*i)->ref(META_HANDLE));
-      if (PathMatcher::EXACT_MATCH == match)
-        return;
     }
   }
 }
@@ -519,17 +389,6 @@ EntryKernel* Directory::GetRootEntry() {
   return GetEntryById(Id());
 }
 
-EntryKernel* Directory::GetEntryByPath(const PathString& path) {
-  CHECK(kernel_);
-  EntryKernel* result = GetRootEntry();
-  CHECK(result) << "There should always be a root node.";
-  for (PathSegmentIterator<PathString> i(path), end;
-       i != end && NULL != result; ++i) {
-    result = GetChildWithName(result->ref(ID), *i);
-  }
-  return result;
-}
-
 void ZeroFields(EntryKernel* entry, int first_field) {
   int i = first_field;
   // Note that bitset<> constructor sets all bits to zero, and strings
@@ -554,29 +413,27 @@ void Directory::InsertEntry(EntryKernel* entry, ScopedKernelLock* lock) {
   CHECK(NULL != entry);
   static const char error[] = "Entry already in memory index.";
   CHECK(kernel_->metahandles_index->insert(entry).second) << error;
-  if (!entry->ref(IS_DEL))
-    CHECK(kernel_->parent_id_and_names_index->insert(entry).second) << error;
+
+  if (!entry->ref(IS_DEL)) {
+    CHECK(kernel_->parent_id_child_index->insert(entry).second) << error;
+  }
   CHECK(kernel_->ids_index->insert(entry).second) << error;
 }
 
-bool Directory::Undelete(EntryKernel* const entry) {
+void Directory::Undelete(EntryKernel* const entry) {
   DCHECK(entry->ref(IS_DEL));
   ScopedKernelLock lock(this);
-  if (NULL != GetChildWithName(entry->ref(PARENT_ID), entry->ref(NAME), &lock))
-    return false;  // Would have duplicated existing entry.
   entry->ref(IS_DEL) = false;
   entry->dirty[IS_DEL] = true;
-  CHECK(kernel_->parent_id_and_names_index->insert(entry).second);
-  return true;
+  CHECK(kernel_->parent_id_child_index->insert(entry).second);
 }
 
-bool Directory::Delete(EntryKernel* const entry) {
+void Directory::Delete(EntryKernel* const entry) {
   DCHECK(!entry->ref(IS_DEL));
   entry->ref(IS_DEL) = true;
   entry->dirty[IS_DEL] = true;
   ScopedKernelLock lock(this);
-  CHECK(1 == kernel_->parent_id_and_names_index->erase(entry));
-  return true;
+  CHECK(1 == kernel_->parent_id_child_index->erase(entry));
 }
 
 bool Directory::ReindexId(EntryKernel* const entry, const Id& new_id) {
@@ -589,30 +446,22 @@ bool Directory::ReindexId(EntryKernel* const entry, const Id& new_id) {
   return true;
 }
 
-bool Directory::ReindexParentIdAndName(EntryKernel* const entry,
-                                       const Id& new_parent_id,
-                                       const PathString& new_name) {
+void Directory::ReindexParentId(EntryKernel* const entry,
+                                const Id& new_parent_id) {
+
   ScopedKernelLock lock(this);
-  PathString new_indexed_name = new_name;
   if (entry->ref(IS_DEL)) {
     entry->ref(PARENT_ID) = new_parent_id;
-    entry->ref(NAME) = new_indexed_name;
-    return true;
+    return;
   }
 
-  // check for a case changing rename
-  if (entry->ref(PARENT_ID) == new_parent_id &&
-    0 == ComparePathNames(entry->ref(NAME), new_indexed_name)) {
-    entry->ref(NAME) = new_indexed_name;
-  } else {
-    if (NULL != GetChildWithName(new_parent_id, new_indexed_name, &lock))
-      return false;
-    CHECK(1 == kernel_->parent_id_and_names_index->erase(entry));
-    entry->ref(PARENT_ID) = new_parent_id;
-    entry->ref(NAME) = new_indexed_name;
-    CHECK(kernel_->parent_id_and_names_index->insert(entry).second);
+  if (entry->ref(PARENT_ID) == new_parent_id) {
+    return;
   }
-  return true;
+
+  CHECK(1 == kernel_->parent_id_child_index->erase(entry));
+  entry->ref(PARENT_ID) = new_parent_id;
+  CHECK(kernel_->parent_id_child_index->insert(entry).second);
 }
 
 // static
@@ -971,7 +820,7 @@ void Directory::CheckTreeInvariants(syncable::BaseTransaction* trans,
     }
     if (!e.Get(IS_DEL)) {
       CHECK(id != parentid) << e;
-      CHECK(!e.Get(NAME).empty()) << e;
+      CHECK(!e.Get(NON_UNIQUE_NAME).empty()) << e;
       int safety_count = handles.size() + 1;
       while (!parentid.IsRoot()) {
         if (!idfilter.ShouldConsider(parentid))
@@ -1148,24 +997,6 @@ Entry::Entry(BaseTransaction* trans, GetByHandle, int64 metahandle)
   kernel_ = trans->directory()->GetEntryByHandle(metahandle);
 }
 
-Entry::Entry(BaseTransaction* trans, GetByPath, const PathString& path)
-    : basetrans_(trans) {
-  kernel_ = trans->directory()->GetEntryByPath(path);
-}
-
-Entry::Entry(BaseTransaction* trans, GetByParentIdAndName, const Id& parentid,
-             const PathString& name)
-    : basetrans_(trans) {
-  kernel_ = trans->directory()->GetChildWithName(parentid, name);
-}
-
-Entry::Entry(BaseTransaction* trans, GetByParentIdAndDBName, const Id& parentid,
-             const PathString& name)
-    : basetrans_(trans) {
-  kernel_ = trans->directory()->GetChildWithDBName(parentid, name);
-}
-
-
 Directory* Entry::dir() const {
   return basetrans_->directory();
 }
@@ -1194,11 +1025,8 @@ void Entry::DeleteAllExtendedAttributes(WriteTransaction *trans) {
 
 MutableEntry::MutableEntry(WriteTransaction* trans, Create,
                            const Id& parent_id, const PathString& name)
-  : Entry(trans), write_transaction_(trans) {
-  if (NULL != trans->directory()->GetChildWithName(parent_id, name)) {
-    kernel_ = NULL;  // would have duplicated an existing entry.
-    return;
-  }
+    : Entry(trans),
+      write_transaction_(trans) {
   Init(trans, parent_id, name);
 }
 
@@ -1213,8 +1041,6 @@ void MutableEntry::Init(WriteTransaction* trans, const Id& parent_id,
   kernel_->dirty[META_HANDLE] = true;
   kernel_->ref(PARENT_ID) = parent_id;
   kernel_->dirty[PARENT_ID] = true;
-  kernel_->ref(NAME) = name;
-  kernel_->dirty[NAME] = true;
   kernel_->ref(NON_UNIQUE_NAME) = name;
   kernel_->dirty[NON_UNIQUE_NAME] = true;
   kernel_->ref(IS_NEW) = true;
@@ -1266,38 +1092,17 @@ MutableEntry::MutableEntry(WriteTransaction* trans, GetByHandle,
   trans->SaveOriginal(kernel_);
 }
 
-MutableEntry::MutableEntry(WriteTransaction* trans, GetByPath,
-                           const PathString& path)
-  : Entry(trans, GET_BY_PATH, path), write_transaction_(trans) {
-  trans->SaveOriginal(kernel_);
-}
-
-MutableEntry::MutableEntry(WriteTransaction* trans, GetByParentIdAndName,
-                           const Id& parentid, const PathString& name)
-  : Entry(trans, GET_BY_PARENTID_AND_NAME, parentid, name),
-    write_transaction_(trans) {
-  trans->SaveOriginal(kernel_);
-}
-
-MutableEntry::MutableEntry(WriteTransaction* trans, GetByParentIdAndDBName,
-                           const Id& parentid, const PathString& name)
-  : Entry(trans, GET_BY_PARENTID_AND_DBNAME, parentid, name),
-    write_transaction_(trans) {
-  trans->SaveOriginal(kernel_);
-}
-
 bool MutableEntry::PutIsDel(bool is_del) {
   DCHECK(kernel_);
-  if (is_del == kernel_->ref(IS_DEL))
+  if (is_del == kernel_->ref(IS_DEL)) {
     return true;
+  }
   if (is_del) {
     UnlinkFromOrder();
-    if (!dir()->Delete(kernel_))
-      return false;
+    dir()->Delete(kernel_);
     return true;
   } else {
-    if (!dir()->Undelete(kernel_))
-      return false;
+    dir()->Undelete(kernel_);
     PutPredecessor(Id());  // Restores position to the 0th index.
     return true;
   }
@@ -1319,8 +1124,8 @@ bool MutableEntry::Put(IdField field, const Id& value) {
       if (!dir()->ReindexId(kernel_, value))
         return false;
     } else if (PARENT_ID == field) {
-      if (!dir()->ReindexParentIdAndName(kernel_, value, kernel_->ref(NAME)))
-        return false;
+      dir()->ReindexParentId(kernel_, value);
+      PutPredecessor(Id());
     } else {
       kernel_->ref(field) = value;
     }
@@ -1345,13 +1150,7 @@ bool MutableEntry::Put(StringField field, const PathString& value) {
 bool MutableEntry::PutImpl(StringField field, const PathString& value) {
   DCHECK(kernel_);
   if (kernel_->ref(field) != value) {
-    if (NAME == field) {
-      if (!dir()->ReindexParentIdAndName(kernel_, kernel_->ref(PARENT_ID),
-          value))
-        return false;
-    } else {
-      kernel_->ref(field) = value;
-    }
+    kernel_->ref(field) = value;
     kernel_->dirty[static_cast<int>(field)] = true;
   }
   return true;
@@ -1377,29 +1176,6 @@ bool MutableEntry::Put(IndexedBitField field, bool value) {
   return true;
 }
 
-// Avoids temporary collision in index when renaming a bookmark to another
-// folder.
-bool MutableEntry::PutParentIdAndName(const Id& parent_id,
-                                      const Name& name) {
-  DCHECK(kernel_);
-  const bool parent_id_changes = parent_id != kernel_->ref(PARENT_ID);
-  bool db_name_changes = name.db_value() != kernel_->ref(NAME);
-  if (parent_id_changes || db_name_changes) {
-    if (!dir()->ReindexParentIdAndName(kernel_, parent_id,
-                                       name.db_value()))
-      return false;
-  }
-  Put(UNSANITIZED_NAME, name.GetUnsanitizedName());
-  Put(NON_UNIQUE_NAME, name.non_unique_value());
-  if (db_name_changes)
-    kernel_->dirty[NAME] = true;
-  if (parent_id_changes) {
-    kernel_->dirty[PARENT_ID] = true;
-    PutPredecessor(Id());  // Put in the 0th position.
-  }
-  return true;
-}
-
 void MutableEntry::UnlinkFromOrder() {
   Id old_previous = Get(PREV_ID);
   Id old_next = Get(NEXT_ID);
@@ -1572,61 +1348,6 @@ bool IsLegalNewParent(BaseTransaction* trans, const Id& entry_id,
   return true;
 }
 
-// returns -1 if s contains any non [0-9] characters
-static int PathStringToInteger(PathString s) {
-  PathString::const_iterator i = s.begin();
-  for (; i != s.end(); ++i) {
-    if (PathString::npos == PathString(PSTR("0123456789")).find(*i))
-      return -1;
-  }
-  return atoi(s.c_str());
-}
-
-// appends ~1 to the end of 's' unless there is already ~#, in which case
-// it just increments the number
-static PathString FixBasenameInCollision(const PathString s) {
-  PathString::size_type last_tilde = s.find_last_of(PSTR('~'));
-  if (PathString::npos == last_tilde) return s + PSTR("~1");
-  if (s.size() == (last_tilde + 1)) return s + PSTR("1");
-  // we have ~, but not necessarily ~# (for some number >= 0). check for that
-  int n;
-  if ((n = PathStringToInteger(s.substr(last_tilde + 1))) != -1) {
-    n++;
-    PathString pre_number = s.substr(0, last_tilde + 1);
-    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
-    return s + PSTR("~1");
-  }
-}
-
-void DBName::MakeNoncollidingForEntry(BaseTransaction* trans,
-                                      const Id& parent_id,
-                                      Entry* e) {
-  const PathString& desired_name = *this;
-  CHECK(!desired_name.empty());
-  PathString::size_type first_dot = desired_name.find_first_of(PSTR('.'));
-  if (PathString::npos == first_dot)
-    first_dot = desired_name.size();
-  PathString basename = desired_name.substr(0, first_dot);
-  PathString dotextension = desired_name.substr(first_dot);
-  CHECK(basename + dotextension == desired_name);
-  for (;;) {
-    // Check for collision.
-    PathString testname = basename + dotextension;
-    Entry same_path_entry(trans, GET_BY_PARENTID_AND_DBNAME,
-                          parent_id, testname);
-    if (!same_path_entry.good() || (e && same_path_entry.Get(ID) == e->Get(ID)))
-      break;
-    // There was a collision, so fix the name.
-    basename = FixBasenameInCollision(basename);
-  }
-  // Set our value to the new value.  This invalidates desired_name.
-  PathString new_value = basename + dotextension;
-  swap(new_value);
-}
-
 const Blob* GetExtendedAttributeValue(const Entry& e,
                                       const PathString& attribute_name) {
   ExtendedAttributeKey key(e.Get(META_HANDLE), attribute_name);
diff --git a/chrome/browser/sync/syncable/syncable.h b/chrome/browser/sync/syncable/syncable.h
index c92ea7b..1b541ed 100644..100755
--- a/chrome/browser/sync/syncable/syncable.h
+++ b/chrome/browser/sync/syncable/syncable.h
@@ -131,20 +131,9 @@ enum {
 };
 
 enum StringField {
-  // The name, transformed so as to be suitable for use as a path-element.  It
-  // is unique, and legal for this client.
-  NAME = STRING_FIELDS_BEGIN,
-  // The local name, pre-sanitization.  It is not necessarily unique.  If this
-  // is empty, it means |NAME| did not require sanitization.
-  UNSANITIZED_NAME,
-  // If NAME/UNSANITIZED_NAME are "Foo (2)", then NON_UNIQUE_NAME may be "Foo".
-  NON_UNIQUE_NAME,
-  // The server version of |NAME|.  It is uniquified, but not necessarily
-  // OS-legal.
-  SERVER_NAME,
-  // The server version of |NON_UNIQUE_NAME|.  Again, if SERVER_NAME is
-  // like "Foo (2)" due to a commit-time name aside, SERVER_NON_UNIQUE_NAME
-  // may hold the value "Foo".
+  // Name, will be truncated by server. Can be duplicated in a folder.
+  NON_UNIQUE_NAME = STRING_FIELDS_BEGIN,
+  // The server version of |NON_UNIQUE_NAME|.
   SERVER_NON_UNIQUE_NAME,
   // For bookmark entries, the URL of the bookmark.
   BOOKMARK_URL,
@@ -186,9 +175,6 @@ enum {
 enum BitTemp {
   SYNCING = BIT_TEMPS_BEGIN,
   IS_NEW,  // Means use INSERT instead of UPDATE to save to db.
-  DEPRECATED_DELETE_ON_CLOSE,  // Set by redirector, IS_OPEN must also be set.
-  DEPRECATED_CHANGED_SINCE_LAST_OPEN,  // Have we been written to since we've
-                                       // been opened.
   BIT_TEMPS_END,
 };
 
@@ -223,19 +209,6 @@ enum GetByHandle {
   GET_BY_HANDLE
 };
 
-enum GetByPath {
-  GET_BY_PATH
-};
-
-enum GetByParentIdAndName {
-  GET_BY_PARENTID_AND_NAME
-};
-
-// DBName is the name stored in the database.
-enum GetByParentIdAndDBName {
-  GET_BY_PARENTID_AND_DBNAME
-};
-
 enum Create {
   CREATE
 };
@@ -246,182 +219,6 @@ enum CreateNewUpdateItem {
 
 typedef std::set<PathString> AttributeKeySet;
 
-// DBName is a PathString with additional transformation methods that are
-// useful when trying to derive a unique and legal database name from an
-// unsanitized sync name.
-class DBName : public PathString {
- public:
-  explicit DBName(const PathString& database_name)
-      : PathString(database_name) { }
-
-  // TODO(ncarter): Remove these codepaths to maintain alternate titles which
-  // are OS legal filenames, Chrome doesn't depend on this like some other
-  // browsers do.
-  void MakeOSLegal() {
-    PathString new_value = MakePathComponentOSLegal(*this);
-    if (!new_value.empty())
-      swap(new_value);
-  }
-
-  // Modify the value of this DBName so that it is not in use by any entry
-  // inside |parent_id|, except maybe |e|.  |e| may be NULL if you are trying
-  // to compute a name for an entry which has yet to be created.
-  void MakeNoncollidingForEntry(BaseTransaction* trans,
-                                const Id& parent_id,
-                                Entry* e);
-};
-
-// SyncName encapsulates a canonical server name.  In general, when we need to
-// muck around with a name that the server sends us (e.g. to make it OS legal),
-// we try to preserve the original value in a SyncName,
-// and distill the new local value into a DBName.
-// At other times, we need to apply transforms in the
-// other direction -- that is, to create a server-appropriate SyncName from a
-// user-updated DBName (which is an OS legal name, but not necessarily in the
-// format that the server wants it to be).  For that sort of thing, you should
-// initialize a SyncName from the DB name value, and use the methods of
-// SyncName to canonicalize it.  At other times, you have a pair of canonical
-// server values -- one (the "value") which is unique in the parent, and another
-// (the "non unique value") which is not unique in the parent -- and you
-// simply want to create a SyncName to hold them as a pair.
-class SyncName {
- public:
-  // Create a SyncName with the initially specified value.
-  explicit SyncName(const PathString& sync_name)
-      : value_(sync_name), non_unique_value_(sync_name) { }
-
-  // Create a SyncName by specifying a value and a non-unique value.  If
-  // you use this constructor, the values you provide should already be
-  // acceptable server names.  Don't use the mutation/sanitization methods
-  // on the resulting instance -- mutation won't work if you have distinct
-  // values for the unique and non-unique fields.
-  SyncName(const PathString& unique_sync_name,
-           const PathString& non_unique_sync_name)
-      : value_(unique_sync_name), non_unique_value_(non_unique_sync_name) { }
-
-  // Transform |value_| so that it's a legal server name.
-  void MakeServerLegal() {
-    DCHECK_EQ(value_, non_unique_value_)
-        << "Deriving value_ will overwrite non_unique_value_.";
-    // Append a trailing space if the value is one of the server's three
-    // forbidden special cases.
-    if (value_.empty() ||
-        value_ == PSTR(".") ||
-        value_ == PSTR("..")) {
-      value_.append(PSTR(" "));
-      non_unique_value_ = value_;
-    }
-    // TODO(ncarter): Handle server's other requirement: truncation to 256
-    // bytes in Unicode NFC.
-  }
-
-  const PathString& value() const { return value_; }
-  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_ &&
-        non_unique_value_ == right_hand_side.non_unique_value_;
-  }
-  inline bool operator!=(const SyncName& right_hand_side) const {
-    return !(*this == right_hand_side);
-  }
- private:
-  PathString value_;
-  PathString non_unique_value_;
-};
-
-// Name is a SyncName which has an additional DBName that provides a way to
-// interpolate the "unsanitized name" according to the syncable convention.
-//
-// A method might accept a Name as an parameter when the sync and database
-// names need to be set simultaneously:
-//
-//    void PutName(const Name& new_name) {
-//       Put(NAME, new_name.db_value());
-//       Put(UNSANITIZED_NAME, new_name.GetUnsanitizedName());
-//    }
-//
-// A code point that is trying to convert between local database names and
-// server sync names can use Name to help with the conversion:
-//
-//    SyncName server_name = entry->GetServerName();
-//    Name name = Name::FromSyncName(server_name);  // Initially, name.value()
-//                                                  // and name.db_value() are
-//                                                  // equal to
-//                                                  // server_name.value().
-//    name.db_value().MakeOSLegal();  // Updates name.db_value in-place,
-//                                    // leaving name.value() unchanged.
-//    foo->PutName(name);
-//
-class Name : public SyncName {
- public:
-  // Create a Name with an initially specified db_value and value.
-  Name(const PathString& db_name, const PathString& sync_name)
-      : SyncName(sync_name), db_value_(db_name) { }
-
-  // Create a Name by specifying the db name, sync name, and non-unique
-  // sync name values.
-  Name(const PathString& db_name, const PathString& sync_name,
-       const PathString& non_unique_sync_name)
-      : SyncName(sync_name, non_unique_sync_name), db_value_(db_name) { }
-
-  // Create a Name with all name values initially equal to the the single
-  // specified argument.
-  explicit Name(const PathString& sync_and_db_name)
-      : SyncName(sync_and_db_name), db_value_(sync_and_db_name) { }
-
-  // Create a Name using the local (non-SERVER) fields of an EntryKernel.
-  static Name FromEntryKernel(struct EntryKernel*);
-
-  // Create a Name from a SyncName.  db_value is initially sync_name.value().
-  // non_unique_value() and value() are copied from |sync_name|.
-  static Name FromSyncName(const SyncName& sync_name) {
-    return Name(sync_name.value(), sync_name.value(),
-                sync_name.non_unique_value());
-  }
-
-  static Name FromDBNameAndSyncName(const PathString& db_name,
-                                    const SyncName& sync_name) {
-    return Name(db_name, sync_name.value(), sync_name.non_unique_value());
-  }
-
-  // Get the database name.  The non-const version is useful for in-place
-  // mutation.
-  const DBName& db_value() const { return db_value_; }
-  DBName& db_value() { return db_value_; }
-
-  // Do the sync names and database names differ?  This indicates that
-  // the sync name has been sanitized, and that GetUnsanitizedName() will
-  // be non-empty.
-  bool HasBeenSanitized() const { return db_value_ != value(); }
-
-  // Compute the value of the unsanitized name from the current sync and db
-  // name values.  The unsanitized name is the sync name value, unless the sync
-  // name is the same as the db name value, in which case the unsanitized name
-  // is empty.
-  PathString GetUnsanitizedName() const {
-    return HasBeenSanitized() ? value() : PathString();
-  }
-
-  inline bool operator==(const Name& right_hand_side) const {
-    return this->SyncName::operator==(right_hand_side) &&
-        db_value_ == right_hand_side.db_value_;
-  }
-  inline bool operator!=(const Name& right_hand_side) const {
-    return !(*this == right_hand_side);
-  }
-
- private:
-  // The database name, which is maintained to be a legal and unique-in-parent
-  // name.
-  DBName db_value_;
-};
-
 // Why the singular enums?  So the code compile-time dispatches instead of
 // runtime dispatches as it would with a single enum and an if() statement.
 
@@ -515,11 +312,6 @@ class Entry {
   Entry(BaseTransaction* trans, GetByHandle, int64 handle);
   Entry(BaseTransaction* trans, GetById, const Id& id);
   Entry(BaseTransaction* trans, GetByTag, const PathString& tag);
-  Entry(BaseTransaction* trans, GetByPath, const PathString& path);
-  Entry(BaseTransaction* trans, GetByParentIdAndName, const Id& id,
-      const PathString& name);
-  Entry(BaseTransaction* trans, GetByParentIdAndDBName, const Id& id,
-      const PathString& name);
 
   bool good() const { return 0 != kernel_; }
 
@@ -563,31 +355,12 @@ class Entry {
     DCHECK(kernel_);
     return kernel_->ref(field);
   }
-  inline Name GetName() const {
-    DCHECK(kernel_);
-    return Name::FromEntryKernel(kernel_);
-  }
-  inline SyncName GetServerName() const {
-    DCHECK(kernel_);
-    return SyncName(kernel_->ref(SERVER_NAME),
-                    kernel_->ref(SERVER_NON_UNIQUE_NAME));
-  }
-  inline bool SyncNameMatchesServerName() const {
-    DCHECK(kernel_);
-    SyncName sync_name(GetName());
-    return sync_name == GetServerName();
-  }
-  inline PathString GetSyncNameValue() const {
-    DCHECK(kernel_);
-    // This should always be equal to GetName().sync_name().value(), but may be
-    // faster.
-    return kernel_->ref(UNSANITIZED_NAME).empty() ? kernel_->ref(NAME) :
-        kernel_->ref(UNSANITIZED_NAME);
-  }
-  inline bool ExistsOnClientBecauseDatabaseNameIsNonEmpty() const {
+
+  inline bool ExistsOnClientBecauseNameIsNonEmpty() const {
     DCHECK(kernel_);
-    return !kernel_->ref(NAME).empty();
+    return !kernel_->ref(NON_UNIQUE_NAME).empty();
   }
+
   inline bool IsRoot() const {
     DCHECK(kernel_);
     return kernel_->ref(ID).IsRoot();
@@ -635,11 +408,6 @@ class MutableEntry : public Entry {
   MutableEntry(WriteTransaction* trans, CreateNewUpdateItem, const Id& id);
   MutableEntry(WriteTransaction* trans, GetByHandle, int64);
   MutableEntry(WriteTransaction* trans, GetById, const Id&);
-  MutableEntry(WriteTransaction* trans, GetByPath, const PathString& path);
-  MutableEntry(WriteTransaction* trans, GetByParentIdAndName, const Id&,
-    const PathString& name);
-  MutableEntry(WriteTransaction* trans, GetByParentIdAndDBName,
-    const Id& parentid, const PathString& name);
 
   inline WriteTransaction* write_transaction() const {
     return write_transaction_;
@@ -648,19 +416,12 @@ class MutableEntry : public Entry {
   // Field Accessors.  Some of them trigger the re-indexing of the entry.
   // Return true on success, return false on failure, which means
   // that putting the value would have caused a duplicate in the index.
+  // TODO(chron): Remove some of these unecessary return values.
   bool Put(Int64Field field, const int64& value);
   bool Put(IdField field, const Id& value);
   bool Put(StringField field, const PathString& value);
   bool Put(BaseVersion field, int64 value);
-  inline bool PutName(const Name& name) {
-    return (Put(NAME, name.db_value()) &&
-            Put(UNSANITIZED_NAME, name.GetUnsanitizedName()) &&
-            Put(NON_UNIQUE_NAME, name.non_unique_value()));
-  }
-  inline bool PutServerName(const SyncName& server_name) {
-    return (Put(SERVER_NAME, server_name.value()) &&
-            Put(SERVER_NON_UNIQUE_NAME, server_name.non_unique_value()));
-  }
+
   inline bool Put(BlobField field, const Blob& value) {
     return PutField(field, value);
   }
@@ -672,10 +433,6 @@ class MutableEntry : public Entry {
   }
   bool Put(IndexedBitField field, bool value);
 
-  // Avoids temporary collision in index when renaming a bookmark into another
-  // folder.
-  bool PutParentIdAndName(const Id& parent_id, const Name& name);
-
   // Sets the position of this item, and updates the entry kernels of the
   // adjacent siblings so that list invariants are maintained.  Returns false
   // and fails if |predecessor_id| does not identify a sibling.  Pass the root
@@ -732,7 +489,7 @@ template <Int64Field field_index>
 class SameField;
 template <Int64Field field_index>
 class HashField;
-class LessParentIdAndNames;
+class LessParentIdAndHandle;
 class LessMultiIncusionTargetAndMetahandle;
 template <typename FieldType, FieldType field_index>
 class LessField;
@@ -922,21 +679,18 @@ class Directory {
   std::string cache_guid() const;
 
  protected:  // for friends, mainly used by Entry constructors
-  EntryKernel* GetChildWithName(const Id& parent_id, const PathString& name);
-  EntryKernel* GetChildWithDBName(const Id& parent_id, const PathString& name);
   EntryKernel* GetEntryByHandle(const int64 handle);
   EntryKernel* GetEntryByHandle(const int64 metahandle, ScopedKernelLock* lock);
   EntryKernel* GetEntryById(const Id& id);
   EntryKernel* GetEntryByTag(const PathString& tag);
   EntryKernel* GetRootEntry();
-  EntryKernel* GetEntryByPath(const PathString& path);
   bool ReindexId(EntryKernel* const entry, const Id& new_id);
-  bool ReindexParentIdAndName(EntryKernel* const entry, const Id& new_parent_id,
-                              const PathString& new_name);
-  // These don't do the semantic checking that the redirector needs.
+  void ReindexParentId(EntryKernel* const entry, const Id& new_parent_id);
+
+  // These don't do semantic checking.
   // The semantic checking is implemented higher up.
-  bool Undelete(EntryKernel* const entry);
-  bool Delete(EntryKernel* const entry);
+  void Undelete(EntryKernel* const entry);
+  void Delete(EntryKernel* const entry);
 
   // Overridden by tests.
   virtual DirectoryBackingStore* CreateBackingStore(
@@ -947,12 +701,6 @@ class Directory {
   // These private versions expect the kernel lock to already be held
   // before calling.
   EntryKernel* GetEntryById(const Id& id, ScopedKernelLock* const lock);
-  EntryKernel* GetChildWithName(const Id& parent_id,
-                                const PathString& name,
-                                ScopedKernelLock* const lock);
-  EntryKernel* GetChildWithNameImpl(const Id& parent_id,
-                                    const PathString& name,
-                                    ScopedKernelLock* const lock);
 
   DirOpenResult OpenImpl(const FilePath& file_path, const PathString& name);
 
@@ -1081,9 +829,10 @@ class Directory {
   typedef std::set<EntryKernel*, LessField<IdField, ID> > IdsIndex;
   // All entries in memory must be in both the MetahandlesIndex and
   // the IdsIndex, but only non-deleted entries will be the
-  // ParentIdAndNamesIndex, because there can be multiple deleted
-  // entries with the same parent id and name.
-  typedef std::set<EntryKernel*, LessParentIdAndNames> ParentIdAndNamesIndex;
+  // ParentIdChildIndex.
+  // This index contains EntryKernels ordered by parent ID and metahandle.
+  // It allows efficient lookup of the children of a given parent.
+  typedef std::set<EntryKernel*, LessParentIdAndHandle> ParentIdChildIndex;
   typedef std::vector<int64> MetahandlesToPurge;
 
  private:
@@ -1116,7 +865,7 @@ class Directory {
     Lock mutex;
     MetahandlesIndex* metahandles_index;  // Entries indexed by metahandle
     IdsIndex* ids_index;  // Entries indexed by id
-    ParentIdAndNamesIndex* parent_id_and_names_index;
+    ParentIdChildIndex* parent_id_child_index;
     // So we don't have to create an EntryKernel every time we want to
     // look something up in an index.  Needle in haystack metaphore.
     EntryKernel needle;
@@ -1179,7 +928,7 @@ class ScopedKernelLock {
   DISALLOW_COPY_AND_ASSIGN(ScopedKernelLock);
 };
 
-// Transactions are now processed FIFO (+overlapping reads).
+// Transactions are now processed FIFO with a straight lock
 class BaseTransaction {
   friend class Entry;
  public:
@@ -1254,9 +1003,6 @@ int ComparePathNames16(void*, int a_bytes, const void* a, int b_bytes,
 
 int64 Now();
 
-// Does wildcard processing.
-BOOL PathNameMatch(const PathString& pathname, const PathString& pathspec);
-
 class ExtendedAttribute {
  public:
   ExtendedAttribute(BaseTransaction* trans, GetByHandle,
diff --git a/chrome/browser/sync/syncable/syncable_columns.h b/chrome/browser/sync/syncable/syncable_columns.h
index cd6f281..92caf64 100644..100755
--- a/chrome/browser/sync/syncable/syncable_columns.h
+++ b/chrome/browser/sync/syncable/syncable_columns.h
@@ -50,11 +50,8 @@ static const ColumnSpec g_metas_columns[] = {
   {"server_is_bookmark_object", "bit default 0"},
   //////////////////////////////////////
   // Strings
-  {"name", "varchar(255) COLLATE PATHNAME"},
-  {"unsanitized_name", "varchar(255) COLLATE PATHNAME"},
   {"non_unique_name", "varchar"},
-  {"server_name", "varchar(255) COLLATE PATHNAME"},
-  {"server_non_unique_name", "varchar"},
+  {"server_non_unique_name", "varchar(255) COLLATE PATHNAME"},
   {"bookmark_url", "varchar"},
   {"server_bookmark_url", "varchar"},
   {"singleton_tag", "varchar"},
diff --git a/chrome/browser/sync/syncable/syncable_unittest.cc b/chrome/browser/sync/syncable/syncable_unittest.cc
index d5bcd8c..2911a46 100644..100755
--- a/chrome/browser/sync/syncable/syncable_unittest.cc
+++ b/chrome/browser/sync/syncable/syncable_unittest.cc
@@ -42,6 +42,7 @@
 #include "chrome/browser/sync/util/path_helpers.h"
 #include "chrome/browser/sync/util/query_helpers.h"
 #include "chrome/test/sync/engine/test_id_factory.h"
+#include "chrome/test/sync/engine/test_syncable_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/sqlite/preprocessed/sqlite3.h"
 
@@ -52,6 +53,7 @@ using std::string;
 
 namespace syncable {
 
+namespace {
 // A lot of these tests were written expecting to be able to read and write
 // object data on entries.  However, the design has changed.
 void PutDataAsExtendedAttribute(WriteTransaction* wtrans,
@@ -68,106 +70,96 @@ void ExpectDataFromExtendedAttributeEquals(BaseTransaction* trans,
                                            Entry* e,
                                            const char* bytes,
                                            size_t bytes_length) {
+  ASSERT_TRUE(e->good());
   Blob expected_value(bytes, bytes + bytes_length);
   ExtendedAttributeKey key(e->Get(META_HANDLE), PSTR("DATA"));
   ExtendedAttribute attr(trans, GET_BY_HANDLE, key);
   EXPECT_FALSE(attr.is_deleted());
   EXPECT_EQ(expected_value, attr.value());
 }
-
+}  // namespace
 
 TEST(Syncable, General) {
   remove("SimpleTest.sqlite3");
   Directory dir;
   FilePath test_db(FILE_PATH_LITERAL("SimpleTest.sqlite3"));
   dir.Open(test_db, PSTR("SimpleTest"));
-  bool entry_exists = false;
-  int64 metahandle;
+
+  int64 written_metahandle;
   const Id id = TestIdFactory::FromNumber(99);
-  // Test simple read operations.
+  PathString name = PSTR("Jeff");
+  // Test simple read operations on an empty DB.
   {
     ReadTransaction rtrans(&dir, __FILE__, __LINE__);
     Entry e(&rtrans, GET_BY_ID, id);
-    if (e.good()) {
-      entry_exists = true;
-      metahandle = e.Get(META_HANDLE);
-    }
+    ASSERT_FALSE(e.good());  // Hasn't been written yet.
+
     Directory::ChildHandles child_handles;
     dir.GetChildHandles(&rtrans, rtrans.root_id(), &child_handles);
-    for (Directory::ChildHandles::iterator i = child_handles.begin();
-         i != child_handles.end(); ++i)
-      cout << *i << endl;
-
-    Entry e2(&rtrans, GET_BY_PATH, PSTR("/Hello\\World/"));
+    EXPECT_TRUE(child_handles.empty());
   }
 
   // Test creating a new meta entry.
   {
     WriteTransaction wtrans(&dir, UNITTEST, __FILE__, __LINE__);
-    MutableEntry me(&wtrans, CREATE, wtrans.root_id(), PSTR("Jeff"));
-    ASSERT_TRUE(entry_exists ? !me.good() : me.good());
-    if (me.good()) {
-      me.Put(ID, id);
-      me.Put(BASE_VERSION, 1);
-      metahandle = me.Get(META_HANDLE);
+    MutableEntry me(&wtrans, CREATE, wtrans.root_id(), name);
+    ASSERT_TRUE(me.good());
+    me.Put(ID, id);
+    me.Put(BASE_VERSION, 1);
+    written_metahandle = me.Get(META_HANDLE);
+  }
+
+  // Test GetChildHandles after something is now in the DB.
+  // Also check that GET_BY_ID works.
+  {
+    ReadTransaction rtrans(&dir, __FILE__, __LINE__);
+    Entry e(&rtrans, GET_BY_ID, id);
+    ASSERT_TRUE(e.good());
+
+    Directory::ChildHandles child_handles;
+    dir.GetChildHandles(&rtrans, rtrans.root_id(), &child_handles);
+    EXPECT_EQ(1u, child_handles.size());
+
+    for (Directory::ChildHandles::iterator i = child_handles.begin();
+         i != child_handles.end(); ++i) {
+      EXPECT_EQ(*i, written_metahandle);
     }
   }
 
-  // Test writing data to an entity.
+  // Test writing data to an entity. Also check that GET_BY_HANDLE works.
   static const char s[] = "Hello World.";
   {
     WriteTransaction trans(&dir, UNITTEST, __FILE__, __LINE__);
-    MutableEntry e(&trans, GET_BY_PATH,
-                   PathString(kPathSeparator) + PSTR("Jeff"));
+    MutableEntry e(&trans, GET_BY_HANDLE, written_metahandle);
     ASSERT_TRUE(e.good());
     PutDataAsExtendedAttribute(&trans, &e, s, sizeof(s));
   }
 
-  // Test reading back the name contents that we just wrote.
+  // Test reading back the contents that we just wrote.
   {
     WriteTransaction trans(&dir, UNITTEST, __FILE__, __LINE__);
-    MutableEntry e(&trans, GET_BY_PATH,
-                   PathString(kPathSeparator) + PSTR("Jeff"));
+    MutableEntry e(&trans, GET_BY_HANDLE, written_metahandle);
     ASSERT_TRUE(e.good());
     ExpectDataFromExtendedAttributeEquals(&trans, &e, s, sizeof(s));
   }
 
+  // Verify it exists in the folder.
+  {
+    ReadTransaction rtrans(&dir, __FILE__, __LINE__);
+    EXPECT_EQ(1, CountEntriesWithName(&rtrans, rtrans.root_id(), name));
+  }
+
   // Now delete it.
   {
     WriteTransaction trans(&dir, UNITTEST, __FILE__, __LINE__);
-    MutableEntry e(&trans, CREATE, trans.root_id(), PSTR("New File"));
+    MutableEntry e(&trans, GET_BY_HANDLE, written_metahandle);
     e.Put(IS_DEL, true);
+
+    EXPECT_EQ(0, CountEntriesWithName(&trans, trans.root_id(), name));
   }
 
   dir.SaveChanges();
-}
-
-TEST(Syncable, NameClassTest) {
-  const PathString foo(PSTR("foo"));
-  const PathString bar(PSTR("bar"));
-
-  Name name1(foo);
-  EXPECT_EQ(name1.value(), foo);
-  EXPECT_EQ(name1.db_value(), foo);
-  EXPECT_FALSE(name1.HasBeenSanitized());
-  EXPECT_TRUE(name1.GetUnsanitizedName().empty());
-
-  Name name2(foo, foo);
-  EXPECT_EQ(name2.value(), foo);
-  EXPECT_EQ(name2.db_value(), foo);
-  EXPECT_FALSE(name2.HasBeenSanitized());
-  EXPECT_TRUE(name2.GetUnsanitizedName().empty());
-
-  Name name3(foo, bar);
-  EXPECT_EQ(name3.value(), bar);
-  EXPECT_EQ(name3.db_value(), foo);
-  EXPECT_TRUE(name3.HasBeenSanitized());
-  EXPECT_EQ(name3.GetUnsanitizedName(), bar);
-
-  EXPECT_TRUE(name1 == name2);
-  EXPECT_FALSE(name1 != name2);
-  EXPECT_FALSE(name2 == name3);
-  EXPECT_TRUE(name2 != name3);
+  remove("SimpleTest.sqlite3");
 }
 
 namespace {
@@ -260,16 +252,6 @@ TEST_F(SyncableDirectoryTest, TestBasicLookupValidID) {
   ASSERT_TRUE(e.good());
 }
 
-TEST_F(SyncableDirectoryTest, TestBasicCaseSensitivity) {
-  PathString name = PSTR("RYAN");
-  PathString conflicting_name = PSTR("ryan");
-  CreateEntry(name);
-
-  WriteTransaction wtrans(dir_.get(), UNITTEST, __FILE__, __LINE__);
-  MutableEntry me(&wtrans, CREATE, wtrans.root_id(), conflicting_name);
-  ASSERT_FALSE(me.good());
-}
-
 TEST_F(SyncableDirectoryTest, TestDelete) {
   PathString name = PSTR("peanut butter jelly time");
   WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
@@ -283,15 +265,13 @@ TEST_F(SyncableDirectoryTest, TestDelete) {
   ASSERT_TRUE(e3.good());
   ASSERT_TRUE(e3.Put(IS_DEL, true));
 
+  ASSERT_TRUE(e1.Put(IS_DEL, false));
+  ASSERT_TRUE(e2.Put(IS_DEL, false));
   ASSERT_TRUE(e3.Put(IS_DEL, false));
-  ASSERT_FALSE(e1.Put(IS_DEL, false));
-  ASSERT_FALSE(e2.Put(IS_DEL, false));
-  ASSERT_TRUE(e3.Put(IS_DEL, true));
 
-  ASSERT_TRUE(e1.Put(IS_DEL, false));
-  ASSERT_FALSE(e2.Put(IS_DEL, false));
-  ASSERT_FALSE(e3.Put(IS_DEL, false));
   ASSERT_TRUE(e1.Put(IS_DEL, true));
+  ASSERT_TRUE(e2.Put(IS_DEL, true));
+  ASSERT_TRUE(e3.Put(IS_DEL, true));
 }
 
 TEST_F(SyncableDirectoryTest, TestGetUnsynced) {
@@ -462,7 +442,7 @@ TEST_F(SyncableDirectoryTest, DeleteBug_531383) {
     ASSERT_TRUE(twin.good());
     ASSERT_TRUE(twin.Put(IS_DEL, true));
     ASSERT_TRUE(grandchild.Put(IS_DEL, false));
-    ASSERT_FALSE(twin.Put(IS_DEL, false));
+
     grandchild_handle = grandchild.Get(META_HANDLE);
     twin_handle = twin.Get(META_HANDLE);
   }
@@ -531,86 +511,98 @@ TEST_F(SyncableDirectoryTest, TestIsLegalNewParent) {
   ASSERT_FALSE(IsLegalNewParent(parent, grandchild));
 }
 
-TEST_F(SyncableDirectoryTest, TestFindEntryInFolder) {
+TEST_F(SyncableDirectoryTest, TestEntryIsInFolder) {
   // Create a subdir and an entry.
   int64 entry_handle;
+  syncable::Id folder_id;
+  syncable::Id entry_id;
+  PathString entry_name = PSTR("entry");
+
   {
     WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
     MutableEntry folder(&trans, CREATE, trans.root_id(), PSTR("folder"));
     ASSERT_TRUE(folder.good());
     EXPECT_TRUE(folder.Put(IS_DIR, true));
     EXPECT_TRUE(folder.Put(IS_UNSYNCED, true));
-    MutableEntry entry(&trans, CREATE, folder.Get(ID), PSTR("entry"));
+    folder_id = folder.Get(ID);
+
+    MutableEntry entry(&trans, CREATE, folder.Get(ID), entry_name);
     ASSERT_TRUE(entry.good());
     entry_handle = entry.Get(META_HANDLE);
     entry.Put(IS_UNSYNCED, true);
+    entry_id = entry.Get(ID);
   }
 
   // Make sure we can find the entry in the folder.
   {
     ReadTransaction trans(dir_.get(), __FILE__, __LINE__);
-    Entry entry(&trans, GET_BY_PATH, PathString(kPathSeparator) +
-                                    PSTR("folder") +
-                                    kPathSeparator + PSTR("entry"));
-    ASSERT_TRUE(entry.good());
-    ASSERT_TRUE(entry.Get(META_HANDLE) == entry_handle);
-  }
-}
+    EXPECT_EQ(0, CountEntriesWithName(&trans, trans.root_id(), entry_name));
+    EXPECT_EQ(1, CountEntriesWithName(&trans, folder_id, entry_name));
 
-TEST_F(SyncableDirectoryTest, TestGetByParentIdAndName) {
-  PathString name = PSTR("Bob");
-  Id id = TestIdFactory::MakeServer("ID for Bob");
-  {
-    WriteTransaction wtrans(dir_.get(), UNITTEST, __FILE__, __LINE__);
-    MutableEntry entry(&wtrans, CREATE, wtrans.root_id() /*entry id*/, name);
+    Entry entry(&trans, GET_BY_ID, entry_id);
     ASSERT_TRUE(entry.good());
-    entry.Put(IS_DIR, true);
-    entry.Put(ID, id);
-    entry.Put(BASE_VERSION, 1);
-    entry.Put(IS_UNSYNCED, true);
-  }
-  {
-    WriteTransaction wtrans(dir_.get(), UNITTEST, __FILE__, __LINE__);
-    MutableEntry entry(&wtrans, GET_BY_PARENTID_AND_NAME, wtrans.root_id(),
-                       name);
-    ASSERT_TRUE(entry.good());
-    ASSERT_TRUE(id == entry.Get(ID));
-  }
-  {
-    ReadTransaction trans(dir_.get(), __FILE__, __LINE__);
-    Entry entry(&trans, GET_BY_PARENTID_AND_NAME, trans.root_id(), name);
-    ASSERT_TRUE(entry.good());
-    ASSERT_TRUE(id == entry.Get(ID));
+    EXPECT_EQ(entry_handle, entry.Get(META_HANDLE));
+    EXPECT_TRUE(entry.Get(NON_UNIQUE_NAME) == entry_name);
+    EXPECT_TRUE(entry.Get(PARENT_ID) == folder_id);
   }
 }
 
-TEST_F(SyncableDirectoryTest, TestParentIDIndexUpdate) {
+TEST_F(SyncableDirectoryTest, TestParentIdIndexUpdate) {
+  PathString child_name = PSTR("child");
+
   WriteTransaction wt(dir_.get(), UNITTEST, __FILE__, __LINE__);
-  MutableEntry folder(&wt, CREATE, wt.root_id(), PSTR("oldname"));
-  folder.Put(NAME, PSTR("newname"));
-  folder.Put(IS_UNSYNCED, true);
-  Entry entry(&wt, GET_BY_PATH, PSTR("newname"));
-  ASSERT_TRUE(entry.good());
+  MutableEntry parent_folder(&wt, CREATE, wt.root_id(), PSTR("folder1"));
+  parent_folder.Put(IS_UNSYNCED, true);
+  EXPECT_TRUE(parent_folder.Put(IS_DIR, true));
+
+  MutableEntry parent_folder2(&wt, CREATE, wt.root_id(), PSTR("folder2"));
+  parent_folder2.Put(IS_UNSYNCED, true);
+  EXPECT_TRUE(parent_folder2.Put(IS_DIR, true));
+
+  MutableEntry child(&wt, CREATE, parent_folder.Get(ID), child_name);
+  EXPECT_TRUE(child.Put(IS_DIR, true));
+  child.Put(IS_UNSYNCED, true);
+
+  ASSERT_TRUE(child.good());
+
+  EXPECT_EQ(0, CountEntriesWithName(&wt, wt.root_id(), child_name));
+  EXPECT_EQ(parent_folder.Get(ID), child.Get(PARENT_ID));
+  EXPECT_EQ(1, CountEntriesWithName(&wt, parent_folder.Get(ID), child_name));
+  EXPECT_EQ(0, CountEntriesWithName(&wt, parent_folder2.Get(ID), child_name));
+  child.Put(PARENT_ID, parent_folder2.Get(ID));
+  EXPECT_EQ(parent_folder2.Get(ID), child.Get(PARENT_ID));
+  EXPECT_EQ(0, CountEntriesWithName(&wt, parent_folder.Get(ID), child_name));
+  EXPECT_EQ(1, CountEntriesWithName(&wt, parent_folder2.Get(ID), child_name));
+
 }
 
 TEST_F(SyncableDirectoryTest, TestNoReindexDeletedItems) {
+  PathString folder_name = PSTR("folder");
+  PathString new_name = PSTR("new_name");
+
   WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
-  MutableEntry folder(&trans, CREATE, trans.root_id(), PSTR("folder"));
+  MutableEntry folder(&trans, CREATE, trans.root_id(), folder_name);
   ASSERT_TRUE(folder.good());
   ASSERT_TRUE(folder.Put(IS_DIR, true));
   ASSERT_TRUE(folder.Put(IS_DEL, true));
-  Entry gone(&trans, GET_BY_PARENTID_AND_NAME, trans.root_id(), PSTR("folder"));
-  ASSERT_FALSE(gone.good());
-  ASSERT_TRUE(folder.PutParentIdAndName(trans.root_id(),
-                                        Name(PSTR("new_name"))));
+
+  EXPECT_EQ(0, CountEntriesWithName(&trans, trans.root_id(), folder_name));
+
+  MutableEntry deleted(&trans, GET_BY_ID, folder.Get(ID));
+  ASSERT_TRUE(deleted.good());
+  ASSERT_TRUE(deleted.Put(PARENT_ID, trans.root_id()));
+  ASSERT_TRUE(deleted.Put(NON_UNIQUE_NAME, new_name));
+
+  EXPECT_EQ(0, CountEntriesWithName(&trans, trans.root_id(), folder_name));
+  EXPECT_EQ(0, CountEntriesWithName(&trans, trans.root_id(), new_name));
 }
 
 TEST_F(SyncableDirectoryTest, TestCaseChangeRename) {
   WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
   MutableEntry folder(&trans, CREATE, trans.root_id(), PSTR("CaseChange"));
   ASSERT_TRUE(folder.good());
-  EXPECT_TRUE(folder.PutParentIdAndName(trans.root_id(),
-                                        Name(PSTR("CASECHANGE"))));
+  EXPECT_TRUE(folder.Put(PARENT_ID, trans.root_id()));
+  EXPECT_TRUE(folder.Put(NON_UNIQUE_NAME, PSTR("CASECHANGE")));
   EXPECT_TRUE(folder.Put(IS_DEL, true));
 }
 
@@ -633,24 +625,29 @@ TEST_F(SyncableDirectoryTest, TestShareInfo) {
 }
 
 TEST_F(SyncableDirectoryTest, TestSimpleFieldsPreservedDuringSaveChanges) {
-  Id id = TestIdFactory::FromNumber(1);
+  Id update_id = TestIdFactory::FromNumber(1);
+  Id create_id;
   EntryKernel create_pre_save, update_pre_save;
   EntryKernel create_post_save, update_post_save;
+  PathString create_name =  PSTR("Create");
+
   {
     WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
-    MutableEntry create(&trans, CREATE, trans.root_id(), PSTR("Create"));
-    MutableEntry update(&trans, CREATE_NEW_UPDATE_ITEM, id);
+    MutableEntry create(&trans, CREATE, trans.root_id(), create_name);
+    MutableEntry update(&trans, CREATE_NEW_UPDATE_ITEM, update_id);
     create.Put(IS_UNSYNCED, true);
     update.Put(IS_UNAPPLIED_UPDATE, true);
     create_pre_save = create.GetKernelCopy();
     update_pre_save = update.GetKernelCopy();
+    create_id = create.Get(ID);
   }
   dir_->SaveChanges();
+
   {
     ReadTransaction trans(dir_.get(), __FILE__, __LINE__);
-    Entry create(&trans, GET_BY_PARENTID_AND_NAME, trans.root_id(),
-                 PSTR("Create"));
-    Entry update(&trans, GET_BY_ID, id);
+    Entry create(&trans, GET_BY_ID, create_id);
+    EXPECT_EQ(1, CountEntriesWithName(&trans, trans.root_id(), create_name));
+    Entry update(&trans, GET_BY_ID, update_id);
     create_post_save = create.GetKernelCopy();
     update_post_save = update.GetKernelCopy();
   }
@@ -713,8 +710,8 @@ TEST_F(SyncableDirectoryTest, TestSaveChangesFailure) {
 
     MutableEntry aguilera(&trans, GET_BY_HANDLE, handle1);
     ASSERT_TRUE(aguilera.good());
-    aguilera.Put(NAME, PSTR("christina"));
-    ASSERT_TRUE(aguilera.GetKernelCopy().dirty[NAME]);
+    aguilera.Put(NON_UNIQUE_NAME, PSTR("christina"));
+    ASSERT_TRUE(aguilera.GetKernelCopy().dirty[NON_UNIQUE_NAME]);
 
     MutableEntry kids_on_block(&trans, CREATE, trans.root_id(), PSTR("kids"));
     ASSERT_TRUE(kids_on_block.good());
@@ -738,7 +735,7 @@ TEST_F(SyncableDirectoryTest, TestSaveChangesFailure) {
      Entry kids_on_block(&trans, GET_BY_HANDLE, handle2);
      ASSERT_TRUE(kids_on_block.good());
 
-     EXPECT_TRUE(aguilera.dirty[NAME]);
+     EXPECT_TRUE(aguilera.dirty[NON_UNIQUE_NAME]);
      EXPECT_TRUE(kids_on_block.Get(IS_NEW));
   }
 }
@@ -749,8 +746,9 @@ void SyncableDirectoryTest::ValidateEntry(BaseTransaction* trans, int64 id,
     bool is_del) {
   Entry e(trans, GET_BY_ID, TestIdFactory::FromNumber(id));
   ASSERT_TRUE(e.good());
-  if (check_name)
-    ASSERT_TRUE(name == e.Get(NAME));
+  if (check_name) {
+    ASSERT_TRUE(name == e.Get(NON_UNIQUE_NAME));
+  }
   ASSERT_TRUE(base_version == e.Get(BASE_VERSION));
   ASSERT_TRUE(server_version == e.Get(SERVER_VERSION));
   ASSERT_TRUE(is_del == e.Get(IS_DEL));
@@ -891,6 +889,7 @@ class DirectoryKernelStalenessBugDelegate : public ThreadBugDelegate {
     const char test_bytes[] = "test data";
     const PathString dirname = PSTR("DirectoryKernelStalenessBug");
     AutoLock scoped_lock(step_->mutex);
+    const Id jeff_id = TestIdFactory::FromNumber(100);
 
     while (step_->number < 4) {
       while (step_->number % 2 != role_) {
@@ -909,7 +908,7 @@ class DirectoryKernelStalenessBugDelegate : public ThreadBugDelegate {
           WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__);
           MutableEntry me(&trans, CREATE, trans.root_id(), PSTR("Jeff"));
           me.Put(BASE_VERSION, 1);
-          me.Put(ID, TestIdFactory::FromNumber(100));
+          me.Put(ID, jeff_id);
           PutDataAsExtendedAttribute(&trans, &me, test_bytes,
                                      sizeof(test_bytes));
         }
@@ -938,7 +937,7 @@ class DirectoryKernelStalenessBugDelegate : public ThreadBugDelegate {
           ScopedDirLookup dir(directory_manager_, dirname);
           CHECK(dir.good());
           ReadTransaction trans(dir, __FILE__, __LINE__);
-          Entry e(&trans, GET_BY_PATH, PSTR("Jeff"));
+          Entry e(&trans, GET_BY_ID, jeff_id);
           ExpectDataFromExtendedAttributeEquals(&trans, &e, test_bytes,
                                                 sizeof(test_bytes));
         }
@@ -994,9 +993,8 @@ class StressTransactionsDelegate : public PlatformThread::Delegate {
        const int rand_action = rand() % 10;
        if (rand_action < 4 && !path_name.empty()) {
          ReadTransaction trans(dir, __FILE__, __LINE__);
-         Entry e(&trans, GET_BY_PARENTID_AND_NAME, trans.root_id(), path_name);
+         CHECK(1 == CountEntriesWithName(&trans, trans.root_id(), path_name));
          PlatformThread::Sleep(rand() % 10);
-         CHECK(e.good());
        } else {
          string unique_name = StringPrintf("%d.%d", thread_number_,
            entry_count++);
@@ -1071,31 +1069,6 @@ TEST(Syncable, ComparePathNames) {
   }
 }
 
-#if defined(OS_WIN)
-TEST(Syncable, PathNameMatch) {
-  // basic stuff, not too many otherwise we're testing the os.
-  EXPECT_TRUE(PathNameMatch(PSTR("bob"), PSTR("bob")));
-  EXPECT_FALSE(PathNameMatch(PSTR("bob"), PSTR("fred")));
-  // Test our ; extension.
-  EXPECT_TRUE(PathNameMatch(PSTR("bo;b"), PSTR("bo;b")));
-  EXPECT_TRUE(PathNameMatch(PSTR("bo;b"), PSTR("bo*")));
-  EXPECT_FALSE(PathNameMatch(PSTR("bo;b"), PSTR("co;b")));
-  EXPECT_FALSE(PathNameMatch(PSTR("bo;b"), PSTR("co*")));
-  // Test our fixes for prepended spaces.
-  EXPECT_TRUE(PathNameMatch(PSTR("  bob"), PSTR("  bo*")));
-  EXPECT_TRUE(PathNameMatch(PSTR("  bob"), PSTR("  bob")));
-  EXPECT_FALSE(PathNameMatch(PSTR("bob"), PSTR("  bob")));
-  EXPECT_FALSE(PathNameMatch(PSTR("  bob"), PSTR("bob")));
-  // Combo test.
-  EXPECT_TRUE(PathNameMatch(PSTR("  b;ob"), PSTR("  b;o*")));
-  EXPECT_TRUE(PathNameMatch(PSTR("  b;ob"), PSTR("  b;ob")));
-  EXPECT_FALSE(PathNameMatch(PSTR("b;ob"), PSTR("  b;ob")));
-  EXPECT_FALSE(PathNameMatch(PSTR("  b;ob"), PSTR("b;ob")));
-  // other whitespace should give no matches.
-  EXPECT_FALSE(PathNameMatch(PSTR("bob"), PSTR("\tbob")));
-}
-#endif  // defined(OS_WIN)
-
 void FakeSync(MutableEntry* e, const char* fake_id) {
   e->Put(IS_UNSYNCED, false);
   e->Put(BASE_VERSION, 2);
@@ -1104,11 +1077,11 @@ void FakeSync(MutableEntry* e, const char* fake_id) {
 
 TEST_F(SyncableDirectoryTest, Bug1509232) {
   const PathString a = PSTR("alpha");
-
-  CreateEntry(a, dir_.get()->NextId());
+  const Id entry_id = dir_.get()->NextId();
+  CreateEntry(a, entry_id);
   {
     WriteTransaction trans(dir_.get(), UNITTEST, __FILE__, __LINE__);
-    MutableEntry e(&trans, GET_BY_PATH, a);
+    MutableEntry e(&trans, GET_BY_ID, entry_id);
     ASSERT_TRUE(e.good());
     ExtendedAttributeKey key(e.Get(META_HANDLE), PSTR("resourcefork"));
     MutableExtendedAttribute ext(&trans, CREATE, key);