diff options
| -rw-r--r-- | chrome/browser/sync/glue/generic_change_processor.cc | 10 | ||||
| -rw-r--r-- | chrome/browser/sync/glue/generic_change_processor_unittest.cc | 109 | ||||
| -rw-r--r-- | chrome/chrome_tests_unit.gypi | 1 | ||||
| -rw-r--r-- | sync/internal_api/base_node.cc | 4 | ||||
| -rw-r--r-- | sync/internal_api/public/base_node.h | 5 | ||||
| -rw-r--r-- | sync/syncable/entry.cc | 4 | ||||
| -rw-r--r-- | sync/syncable/entry.h | 6 |
7 files changed, 135 insertions, 4 deletions
diff --git a/chrome/browser/sync/glue/generic_change_processor.cc b/chrome/browser/sync/glue/generic_change_processor.cc index 38d952c..41a1411 100644 --- a/chrome/browser/sync/glue/generic_change_processor.cc +++ b/chrome/browser/sync/glue/generic_change_processor.cc @@ -116,10 +116,13 @@ syncer::SyncError GenericChangeProcessor::GetSyncDataForType( // TODO(akalin): We'll have to do a tree traversal for bookmarks. DCHECK_NE(type, syncer::BOOKMARKS); - int64 sync_child_id = root.GetFirstChildId(); - while (sync_child_id != syncer::kInvalidId) { + std::vector<int64> child_ids; + root.GetChildIds(&child_ids); + + for (std::vector<int64>::iterator it = child_ids.begin(); + it != child_ids.end(); ++it) { syncer::ReadNode sync_child_node(&trans); - if (sync_child_node.InitByIdLookup(sync_child_id) != + if (sync_child_node.InitByIdLookup(*it) != syncer::BaseNode::INIT_OK) { syncer::SyncError error(FROM_HERE, "Failed to fetch child node for type " + type_name + ".", @@ -128,7 +131,6 @@ syncer::SyncError GenericChangeProcessor::GetSyncDataForType( } current_sync_data->push_back(syncer::SyncData::CreateRemoteData( sync_child_node.GetId(), sync_child_node.GetEntitySpecifics())); - sync_child_id = sync_child_node.GetSuccessorId(); } return syncer::SyncError(); } diff --git a/chrome/browser/sync/glue/generic_change_processor_unittest.cc b/chrome/browser/sync/glue/generic_change_processor_unittest.cc new file mode 100644 index 0000000..6ab8a61 --- /dev/null +++ b/chrome/browser/sync/glue/generic_change_processor_unittest.cc @@ -0,0 +1,109 @@ +// Copyright 2013 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/glue/generic_change_processor.h" + +#include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" +#include "base/message_loop.h" +#include "base/stringprintf.h" +#include "chrome/browser/sync/glue/data_type_error_handler_mock.h" +#include "sync/api/fake_syncable_service.h" +#include "sync/api/sync_merge_result.h" +#include "sync/internal_api/public/base/model_type.h" +#include "sync/internal_api/public/read_node.h" +#include "sync/internal_api/public/test/test_user_share.h" +#include "sync/internal_api/public/user_share.h" +#include "sync/internal_api/public/write_node.h" +#include "sync/internal_api/public/write_transaction.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace browser_sync { + +namespace { + +class GenericChangeProcessorTest : public testing::Test { + public: + // It doesn't matter which type we use. Just pick one. + static const syncer::ModelType kType = syncer::PREFERENCES; + + GenericChangeProcessorTest() : + loop(base::MessageLoop::TYPE_UI), + sync_merge_result_(kType), + merge_result_ptr_factory_(&sync_merge_result_), + syncable_service_ptr_factory_(&fake_syncable_service_) { + } + + virtual void SetUp() OVERRIDE { + test_user_share_.SetUp(); + syncer::TestUserShare::CreateRoot(kType, test_user_share_.user_share()); + change_processor_.reset( + new GenericChangeProcessor( + &data_type_error_handler_, + syncable_service_ptr_factory_.GetWeakPtr(), + merge_result_ptr_factory_.GetWeakPtr(), + test_user_share_.user_share())); + } + + virtual void TearDown() OVERRIDE { + test_user_share_.TearDown(); + } + + void BuildChildNodes(int n) { + syncer::WriteTransaction trans(FROM_HERE, user_share()); + syncer::ReadNode root(&trans); + ASSERT_EQ(syncer::BaseNode::INIT_OK, + root.InitByTagLookup(syncer::ModelTypeToRootTag(kType))); + for (int i = 0; i < n; ++i) { + syncer::WriteNode node(&trans); + node.InitUniqueByCreation(kType, root, base::StringPrintf("node%05d", i)); + } + } + + GenericChangeProcessor* change_processor() { + return change_processor_.get(); + } + + syncer::UserShare* user_share() { + return test_user_share_.user_share(); + } + + private: + MessageLoop loop; + + syncer::SyncMergeResult sync_merge_result_; + base::WeakPtrFactory<syncer::SyncMergeResult> merge_result_ptr_factory_; + + syncer::FakeSyncableService fake_syncable_service_; + base::WeakPtrFactory<syncer::FakeSyncableService> + syncable_service_ptr_factory_; + + DataTypeErrorHandlerMock data_type_error_handler_; + syncer::TestUserShare test_user_share_; + + scoped_ptr<GenericChangeProcessor> change_processor_; +}; + +// This test exercises GenericChangeProcessor's GetSyncDataForType function. +// It's not a great test, but, by modifying some of the parameters, you could +// turn it into a micro-benchmark for model association. +TEST_F(GenericChangeProcessorTest, StressGetSyncDataForType) { + const int kNumChildNodes = 1000; + const int kRepeatCount = 1; + + ASSERT_NO_FATAL_FAILURE(BuildChildNodes(kNumChildNodes)); + + for (int i = 0; i < kRepeatCount; ++i) { + syncer::SyncDataList sync_data; + change_processor()->GetSyncDataForType(kType, &sync_data); + + // Start with a simple test. We can add more in-depth testing later. + EXPECT_EQ(static_cast<size_t>(kNumChildNodes), sync_data.size()); + } +} + +} // namespace + +} // namespace browser_sync + diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 6db3447..d24f7e5 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1184,6 +1184,7 @@ 'browser/sync/glue/frontend_data_type_controller_mock.cc', 'browser/sync/glue/frontend_data_type_controller_mock.h', 'browser/sync/glue/frontend_data_type_controller_unittest.cc', + 'browser/sync/glue/generic_change_processor_unittest.cc', 'browser/sync/glue/model_association_manager_unittest.cc', 'browser/sync/glue/model_associator_mock.cc', 'browser/sync/glue/model_associator_mock.h', diff --git a/sync/internal_api/base_node.cc b/sync/internal_api/base_node.cc index 88dec8c..f9bf79b 100644 --- a/sync/internal_api/base_node.cc +++ b/sync/internal_api/base_node.cc @@ -215,6 +215,10 @@ int64 BaseNode::GetFirstChildId() const { return IdToMetahandle(GetTransaction()->GetWrappedTrans(), id_string); } +void BaseNode::GetChildIds(std::vector<int64>* result) const { + GetEntry()->GetChildHandles(result); +} + int BaseNode::GetTotalNodeCount() const { syncable::BaseTransaction* trans = GetTransaction()->GetWrappedTrans(); diff --git a/sync/internal_api/public/base_node.h b/sync/internal_api/public/base_node.h index 3c56b5e..6c54ce2 100644 --- a/sync/internal_api/public/base_node.h +++ b/sync/internal_api/public/base_node.h @@ -195,6 +195,11 @@ class SYNC_EXPORT BaseNode { // children, return 0. int64 GetFirstChildId() const; + // Returns the IDs of the children of this node. + // If this type supports user-defined positions the returned IDs will be in + // the correct order. + void GetChildIds(std::vector<int64>* result) const; + // Returns the total number of nodes including and beneath this node. // Recursively iterates through all children. int GetTotalNodeCount() const; diff --git a/sync/syncable/entry.cc b/sync/syncable/entry.cc index 2d98c6f..ac62448 100644 --- a/sync/syncable/entry.cc +++ b/sync/syncable/entry.cc @@ -104,6 +104,10 @@ Id Entry::GetFirstChildId() const { return dir()->GetFirstChildId(basetrans_, kernel_); } +void Entry::GetChildHandles(std::vector<int64>* result) const { + dir()->GetChildHandlesById(basetrans_, Get(ID), result); +} + bool Entry::ShouldMaintainPosition() const { return kernel_->ShouldMaintainPosition(); } diff --git a/sync/syncable/entry.h b/sync/syncable/entry.h index f9d3f90..26ccc0a 100644 --- a/sync/syncable/entry.h +++ b/sync/syncable/entry.h @@ -109,6 +109,12 @@ class SYNC_EXPORT Entry { Id GetSuccessorId() const; Id GetFirstChildId() const; + // Returns a vector of this node's children's handles. + // Clears |result| if there are no children. If this node is of a type that + // supports user-defined ordering then the resulting vector will be in the + // proper order. + void GetChildHandles(std::vector<int64>* result) const; + inline bool ExistsOnClientBecauseNameIsNonEmpty() const { DCHECK(kernel_); return !kernel_->ref(NON_UNIQUE_NAME).empty(); |