Skip to content

Commit

Permalink
ModelTypeEntity refactoring to use EntityData and EntityMetadata
Browse files Browse the repository at this point in the history
1) Changed existing ModelTypeEntity to be based on
EntityMetadata, client key, and EntityDataPtr. Changed the
class construction functions slighly to better reuse the
code and to be able to pass the data without copying.

2) Moved model_type_entity* files next to shared_model_type_processor*

3) Did initial implementation of the data caching, although
it might still need some refinement. The code that prepares
commit request data structure now uses the cached EntityDataPtr.

4) Made necessary changes in SharedModelTypeProcessor to
ensure that all existing tests pass. Had to disable two
re-encryption tests because they expect the data received
in updates to be cached in ModelTypeEntity which isn't the
case anymore.

BUG=517657, 553638

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

Cr-Commit-Position: refs/heads/master@{#362475}
  • Loading branch information
stanisc authored and Commit bot committed Dec 1, 2015
1 parent 4bb8a45 commit 29a9029
Show file tree
Hide file tree
Showing 15 changed files with 696 additions and 641 deletions.
6 changes: 3 additions & 3 deletions sync/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ source_set("sync_core") {
"engine/get_updates_delegate.h",
"engine/get_updates_processor.cc",
"engine/get_updates_processor.h",
"engine/model_type_entity.cc",
"engine/model_type_entity.h",
"engine/model_type_worker.cc",
"engine/model_type_worker.h",
"engine/net/server_connection_manager.cc",
Expand Down Expand Up @@ -173,6 +171,7 @@ source_set("sync_core") {
"internal_api/js_sync_encryption_handler_observer.h",
"internal_api/js_sync_manager_observer.cc",
"internal_api/js_sync_manager_observer.h",
"internal_api/model_type_entity.cc",
"internal_api/model_type_store_backend.cc",
"internal_api/model_type_store_impl.cc",
"internal_api/protocol_event_buffer.cc",
Expand Down Expand Up @@ -243,6 +242,7 @@ source_set("sync_core") {
"internal_api/public/http_post_provider_interface.h",
"internal_api/public/internal_components_factory.h",
"internal_api/public/internal_components_factory_impl.h",
"internal_api/public/model_type_entity.h",
"internal_api/public/model_type_processor.cc",
"internal_api/public/model_type_processor.h",
"internal_api/public/model_type_store_backend.h",
Expand Down Expand Up @@ -632,7 +632,6 @@ test("sync_unit_tests") {
"engine/directory_update_handler_unittest.cc",
"engine/entity_tracker_unittest.cc",
"engine/get_updates_processor_unittest.cc",
"engine/model_type_entity_unittest.cc",
"engine/model_type_worker_unittest.cc",
"engine/sync_scheduler_unittest.cc",
"engine/syncer_proto_util_unittest.cc",
Expand All @@ -654,6 +653,7 @@ test("sync_unit_tests") {
"internal_api/js_mutation_event_observer_unittest.cc",
"internal_api/js_sync_encryption_handler_observer_unittest.cc",
"internal_api/js_sync_manager_observer_unittest.cc",
"internal_api/model_type_entity_unittest.cc",
"internal_api/model_type_store_backend_unittest.cc",
"internal_api/model_type_store_impl_unittest.cc",
"internal_api/protocol_event_buffer_unittest.cc",
Expand Down
6 changes: 3 additions & 3 deletions sync/engine/entity_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@ namespace syncer_v2 {

scoped_ptr<EntityTracker> EntityTracker::FromUpdateResponse(
const UpdateResponseData& data) {
// TODO(stanisc): Share entire EntityData with EntityTracker.
return make_scoped_ptr(new EntityTracker(
data.entity->id, data.entity->client_tag_hash, 0, data.response_version));
}

scoped_ptr<EntityTracker> EntityTracker::FromCommitRequest(
const CommitRequestData& data) {
// TODO(stanisc): Share entire EntityData with EntityTracker.
return make_scoped_ptr(
new EntityTracker(data.entity->id, data.entity->client_tag_hash, 0, 0));
}
Expand Down Expand Up @@ -96,6 +94,8 @@ void EntityTracker::RequestCommit(const CommitRequestData& data) {

// This entity is identified by its client tag. That value can never change.
DCHECK_EQ(client_tag_hash_, data.entity->client_tag_hash);
// TODO(stanisc): consider simply copying CommitRequestData instead of
// allocating one dynamically.
pending_commit_.reset(new CommitRequestData(data));

// Do our counter values indicate a conflict? If so, don't commit.
Expand Down Expand Up @@ -209,4 +209,4 @@ void EntityTracker::ClearPendingCommit() {
pending_commit_.reset();
}

} // namespace syncer
} // namespace syncer_v2
24 changes: 3 additions & 21 deletions sync/engine/entity_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,27 +81,13 @@ class SYNC_EXPORT EntityTracker {
void ClearPendingUpdate();

private:
// Initializes received update state. Does not initialize state related to
// pending commits and sets |is_commit_pending_| to false.
// Initializes the entity tracker's main fields. Does not initialize state
// related to a pending commit.
EntityTracker(const std::string& id,
const std::string& client_tag_hash,
int64 highest_commit_response_version,
int64 highest_gu_response_version);

// Initializes all fields. Sets |is_commit_pending_| to true.
EntityTracker(const std::string& id,
const std::string& client_tag_hash,
int64 highest_commit_response_version,
int64 highest_gu_response_version,
bool is_commit_pending,
int64 sequence_number,
int64 base_version,
base::Time ctime,
base::Time mtime,
const std::string& non_unique_name,
bool deleted,
const sync_pb::EntitySpecifics& specifics);

// Checks if the current state indicates a conflict.
//
// This can be true only while a call to this object is in progress.
Expand All @@ -126,10 +112,6 @@ class SYNC_EXPORT EntityTracker {
// The highest version seen in a GU response for this entry.
int64 highest_gu_response_version_;

// Flag that indicates whether or not we're waiting for a chance to commit
// this item.
bool is_commit_pending_;

// Used to track in-flight commit requests on the model thread. All we need
// to do here is return it back to the model thread when the pending commit
// is completed and confirmed. Not valid if no commit is pending.
Expand All @@ -149,6 +131,6 @@ class SYNC_EXPORT EntityTracker {
DISALLOW_COPY_AND_ASSIGN(EntityTracker);
};

} // namespace syncer
} // namespace syncer_v2

#endif // SYNC_ENGINE_ENTITY_TRACKER_H_
191 changes: 0 additions & 191 deletions sync/engine/model_type_entity.cc

This file was deleted.

Loading

0 comments on commit 29a9029

Please sign in to comment.