Skip to content

Commit

Permalink
sync: Expose DirectoryDebugInfoEmitters in engine
Browse files Browse the repository at this point in the history
Changes the lifetime of DirectoryTypeDebugInfoEmitters,
DirectoryCommitContributors, and DirectoryUpdateHandlers.  Under the old
lifetime management design, we had no state that lived beyond
a configure cycle.  This would be bad, since we want the debug info
counter values to remain after the initial configure is done.  The
refactoring ensures that the DirectoryTypeDebugInfoEmitters will live as
long as their data type is enabled.  As a side effect, the commit
contributors and update handlers get to live that long, too.

Plumbs the DirectoryTypeDebugInfoEmitters through the commit contributor
and update handler classes.  This prepares them for updating counters
and emitting events from sync/engine/ code, although this functionality
won't be implemented until the next patch in the series.

BUG=328606

Review URL: https://codereview.chromium.org/260613002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267740 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rlarocque@chromium.org committed May 2, 2014
1 parent 2206942 commit d667e6e
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 45 deletions.
18 changes: 14 additions & 4 deletions sync/engine/directory_commit_contribution.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ DirectoryCommitContribution::~DirectoryCommitContribution() {
scoped_ptr<DirectoryCommitContribution> DirectoryCommitContribution::Build(
syncable::Directory* dir,
ModelType type,
size_t max_entries) {
size_t max_entries,
DirectoryTypeDebugInfoEmitter* debug_info_emitter) {
DCHECK(debug_info_emitter);

std::vector<int64> metahandles;

syncable::ModelNeutralWriteTransaction trans(FROM_HERE, SYNCER, dir);
Expand All @@ -45,7 +48,12 @@ scoped_ptr<DirectoryCommitContribution> DirectoryCommitContribution::Build(
dir->GetDataTypeContext(&trans, type, &context);

return scoped_ptr<DirectoryCommitContribution>(
new DirectoryCommitContribution(metahandles, entities, context, dir));
new DirectoryCommitContribution(
metahandles,
entities,
context,
dir,
debug_info_emitter));
}

void DirectoryCommitContribution::AddToCommitMessage(
Expand Down Expand Up @@ -150,13 +158,15 @@ DirectoryCommitContribution::DirectoryCommitContribution(
const std::vector<int64>& metahandles,
const google::protobuf::RepeatedPtrField<sync_pb::SyncEntity>& entities,
const sync_pb::DataTypeContext& context,
syncable::Directory* dir)
syncable::Directory* dir,
DirectoryTypeDebugInfoEmitter* debug_info_emitter)
: dir_(dir),
metahandles_(metahandles),
entities_(entities),
context_(context),
entries_start_index_(0xDEADBEEF),
syncing_bits_set_(true) {}
syncing_bits_set_(true),
debug_info_emitter_(debug_info_emitter) {}

void DirectoryCommitContribution::UnsetSyncingBits() {
syncable::ModelNeutralWriteTransaction trans(FROM_HERE, SYNCER, dir_);
Expand Down
10 changes: 8 additions & 2 deletions sync/engine/directory_commit_contribution.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "sync/internal_api/public/base/model_type.h"
#include "sync/internal_api/public/util/syncer_error.h"
#include "sync/protocol/sync.pb.h"
#include "sync/sessions/directory_type_debug_info_emitter.h"
#include "sync/sessions/status_controller.h"

namespace syncer {
Expand Down Expand Up @@ -47,7 +48,8 @@ class SYNC_EXPORT_PRIVATE DirectoryCommitContribution
static scoped_ptr<DirectoryCommitContribution> Build(
syncable::Directory* dir,
ModelType type,
size_t max_items);
size_t max_items,
DirectoryTypeDebugInfoEmitter* debug_info_emitter);

// Serialize this contribution's entries to the given commit request |msg|.
//
Expand Down Expand Up @@ -83,7 +85,8 @@ class SYNC_EXPORT_PRIVATE DirectoryCommitContribution
const std::vector<int64>& metahandles,
const google::protobuf::RepeatedPtrField<sync_pb::SyncEntity>& entities,
const sync_pb::DataTypeContext& context,
syncable::Directory* directory);
syncable::Directory* directory,
DirectoryTypeDebugInfoEmitter* debug_info_emitter);

void UnsetSyncingBits();

Expand All @@ -99,6 +102,9 @@ class SYNC_EXPORT_PRIVATE DirectoryCommitContribution
// called. This flag must be unset by the time our destructor is called.
bool syncing_bits_set_;

// A pointer to the commit counters of our parent CommitContributor.
DirectoryTypeDebugInfoEmitter* debug_info_emitter_;

DISALLOW_COPY_AND_ASSIGN(DirectoryCommitContribution);
};

Expand Down
18 changes: 12 additions & 6 deletions sync/engine/directory_commit_contribution_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ TEST_F(DirectoryCommitContributionTest, GatherByTypes) {
CreateUnsyncedItem(&trans, EXTENSIONS, "extension1");
}

DirectoryTypeDebugInfoEmitter emitter;
scoped_ptr<DirectoryCommitContribution> cc(
DirectoryCommitContribution::Build(dir(), PREFERENCES, 5));
DirectoryCommitContribution::Build(dir(), PREFERENCES, 5, &emitter));
ASSERT_EQ(2U, cc->GetNumEntries());

const std::vector<int64>& metahandles = cc->metahandles_;
Expand All @@ -110,8 +111,9 @@ TEST_F(DirectoryCommitContributionTest, GatherAndTruncate) {
CreateUnsyncedItem(&trans, EXTENSIONS, "extension1");
}

DirectoryTypeDebugInfoEmitter emitter;
scoped_ptr<DirectoryCommitContribution> cc(
DirectoryCommitContribution::Build(dir(), PREFERENCES, 1));
DirectoryCommitContribution::Build(dir(), PREFERENCES, 1, &emitter));
ASSERT_EQ(1U, cc->GetNumEntries());

int64 only_metahandle = cc->metahandles_[0];
Expand All @@ -132,10 +134,12 @@ TEST_F(DirectoryCommitContributionTest, PrepareCommit) {
CreateUnsyncedItem(&trans, EXTENSIONS, "extension1");
}

DirectoryTypeDebugInfoEmitter emitter1;
DirectoryTypeDebugInfoEmitter emitter2;
scoped_ptr<DirectoryCommitContribution> pref_cc(
DirectoryCommitContribution::Build(dir(), PREFERENCES, 25));
DirectoryCommitContribution::Build(dir(), PREFERENCES, 25, &emitter1));
scoped_ptr<DirectoryCommitContribution> ext_cc(
DirectoryCommitContribution::Build(dir(), EXTENSIONS, 25));
DirectoryCommitContribution::Build(dir(), EXTENSIONS, 25, &emitter2));

sync_pb::ClientToServerMessage message;
pref_cc->AddToCommitMessage(&message);
Expand Down Expand Up @@ -184,10 +188,12 @@ TEST_F(DirectoryCommitContributionTest, ProcessCommitResponse) {
ext1_handle = CreateUnsyncedItem(&trans, EXTENSIONS, "extension1");
}

DirectoryTypeDebugInfoEmitter emitter1;
DirectoryTypeDebugInfoEmitter emitter2;
scoped_ptr<DirectoryCommitContribution> pref_cc(
DirectoryCommitContribution::Build(dir(), PREFERENCES, 25));
DirectoryCommitContribution::Build(dir(), PREFERENCES, 25, &emitter1));
scoped_ptr<DirectoryCommitContribution> ext_cc(
DirectoryCommitContribution::Build(dir(), EXTENSIONS, 25));
DirectoryCommitContribution::Build(dir(), EXTENSIONS, 25, &emitter2));

sync_pb::ClientToServerMessage message;
pref_cc->AddToCommitMessage(&message);
Expand Down
14 changes: 10 additions & 4 deletions sync/engine/directory_commit_contributor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,27 @@
#include "sync/engine/directory_commit_contributor.h"

#include "sync/engine/directory_commit_contribution.h"
#include "sync/sessions/directory_type_debug_info_emitter.h"

namespace syncer {

DirectoryCommitContributor::DirectoryCommitContributor(
syncable::Directory* dir,
ModelType type)
ModelType type,
DirectoryTypeDebugInfoEmitter* debug_info_emitter)
: dir_(dir),
type_(type) {}
type_(type),
debug_info_emitter_(debug_info_emitter) {}

DirectoryCommitContributor::~DirectoryCommitContributor() {}

scoped_ptr<CommitContribution>
DirectoryCommitContributor::GetContribution(size_t max_entries) {
return DirectoryCommitContribution::Build(dir_, type_, max_entries)
.PassAs<CommitContribution>();
return DirectoryCommitContribution::Build(
dir_,
type_,
max_entries,
debug_info_emitter_).PassAs<CommitContribution>();
}

} // namespace syncer
8 changes: 7 additions & 1 deletion sync/engine/directory_commit_contributor.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ namespace syncable {
class Directory;
}

class DirectoryTypeDebugInfoEmitter;

// This class represents the syncable::Directory as a source of items to commit
// to the sync server.
//
Expand All @@ -27,7 +29,9 @@ class Directory;
// of a DirectoryCommitContribution.
class DirectoryCommitContributor : public CommitContributor {
public:
DirectoryCommitContributor(syncable::Directory* dir, ModelType type);
DirectoryCommitContributor(syncable::Directory* dir,
ModelType type,
DirectoryTypeDebugInfoEmitter* debug_info_emitter);
virtual ~DirectoryCommitContributor();

virtual scoped_ptr<CommitContribution> GetContribution(
Expand All @@ -37,6 +41,8 @@ class DirectoryCommitContributor : public CommitContributor {
syncable::Directory* dir_;
ModelType type_;

DirectoryTypeDebugInfoEmitter* debug_info_emitter_;

DISALLOW_COPY_AND_ASSIGN(DirectoryCommitContributor);
};

Expand Down
8 changes: 5 additions & 3 deletions sync/engine/directory_update_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "sync/engine/conflict_resolver.h"
#include "sync/engine/process_updates_util.h"
#include "sync/engine/update_applicator.h"
#include "sync/sessions/status_controller.h"
#include "sync/sessions/directory_type_debug_info_emitter.h"
#include "sync/syncable/directory.h"
#include "sync/syncable/syncable_model_neutral_write_transaction.h"
#include "sync/syncable/syncable_write_transaction.h"
Expand All @@ -19,10 +19,12 @@ using syncable::SYNCER;
DirectoryUpdateHandler::DirectoryUpdateHandler(
syncable::Directory* dir,
ModelType type,
scoped_refptr<ModelSafeWorker> worker)
scoped_refptr<ModelSafeWorker> worker,
DirectoryTypeDebugInfoEmitter* debug_info_emitter)
: dir_(dir),
type_(type),
worker_(worker) {}
worker_(worker),
debug_info_emitter_(debug_info_emitter) {}

DirectoryUpdateHandler::~DirectoryUpdateHandler() {}

Expand Down
5 changes: 4 additions & 1 deletion sync/engine/directory_update_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ namespace syncable {
class Directory;
}

class DirectoryTypeDebugInfoEmitter;
class ModelSafeWorker;

// This class represents the syncable::Directory's processes for requesting and
Expand All @@ -44,7 +45,8 @@ class SYNC_EXPORT_PRIVATE DirectoryUpdateHandler : public UpdateHandler {
public:
DirectoryUpdateHandler(syncable::Directory* dir,
ModelType type,
scoped_refptr<ModelSafeWorker> worker);
scoped_refptr<ModelSafeWorker> worker,
DirectoryTypeDebugInfoEmitter* debug_info_emitter);
virtual ~DirectoryUpdateHandler();

// UpdateHandler implementation.
Expand Down Expand Up @@ -93,6 +95,7 @@ class SYNC_EXPORT_PRIVATE DirectoryUpdateHandler : public UpdateHandler {
syncable::Directory* dir_;
ModelType type_;
scoped_refptr<ModelSafeWorker> worker_;
DirectoryTypeDebugInfoEmitter* debug_info_emitter_;

scoped_ptr<sync_pb::GarbageCollectionDirective> cached_gc_directive_;

Expand Down
33 changes: 24 additions & 9 deletions sync/engine/directory_update_handler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "sync/internal_api/public/base/model_type.h"
#include "sync/internal_api/public/test/test_entry_factory.h"
#include "sync/protocol/sync.pb.h"
#include "sync/sessions/directory_type_debug_info_emitter.h"
#include "sync/sessions/status_controller.h"
#include "sync/syncable/directory.h"
#include "sync/syncable/entry.h"
Expand Down Expand Up @@ -55,6 +56,7 @@ class DirectoryUpdateHandlerProcessUpdateTest : public ::testing::Test {
syncable::Directory* dir() {
return dir_maker_.directory();
}

protected:
scoped_ptr<sync_pb::SyncEntity> CreateUpdate(
const std::string& id,
Expand Down Expand Up @@ -123,7 +125,8 @@ static const char kCacheGuid[] = "IrcjZ2jyzHDV9Io4+zKcXQ==";

// Test that the bookmark tag is set on newly downloaded items.
TEST_F(DirectoryUpdateHandlerProcessUpdateTest, NewBookmarkTag) {
DirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker());
DirectoryTypeDebugInfoEmitter emitter;
DirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker(), &emitter);
sync_pb::GetUpdatesResponse gu_response;
sessions::StatusController status;

Expand Down Expand Up @@ -161,7 +164,8 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest, NewBookmarkTag) {
// Test the receipt of a type root node.
TEST_F(DirectoryUpdateHandlerProcessUpdateTest,
ReceiveServerCreatedBookmarkFolders) {
DirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker());
DirectoryTypeDebugInfoEmitter emitter;
DirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker(), &emitter);
sync_pb::GetUpdatesResponse gu_response;
sessions::StatusController status;

Expand Down Expand Up @@ -195,7 +199,8 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest,

// Test the receipt of a non-bookmark item.
TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ReceiveNonBookmarkItem) {
DirectoryUpdateHandler handler(dir(), AUTOFILL, ui_worker());
DirectoryTypeDebugInfoEmitter emitter;
DirectoryUpdateHandler handler(dir(), AUTOFILL, ui_worker(), &emitter);
sync_pb::GetUpdatesResponse gu_response;
sessions::StatusController status;

Expand Down Expand Up @@ -226,7 +231,8 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ReceiveNonBookmarkItem) {

// Tests the setting of progress markers.
TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ProcessNewProgressMarkers) {
DirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker());
DirectoryTypeDebugInfoEmitter emitter;
DirectoryUpdateHandler handler(dir(), BOOKMARKS, ui_worker(), &emitter);

sync_pb::DataTypeProgressMarker progress;
progress.set_data_type_id(GetSpecificsFieldNumberFromModelType(BOOKMARKS));
Expand All @@ -242,7 +248,9 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ProcessNewProgressMarkers) {
}

TEST_F(DirectoryUpdateHandlerProcessUpdateTest, GarbageCollectionByVersion) {
DirectoryUpdateHandler handler(dir(), SYNCED_NOTIFICATIONS, ui_worker());
DirectoryTypeDebugInfoEmitter emitter;
DirectoryUpdateHandler handler(dir(), SYNCED_NOTIFICATIONS,
ui_worker(), &emitter);
sessions::StatusController status;

sync_pb::DataTypeProgressMarker progress;
Expand Down Expand Up @@ -305,7 +313,9 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest, GarbageCollectionByVersion) {
}

TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ContextVersion) {
DirectoryUpdateHandler handler(dir(), SYNCED_NOTIFICATIONS, ui_worker());
DirectoryTypeDebugInfoEmitter emitter;
DirectoryUpdateHandler handler(dir(), SYNCED_NOTIFICATIONS,
ui_worker(), &emitter);
sessions::StatusController status;
int field_number = GetSpecificsFieldNumberFromModelType(SYNCED_NOTIFICATIONS);

Expand Down Expand Up @@ -411,12 +421,14 @@ class DirectoryUpdateHandlerApplyUpdateTest : public ::testing::Test {

update_handler_map_.insert(std::make_pair(
BOOKMARKS,
new DirectoryUpdateHandler(directory(), BOOKMARKS, ui_worker_)));
new DirectoryUpdateHandler(directory(), BOOKMARKS,
ui_worker_, &bookmarks_emitter_)));
update_handler_map_.insert(std::make_pair(
PASSWORDS,
new DirectoryUpdateHandler(directory(),
PASSWORDS,
password_worker_)));
PASSWORDS,
password_worker_,
&passwords_emitter_)));
}

virtual void TearDown() OVERRIDE {
Expand Down Expand Up @@ -451,6 +463,9 @@ class DirectoryUpdateHandlerApplyUpdateTest : public ::testing::Test {
scoped_refptr<FakeModelWorker> password_worker_;
scoped_refptr<FakeModelWorker> passive_worker_;

DirectoryTypeDebugInfoEmitter bookmarks_emitter_;
DirectoryTypeDebugInfoEmitter passwords_emitter_;

UpdateHandlerMap update_handler_map_;
STLValueDeleter<UpdateHandlerMap> update_handler_map_deleter_;
};
Expand Down
Loading

0 comments on commit d667e6e

Please sign in to comment.