Skip to content

Commit

Permalink
sync: Cleanups in and around directory.h
Browse files Browse the repository at this point in the history
- Remove MockDirectorySyncerCommand (not used).
- Remove MockDirectory (used only by MockDirectorySyncerCommand).
- Remove Directory::InitKernelForTest (used only by MockDirectory).
- Merge ChildHandles and UnsyncedMetahandles typedefs into a single
  Metahandles typedef.
- Rearrange declarations in directory.h to match Google C++ style.
- Create scoped_kernel_lock.cc; move related definitions from
  directory.cc to this new file.
- Use DISALLOW_COPY_AND_ASSIGN instead of declaring an undefined
  Directory::operator= manually.

BUG=245931

Review URL: https://chromiumcodereview.appspot.com/16591002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204828 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rlarocque@chromium.org committed Jun 7, 2013
1 parent 43a0e55 commit 560009e
Show file tree
Hide file tree
Showing 19 changed files with 144 additions and 267 deletions.
2 changes: 1 addition & 1 deletion sync/engine/conflict_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ void ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
entry.Put(syncable::BASE_SERVER_SPECIFICS, sync_pb::EntitySpecifics());
} else { // SERVER_IS_DEL is true
if (entry.Get(syncable::IS_DIR)) {
Directory::ChildHandles children;
Directory::Metahandles children;
trans->directory()->GetChildHandlesById(trans,
entry.Get(syncable::ID),
&children);
Expand Down
8 changes: 4 additions & 4 deletions sync/engine/get_commit_ids_command.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ SyncerError GetCommitIdsCommand::ExecuteImpl(SyncSession* session) {
// Gather the full set of unsynced items and store it in the session. They
// are not in the correct order for commit.
std::set<int64> ready_unsynced_set;
syncable::Directory::UnsyncedMetaHandles all_unsynced_handles;
syncable::Directory::Metahandles all_unsynced_handles;
GetUnsyncedEntries(trans_,
&all_unsynced_handles);

Expand Down Expand Up @@ -165,10 +165,10 @@ void GetCommitIdsCommand::FilterUnreadyEntries(
ModelTypeSet throttled_types,
ModelTypeSet encrypted_types,
bool passphrase_missing,
const syncable::Directory::UnsyncedMetaHandles& unsynced_handles,
const syncable::Directory::Metahandles& unsynced_handles,
std::set<int64>* ready_unsynced_set) {
for (syncable::Directory::UnsyncedMetaHandles::const_iterator iter =
unsynced_handles.begin(); iter != unsynced_handles.end(); ++iter) {
for (syncable::Directory::Metahandles::const_iterator iter =
unsynced_handles.begin(); iter != unsynced_handles.end(); ++iter) {
syncable::Entry entry(trans, syncable::GET_BY_HANDLE, *iter);
if (IsEntryReadyForCommit(throttled_types,
encrypted_types,
Expand Down
2 changes: 1 addition & 1 deletion sync/engine/get_commit_ids_command.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class SYNC_EXPORT_PRIVATE GetCommitIdsCommand : public SyncerCommand {
ModelTypeSet throttled_types,
ModelTypeSet encrypted_types,
bool passphrase_missing,
const syncable::Directory::UnsyncedMetaHandles& unsynced_handles,
const syncable::Directory::Metahandles& unsynced_handles,
std::set<int64>* ready_unsynced_set);

private:
Expand Down
14 changes: 7 additions & 7 deletions sync/engine/syncer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ class SyncerTest : public testing::Test,
syncer_ = new Syncer();

syncable::ReadTransaction trans(FROM_HERE, directory());
syncable::Directory::ChildHandles children;
syncable::Directory::Metahandles children;
directory()->GetChildHandlesById(&trans, trans.root_id(), &children);
ASSERT_EQ(0u, children.size());
saw_syncer_event_ = false;
Expand Down Expand Up @@ -2350,7 +2350,7 @@ TEST_F(SyncerTest, DoublyChangedWithResolver) {
local_cache_guid(), local_id.GetServerId());
mock_server_->set_conflict_all_commits(true);
SyncShareNudge();
syncable::Directory::ChildHandles children;
syncable::Directory::Metahandles children;
{
syncable::ReadTransaction trans(FROM_HERE, directory());
directory()->GetChildHandlesById(&trans, parent_id_, &children);
Expand Down Expand Up @@ -2453,15 +2453,15 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) {
SyncShareNudge();
{
syncable::ReadTransaction trans(FROM_HERE, directory());
Directory::ChildHandles children;
Directory::Metahandles children;
directory()->GetChildHandlesById(&trans, root_id_, &children);
EXPECT_EQ(1u, children.size());
directory()->GetChildHandlesById(&trans, parent_id, &children);
EXPECT_EQ(1u, children.size());
std::vector<int64> unapplied;
directory()->GetUnappliedUpdateMetaHandles(&trans, all_types, &unapplied);
EXPECT_EQ(0u, unapplied.size());
syncable::Directory::UnsyncedMetaHandles unsynced;
syncable::Directory::Metahandles unsynced;
directory()->GetUnsyncedMetaHandles(&trans, &unsynced);
EXPECT_EQ(0u, unsynced.size());
saw_syncer_event_ = false;
Expand Down Expand Up @@ -3920,7 +3920,7 @@ TEST_F(SyncerTest, ClientTagUpdateClashesWithLocalEntry) {
EXPECT_EQ("tag2", tag2.Get(UNIQUE_CLIENT_TAG));
tag2_metahandle = tag2.Get(META_HANDLE);

syncable::Directory::ChildHandles children;
syncable::Directory::Metahandles children;
directory()->GetChildHandlesById(&trans, trans.root_id(), &children);
ASSERT_EQ(2U, children.size());
}
Expand Down Expand Up @@ -3960,7 +3960,7 @@ TEST_F(SyncerTest, ClientTagUpdateClashesWithLocalEntry) {
EXPECT_EQ("tag2", tag2.Get(UNIQUE_CLIENT_TAG));
EXPECT_EQ(tag2_metahandle, tag2.Get(META_HANDLE));

syncable::Directory::ChildHandles children;
syncable::Directory::Metahandles children;
directory()->GetChildHandlesById(&trans, trans.root_id(), &children);
ASSERT_EQ(2U, children.size());
}
Expand Down Expand Up @@ -4039,7 +4039,7 @@ TEST_F(SyncerTest, ClientTagClashWithinBatchOfUpdates) {
EXPECT_EQ(21, tag_c.Get(BASE_VERSION));
EXPECT_EQ("tag c", tag_c.Get(UNIQUE_CLIENT_TAG));

syncable::Directory::ChildHandles children;
syncable::Directory::Metahandles children;
directory()->GetChildHandlesById(&trans, trans.root_id(), &children);
ASSERT_EQ(3U, children.size());
}
Expand Down
6 changes: 3 additions & 3 deletions sync/engine/syncer_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ UpdateAttemptResponse AttemptToUpdateEntry(
}
}
} else if (entry->Get(IS_DIR)) {
Directory::ChildHandles handles;
Directory::Metahandles handles;
trans->directory()->GetChildHandlesById(trans, id, &handles);
if (!handles.empty()) {
// If we have still-existing children, then we need to deal with
Expand Down Expand Up @@ -492,14 +492,14 @@ void MarkDeletedChildrenSynced(
// may be sensible if this code shows up in profiling.
if (deleted_folders->empty())
return;
Directory::UnsyncedMetaHandles handles;
Directory::Metahandles handles;
{
syncable::ReadTransaction trans(FROM_HERE, dir);
dir->GetUnsyncedMetaHandles(&trans, &handles);
}
if (handles.empty())
return;
Directory::UnsyncedMetaHandles::iterator it;
Directory::Metahandles::iterator it;
for (it = handles.begin() ; it != handles.end() ; ++it) {
// Single transaction / entry we deal with.
WriteTransaction trans(FROM_HERE, SYNCER, dir);
Expand Down
4 changes: 2 additions & 2 deletions sync/internal_api/sync_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1223,10 +1223,10 @@ JsArgList SyncManagerImpl::GetChildNodeIds(const JsArgList& args) {
int64 id = GetId(args.Get(), 0);
if (id != kInvalidId) {
ReadTransaction trans(FROM_HERE, GetUserShare());
syncable::Directory::ChildHandles child_handles;
syncable::Directory::Metahandles child_handles;
trans.GetDirectory()->GetChildHandlesByHandle(trans.GetWrappedTrans(),
id, &child_handles);
for (syncable::Directory::ChildHandles::const_iterator it =
for (syncable::Directory::Metahandles::const_iterator it =
child_handles.begin(); it != child_handles.end(); ++it) {
child_ids->Append(new base::StringValue(base::Int64ToString(*it)));
}
Expand Down
1 change: 1 addition & 0 deletions sync/sync_core.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@
'syncable/on_disk_directory_backing_store.h',
'syncable/parent_child_index.cc',
'syncable/parent_child_index.h',
'syncable/scoped_kernel_lock.cc',
'syncable/scoped_kernel_lock.h',
'syncable/scoped_parent_child_index_updater.cc',
'syncable/scoped_parent_child_index_updater.h',
Expand Down
2 changes: 0 additions & 2 deletions sync/sync_tests.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
'js/js_test_util.h',
'sessions/test_util.cc',
'sessions/test_util.h',
'syncable/syncable_mock.cc',
'syncable/syncable_mock.h',
'test/callback_counter.h',
'test/engine/fake_model_worker.cc',
'test/engine/fake_model_worker.h',
Expand Down
21 changes: 5 additions & 16 deletions sync/syncable/directory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "sync/syncable/entry_kernel.h"
#include "sync/syncable/in_memory_directory_backing_store.h"
#include "sync/syncable/on_disk_directory_backing_store.h"
#include "sync/syncable/scoped_kernel_lock.h"
#include "sync/syncable/scoped_parent_child_index_updater.h"
#include "sync/syncable/syncable-inl.h"
#include "sync/syncable/syncable_base_transaction.h"
Expand All @@ -31,14 +32,6 @@ namespace syncable {
const base::FilePath::CharType Directory::kSyncDatabaseFilename[] =
FILE_PATH_LITERAL("SyncData.sqlite3");

void Directory::InitKernelForTest(
const std::string& name,
DirectoryChangeDelegate* delegate,
const WeakHandle<TransactionObserver>& transaction_observer) {
DCHECK(!kernel_);
kernel_ = new Kernel(name, KernelLoadInfo(), delegate, transaction_observer);
}

Directory::PersistedKernelInfo::PersistedKernelInfo()
: next_id(0) {
ModelTypeSet protocol_types = ProtocolTypes();
Expand Down Expand Up @@ -265,7 +258,7 @@ EntryKernel* Directory::GetEntryByHandle(int64 metahandle,

bool Directory::GetChildHandlesById(
BaseTransaction* trans, const Id& parent_id,
Directory::ChildHandles* result) {
Directory::Metahandles* result) {
if (!SyncAssert(this == trans->directory(), FROM_HERE,
"Directories don't match", trans))
return false;
Expand All @@ -278,7 +271,7 @@ bool Directory::GetChildHandlesById(

bool Directory::GetChildHandlesByHandle(
BaseTransaction* trans, int64 handle,
Directory::ChildHandles* result) {
Directory::Metahandles* result) {
if (!SyncAssert(this == trans->directory(), FROM_HERE,
"Directories don't match", trans))
return false;
Expand Down Expand Up @@ -893,7 +886,7 @@ void Directory::GetAllEntryKernels(BaseTransaction* trans,
}

void Directory::GetUnsyncedMetaHandles(BaseTransaction* trans,
UnsyncedMetaHandles* result) {
Metahandles* result) {
result->clear();
ScopedKernelLock lock(this);
copy(kernel_->unsynced_metahandles.begin(),
Expand Down Expand Up @@ -1285,7 +1278,7 @@ void Directory::PutPredecessor(EntryKernel* e, EntryKernel* predecessor) {
// TODO(rlarocque): Avoid this indirection. Just return the set.
void Directory::AppendChildHandles(const ScopedKernelLock& lock,
const Id& parent_id,
Directory::ChildHandles* result) {
Directory::Metahandles* result) {
const OrderedChildSet* children =
kernel_->parent_child_index.GetChildren(parent_id);
if (!children)
Expand All @@ -1298,9 +1291,5 @@ void Directory::AppendChildHandles(const ScopedKernelLock& lock,
}
}

ScopedKernelLock::ScopedKernelLock(const Directory* dir)
: scoped_lock_(dir->kernel_->mutex), dir_(const_cast<Directory*>(dir)) {
}

} // namespace syncable
} // namespace syncer
Loading

0 comments on commit 560009e

Please sign in to comment.