[Sync] Speed up sync node browser/search in about:sync

chrome/browser/sync/syncable/syncable.cc

Index: chrome/browser/sync/syncable/syncable.cc
diff --git a/chrome/browser/sync/syncable/syncable.cc b/chrome/browser/sync/syncable/syncable.cc
index 4a0340b96e505ab76ad9d9399ad260255b760759..c99ae8a1a636392266921a75c8485cedad7a1e0d 100644
--- a/chrome/browser/sync/syncable/syncable.cc
+++ b/chrome/browser/sync/syncable/syncable.cc
@@ -506,20 +506,29 @@ EntryKernel* Directory::GetEntryByHandle(int64 metahandle,
   return NULL;
 }
 
-void Directory::GetChildHandles(BaseTransaction* trans, const Id& parent_id,
-                                Directory::ChildHandles* result) {
+void Directory::GetChildHandlesById(
+    BaseTransaction* trans, const Id& parent_id,
+    Directory::ChildHandles* result) {
   CHECK(this == trans->directory());
   result->clear();

[lipalani1, 2011/06/02 18:51:52]

Can we move result->clear to the private function?

[akalin, 2011/06/02 22:13:33]

I don't think we should; the other function (GetChildHandlesByHandle needs to
clear it itself because it can return before calling GetChildHandles.

Renamed GetChildHandles to AppendChildHandles to make it more obvious.

   {
     ScopedKernelLock lock(this);

[lipalani1, 2011/06/02 18:51:52]

If the result->clear is moved is the block necessary.

[akalin, 2011/06/02 22:13:33]

The block isn't necessary anyway, so removed it.

+    GetChildHandles(lock, parent_id, result);
+  }
+}
 
-    typedef ParentIdChildIndex::iterator iterator;
-    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));
-    }
+void Directory::GetChildHandlesByHandle(
+    BaseTransaction* trans, int64 handle,
+    Directory::ChildHandles* result) {
+  CHECK(this == trans->directory());
+  result->clear();
+  {
+    ScopedKernelLock lock(this);
+    EntryKernel* kernel = GetEntryByHandle(handle, &lock);
+    if (!kernel)
+      return;
+
+    GetChildHandles(lock, kernel->ref(ID), result);
   }
 }
 
@@ -1952,4 +1961,16 @@ Directory::GetParentChildIndexUpperBound(const ScopedKernelLock& lock,
       parent_id.GetLexicographicSuccessor());
 }
 
+void Directory::GetChildHandles(const ScopedKernelLock& lock,
+                                const Id& parent_id,

[lipalani1, 2011/06/02 18:51:52]

Can this function be const? Also the callers? I think the lock could be a
problem...

[akalin, 2011/06/02 22:13:33]

In this case, it isn't the lock, but the fact that the function it calls
modifies kernel_->needle, which is used as a scratch variable.  Ugh.

+                                Directory::ChildHandles* result) {

[lipalani1, 2011/06/02 18:51:52]

Nit - CHECK(result != NULL)

[akalin, 2011/06/02 22:13:33]

Done.

+  typedef ParentIdChildIndex::iterator iterator;
+  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));
+  }
+}
+
 }  // namespace syncable