[Sync] Add HasChildren() function and use it instead of GetFirstChildId()

chrome/browser/sync/syncable/syncable.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/syncable/syncable.h"
 
 #include <algorithm>
 #include <cstring>
 #include <functional>
 #include <iomanip>
@@ skipping 1747 matching lines @@
   int64 result;
   {
     ScopedKernelLock lock(this);
     result = (kernel_->persisted_info.next_id)--;
     kernel_->info_status = KERNEL_SHARE_INFO_DIRTY;
   }
   DCHECK_LT(result, 0);
   return Id::CreateFromClientString(base::Int64ToString(result));
 }
 
+bool Directory::HasChildren(BaseTransaction* trans, const Id& id) {
+  ScopedKernelLock lock(this);
+  return (GetPossibleFirstChild(lock, id) != NULL);
+}
+
 Id Directory::GetFirstChildId(BaseTransaction* trans,
                               const Id& parent_id) {
   ScopedKernelLock lock(this);
-  // We can use the server positional ordering as a hint because it's generally
-  // in sync with the local (linked-list) positional ordering, and we have an
-  // index on it.
-  ParentIdChildIndex::iterator candidate =
-      GetParentChildIndexLowerBound(lock, parent_id);
-  ParentIdChildIndex::iterator end_range =
-      GetParentChildIndexUpperBound(lock, parent_id);
-  for (; candidate != end_range; ++candidate) {
-    EntryKernel* entry = *candidate;
-    // Filter out self-looped items, which are temporarily not in the child
-    // ordering.
-    if (entry->ref(PREV_ID).IsRoot() ||
-        entry->ref(PREV_ID) != entry->ref(NEXT_ID)) {
-      // Walk to the front of the list; the server position ordering
-      // is commonly identical to the linked-list ordering, but pending
-      // unsynced or unapplied items may diverge.
-      while (!entry->ref(PREV_ID).IsRoot()) {
-        entry = GetEntryById(entry->ref(PREV_ID), &lock);
-      }
-      return entry->ref(ID);
-    }
+  EntryKernel* entry = GetPossibleFirstChild(lock, parent_id);
+  if (!entry)
+    return Id();
+
+  // Walk to the front of the list; the server position ordering
+  // is commonly identical to the linked-list ordering, but pending
+  // unsynced or unapplied items may diverge.
+  while (!entry->ref(PREV_ID).IsRoot()) {
+    // TODO(akalin): Gracefully handle GetEntryById returning NULL
+    // (http://crbug.com/100907).
+    entry = GetEntryById(entry->ref(PREV_ID), &lock);
   }
-  // There were no children in the linked list.
-  return Id();
+  return entry->ref(ID);
 }
 
 Id Directory::GetLastChildId(BaseTransaction* trans,
                              const Id& parent_id) {
   ScopedKernelLock lock(this);
   // We can use the server positional ordering as a hint because it's generally
   // in sync with the local (linked-list) positional ordering, and we have an
   // index on it.
   ParentIdChildIndex::iterator begin_range =
       GetParentChildIndexLowerBound(lock, parent_id);
   ParentIdChildIndex::iterator candidate =
       GetParentChildIndexUpperBound(lock, parent_id);
 
   while (begin_range != candidate) {
     --candidate;
     EntryKernel* entry = *candidate;
 
     // Filter out self-looped items, which are temporarily not in the child
     // ordering.
     if (entry->ref(NEXT_ID).IsRoot() ||
         entry->ref(NEXT_ID) != entry->ref(PREV_ID)) {
       // Walk to the back of the list; the server position ordering
       // is commonly identical to the linked-list ordering, but pending
       // unsynced or unapplied items may diverge.
       while (!entry->ref(NEXT_ID).IsRoot())
+        // TODO(akalin): Gracefully handle GetEntryById returning NULL
+        // (http://crbug.com/100907).
         entry = GetEntryById(entry->ref(NEXT_ID), &lock);
       return entry->ref(ID);
     }
   }
   // There were no children in the linked list.
   return Id();
 }
 
 Id Directory::ComputePrevIdFromServerPosition(
     const EntryKernel* entry,
@@ skipping 124 matching lines @@
 Directory::ParentIdChildIndex::iterator
 Directory::GetParentChildIndexLowerBound(const ScopedKernelLock& lock,
                                          const Id& parent_id) {
   // Peg the parent ID, and use the least values for the remaining
   // index variables.
   return LocateInParentChildIndex(lock, parent_id,
       std::numeric_limits<int64>::min(),
       Id::GetLeastIdForLexicographicComparison());
 }
 
-
 Directory::ParentIdChildIndex::iterator
 Directory::GetParentChildIndexUpperBound(const ScopedKernelLock& lock,
                                          const Id& parent_id) {
   // The upper bound of |parent_id|'s range is the lower
   // bound of |++parent_id|'s range.
   return GetParentChildIndexLowerBound(lock,
       parent_id.GetLexicographicSuccessor());
 }
 
 void Directory::AppendChildHandles(const ScopedKernelLock& lock,
                                    const Id& parent_id,
                                    Directory::ChildHandles* result) {
   typedef ParentIdChildIndex::iterator iterator;
   CHECK(result);
   for (iterator i = GetParentChildIndexLowerBound(lock, parent_id),
            end = GetParentChildIndexUpperBound(lock, parent_id);
        i != end; ++i) {
     DCHECK_EQ(parent_id, (*i)->ref(PARENT_ID));
     result->push_back((*i)->ref(META_HANDLE));
   }
 }
 
+EntryKernel* Directory::GetPossibleFirstChild(

[rlarocque, 2011/10/26 22:43:18]

nit:
Could we name this "GetFirstChildInServerOrdering" instead?  I think that would
more precisely describe what it does.

[akalin, 2011/10/27 00:19:21]

Meh, I kinda disagree.  The callers of this class need know only that they
receive some node which is likely to be the first one.  How the node is found is
an implementation detail. (Even though this is a private function, it helps to
still think of it as an interface to the public methods of the class.)

+    const ScopedKernelLock& lock, const Id& parent_id) {
+  // We can use the server positional ordering as a hint because it's generally
+  // in sync with the local (linked-list) positional ordering, and we have an
+  // index on it.
+  ParentIdChildIndex::iterator candidate =
+      GetParentChildIndexLowerBound(lock, parent_id);
+  ParentIdChildIndex::iterator end_range =
+      GetParentChildIndexUpperBound(lock, parent_id);
+  for (; candidate != end_range; ++candidate) {
+    EntryKernel* entry = *candidate;
+    // Filter out self-looped items, which are temporarily not in the child
+    // ordering.
+    if (entry->ref(PREV_ID).IsRoot() ||
+        entry->ref(PREV_ID) != entry->ref(NEXT_ID)) {
+      return entry;
+    }
+  }
+  // There were no children in the linked list.
+  return NULL;
+}
+
 }  // namespace syncable