summaryrefslogtreecommitdiffstats
path: root/sync/engine
diff options
context:
space:
mode:
Diffstat (limited to 'sync/engine')
-rw-r--r--sync/engine/build_commit_command.cc33
-rw-r--r--sync/engine/build_commit_command.h16
-rw-r--r--sync/engine/build_commit_command_unittest.cc112
-rw-r--r--sync/engine/commit.cc43
-rw-r--r--sync/engine/get_commit_ids_command.cc41
-rw-r--r--sync/engine/get_commit_ids_command.h17
-rw-r--r--sync/engine/process_commit_response_command.cc19
-rw-r--r--sync/engine/process_commit_response_command.h3
-rw-r--r--sync/engine/process_commit_response_command_unittest.cc78
-rw-r--r--sync/engine/syncer_unittest.cc79
10 files changed, 227 insertions, 214 deletions
diff --git a/sync/engine/build_commit_command.cc b/sync/engine/build_commit_command.cc
index fb49553..ea0346c 100644
--- a/sync/engine/build_commit_command.cc
+++ b/sync/engine/build_commit_command.cc
@@ -16,10 +16,10 @@
#include "sync/sessions/ordered_commit_set.h"
#include "sync/sessions/sync_session.h"
#include "sync/syncable/directory.h"
-#include "sync/syncable/mutable_entry.h"
+#include "sync/syncable/entry.h"
+#include "sync/syncable/syncable_base_transaction.h"
#include "sync/syncable/syncable_changes_version.h"
#include "sync/syncable/syncable_proto_util.h"
-#include "sync/syncable/syncable_write_transaction.h"
#include "sync/util/time.h"
// TODO(vishwath): Remove this include after node positions have
@@ -40,7 +40,6 @@ using syncable::SERVER_ORDINAL_IN_PARENT;
using syncable::IS_UNAPPLIED_UPDATE;
using syncable::IS_UNSYNCED;
using syncable::Id;
-using syncable::MutableEntry;
using syncable::SPECIFICS;
// static
@@ -59,9 +58,14 @@ int64 BuildCommitCommand::GetGap() {
}
BuildCommitCommand::BuildCommitCommand(
+ syncable::BaseTransaction* trans,
const sessions::OrderedCommitSet& batch_commit_set,
- sync_pb::ClientToServerMessage* commit_message)
- : batch_commit_set_(batch_commit_set), commit_message_(commit_message) {
+ sync_pb::ClientToServerMessage* commit_message,
+ ExtensionsActivityMonitor::Records* extensions_activity_buffer)
+ : trans_(trans),
+ batch_commit_set_(batch_commit_set),
+ commit_message_(commit_message),
+ extensions_activity_buffer_(extensions_activity_buffer) {
}
BuildCommitCommand::~BuildCommitCommand() {}
@@ -81,10 +85,10 @@ void BuildCommitCommand::AddExtensionsActivityToMessage(
// We will push this list of extensions activity back into the
// ExtensionsActivityMonitor if this commit fails. That's why we must keep
// a copy of these records in the session.
- monitor->GetAndClearRecords(session->mutable_extensions_activity());
+ monitor->GetAndClearRecords(extensions_activity_buffer_);
const ExtensionsActivityMonitor::Records& records =
- session->extensions_activity();
+ *extensions_activity_buffer_;
for (ExtensionsActivityMonitor::Records::const_iterator it =
records.begin();
it != records.end(); ++it) {
@@ -111,7 +115,7 @@ void BuildCommitCommand::AddClientConfigParamsToMessage(
}
namespace {
-void SetEntrySpecifics(MutableEntry* meta_entry,
+void SetEntrySpecifics(Entry* meta_entry,
sync_pb::SyncEntity* sync_entry) {
// Add the new style extension and the folder bit.
sync_entry->mutable_specifics()->CopyFrom(meta_entry->Get(SPECIFICS));
@@ -126,8 +130,7 @@ SyncerError BuildCommitCommand::ExecuteImpl(SyncSession* session) {
commit_message_->set_message_contents(sync_pb::ClientToServerMessage::COMMIT);
sync_pb::CommitMessage* commit_message = commit_message_->mutable_commit();
- commit_message->set_cache_guid(
- session->write_transaction()->directory()->cache_guid());
+ commit_message->set_cache_guid(trans_->directory()->cache_guid());
AddExtensionsActivityToMessage(session, commit_message);
AddClientConfigParamsToMessage(session, commit_message);
@@ -143,8 +146,7 @@ SyncerError BuildCommitCommand::ExecuteImpl(SyncSession* session) {
Id id = batch_commit_set_.GetCommitIdAt(i);
sync_pb::SyncEntity* sync_entry = commit_message->add_entries();
sync_entry->set_id_string(SyncableIdToProto(id));
- MutableEntry meta_entry(session->write_transaction(),
- syncable::GET_BY_ID, id);
+ Entry meta_entry(trans_, syncable::GET_BY_ID, id);
CHECK(meta_entry.good());
DCHECK_NE(0UL,
@@ -173,7 +175,7 @@ SyncerError BuildCommitCommand::ExecuteImpl(SyncSession* session) {
Id new_parent_id;
if (meta_entry.Get(syncable::IS_DEL) &&
!meta_entry.Get(syncable::PARENT_ID).ServerKnows()) {
- new_parent_id = session->write_transaction()->root_id();
+ new_parent_id = trans_->root_id();
} else {
new_parent_id = meta_entry.Get(syncable::PARENT_ID);
}
@@ -231,8 +233,8 @@ SyncerError BuildCommitCommand::ExecuteImpl(SyncSession* session) {
FindAnchorPosition(syncable::PREV_ID, meta_entry),
FindAnchorPosition(syncable::NEXT_ID, meta_entry));
}
- position_block.first = InterpolatePosition(position_block.first,
- position_block.second);
+ position_block.first = BuildCommitCommand::InterpolatePosition(
+ position_block.first, position_block.second);
position_map[id] = position_block;
sync_entry->set_position_in_parent(position_block.first);
@@ -261,6 +263,7 @@ int64 BuildCommitCommand::FindAnchorPosition(syncable::IdField direction,
GetFirstPosition() : GetLastPosition();
}
+// static
int64 BuildCommitCommand::InterpolatePosition(const int64 lo,
const int64 hi) {
DCHECK_LE(lo, hi);
diff --git a/sync/engine/build_commit_command.h b/sync/engine/build_commit_command.h
index 253b6c0..a1b101d 100644
--- a/sync/engine/build_commit_command.h
+++ b/sync/engine/build_commit_command.h
@@ -11,6 +11,7 @@
#include "sync/base/sync_export.h"
#include "sync/engine/syncer_command.h"
#include "sync/syncable/entry_kernel.h"
+#include "sync/util/extensions_activity_monitor.h"
namespace syncer {
@@ -20,6 +21,7 @@ class OrderedCommitSet;
namespace syncable {
class Entry;
+class BaseTransaction;
}
// A class that contains the code used to serialize a set of sync items into a
@@ -36,8 +38,11 @@ class SYNC_EXPORT_PRIVATE BuildCommitCommand : public SyncerCommand {
//
// The commit_message parameter is an output parameter which will contain the
// fully initialized commit message once ExecuteImpl() has been called.
- BuildCommitCommand(const sessions::OrderedCommitSet& batch_commit_set,
- sync_pb::ClientToServerMessage* commit_message);
+ BuildCommitCommand(
+ syncable::BaseTransaction* trans,
+ const sessions::OrderedCommitSet& batch_commit_set,
+ sync_pb::ClientToServerMessage* commit_message,
+ ExtensionsActivityMonitor::Records* extensions_activity_buffer);
virtual ~BuildCommitCommand();
// SyncerCommand implementation.
@@ -67,15 +72,20 @@ class SYNC_EXPORT_PRIVATE BuildCommitCommand : public SyncerCommand {
const syncable::Entry& entry);
// Given two values of the type returned by FindAnchorPosition,
// compute a third value in between the two ranges.
- int64 InterpolatePosition(int64 lo, int64 hi);
+ static int64 InterpolatePosition(int64 lo, int64 hi);
DISALLOW_COPY_AND_ASSIGN(BuildCommitCommand);
+ // A pointer to a valid transaction not owned by this class.
+ syncable::BaseTransaction* trans_;
+
// Input parameter; see constructor comment.
const sessions::OrderedCommitSet& batch_commit_set_;
// Output parameter; see constructor comment.
sync_pb::ClientToServerMessage* commit_message_;
+
+ ExtensionsActivityMonitor::Records* extensions_activity_buffer_;
};
} // namespace syncer
diff --git a/sync/engine/build_commit_command_unittest.cc b/sync/engine/build_commit_command_unittest.cc
index 6ad3c55..1ad8667 100644
--- a/sync/engine/build_commit_command_unittest.cc
+++ b/sync/engine/build_commit_command_unittest.cc
@@ -8,98 +8,98 @@
namespace syncer {
// A test fixture for tests exercising ClearDataCommandTest.
-class BuildCommitCommandTest : public SyncerCommandTest {
- protected:
- BuildCommitCommandTest()
- : batch_commit_set_(ModelSafeRoutingInfo()),
- command_(batch_commit_set_, &commit_message_) {
- }
-
- private:
- sessions::OrderedCommitSet batch_commit_set_;
- sync_pb::ClientToServerMessage commit_message_;
-
- protected:
- BuildCommitCommand command_;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(BuildCommitCommandTest);
-};
+class BuildCommitCommandTest : public SyncerCommandTest { };
TEST_F(BuildCommitCommandTest, InterpolatePosition) {
EXPECT_LT(BuildCommitCommand::GetFirstPosition(),
BuildCommitCommand::GetLastPosition());
// Dense ranges.
- EXPECT_EQ(10, command_.InterpolatePosition(10, 10));
- EXPECT_EQ(11, command_.InterpolatePosition(10, 11));
- EXPECT_EQ(11, command_.InterpolatePosition(10, 12));
- EXPECT_EQ(11, command_.InterpolatePosition(10, 13));
- EXPECT_EQ(11, command_.InterpolatePosition(10, 14));
- EXPECT_EQ(11, command_.InterpolatePosition(10, 15));
- EXPECT_EQ(11, command_.InterpolatePosition(10, 16));
- EXPECT_EQ(11, command_.InterpolatePosition(10, 17));
- EXPECT_EQ(11, command_.InterpolatePosition(10, 18));
- EXPECT_EQ(12, command_.InterpolatePosition(10, 19));
- EXPECT_EQ(12, command_.InterpolatePosition(10, 20));
+ EXPECT_EQ(10, BuildCommitCommand::InterpolatePosition(10, 10));
+ EXPECT_EQ(11, BuildCommitCommand::InterpolatePosition(10, 11));
+ EXPECT_EQ(11, BuildCommitCommand::InterpolatePosition(10, 12));
+ EXPECT_EQ(11, BuildCommitCommand::InterpolatePosition(10, 13));
+ EXPECT_EQ(11, BuildCommitCommand::InterpolatePosition(10, 14));
+ EXPECT_EQ(11, BuildCommitCommand::InterpolatePosition(10, 15));
+ EXPECT_EQ(11, BuildCommitCommand::InterpolatePosition(10, 16));
+ EXPECT_EQ(11, BuildCommitCommand::InterpolatePosition(10, 17));
+ EXPECT_EQ(11, BuildCommitCommand::InterpolatePosition(10, 18));
+ EXPECT_EQ(12, BuildCommitCommand::InterpolatePosition(10, 19));
+ EXPECT_EQ(12, BuildCommitCommand::InterpolatePosition(10, 20));
// Sparse ranges.
EXPECT_EQ(0x32535ffe3dc97LL + BuildCommitCommand::GetGap(),
- command_.InterpolatePosition(0x32535ffe3dc97LL, 0x61abcd323122cLL));
+ BuildCommitCommand::InterpolatePosition(
+ 0x32535ffe3dc97LL, 0x61abcd323122cLL));
EXPECT_EQ(~0x61abcd323122cLL + BuildCommitCommand::GetGap(),
- command_.InterpolatePosition(~0x61abcd323122cLL, ~0x32535ffe3dc97LL));
+ BuildCommitCommand::InterpolatePosition(
+ ~0x61abcd323122cLL, ~0x32535ffe3dc97LL));
// Lower limits
EXPECT_EQ(BuildCommitCommand::GetFirstPosition() + 0x20,
- command_.InterpolatePosition(
- BuildCommitCommand::GetFirstPosition(),
- BuildCommitCommand::GetFirstPosition() + 0x100));
+ BuildCommitCommand::InterpolatePosition(
+ BuildCommitCommand::GetFirstPosition(),
+ BuildCommitCommand::GetFirstPosition() + 0x100));
EXPECT_EQ(BuildCommitCommand::GetFirstPosition() + 2,
- command_.InterpolatePosition(BuildCommitCommand::GetFirstPosition() + 1,
- BuildCommitCommand::GetFirstPosition() + 2));
+ BuildCommitCommand::InterpolatePosition(
+ BuildCommitCommand::GetFirstPosition() + 1,
+ BuildCommitCommand::GetFirstPosition() + 2));
EXPECT_EQ(BuildCommitCommand::GetFirstPosition() +
BuildCommitCommand::GetGap()/8 + 1,
- command_.InterpolatePosition(
- BuildCommitCommand::GetFirstPosition() + 1,
- BuildCommitCommand::GetFirstPosition() + 1 +
- BuildCommitCommand::GetGap()));
+ BuildCommitCommand::InterpolatePosition(
+ BuildCommitCommand::GetFirstPosition() + 1,
+ BuildCommitCommand::GetFirstPosition() + 1 +
+ BuildCommitCommand::GetGap()));
// Extremal cases.
EXPECT_EQ(0,
- command_.InterpolatePosition(BuildCommitCommand::GetFirstPosition(),
- BuildCommitCommand::GetLastPosition()));
+ BuildCommitCommand::InterpolatePosition(
+ BuildCommitCommand::GetFirstPosition(),
+ BuildCommitCommand::GetLastPosition()));
EXPECT_EQ(BuildCommitCommand::GetFirstPosition() + 1 +
BuildCommitCommand::GetGap(),
- command_.InterpolatePosition(BuildCommitCommand::GetFirstPosition() + 1,
- BuildCommitCommand::GetLastPosition()));
+ BuildCommitCommand::InterpolatePosition(
+ BuildCommitCommand::GetFirstPosition() + 1,
+ BuildCommitCommand::GetLastPosition()));
EXPECT_EQ(BuildCommitCommand::GetFirstPosition() + 1 +
BuildCommitCommand::GetGap(),
- command_.InterpolatePosition(BuildCommitCommand::GetFirstPosition() + 1,
- BuildCommitCommand::GetLastPosition() - 1));
+ BuildCommitCommand::InterpolatePosition(
+ BuildCommitCommand::GetFirstPosition() + 1,
+ BuildCommitCommand::GetLastPosition() - 1));
EXPECT_EQ(BuildCommitCommand::GetLastPosition() - 1 -
BuildCommitCommand::GetGap(),
- command_.InterpolatePosition(BuildCommitCommand::GetFirstPosition(),
- BuildCommitCommand::GetLastPosition() - 1));
+ BuildCommitCommand::InterpolatePosition(
+ BuildCommitCommand::GetFirstPosition(),
+ BuildCommitCommand::GetLastPosition() - 1));
// Edge cases around zero.
EXPECT_EQ(BuildCommitCommand::GetGap(),
- command_.InterpolatePosition(0, BuildCommitCommand::GetLastPosition()));
+ BuildCommitCommand::InterpolatePosition(
+ 0, BuildCommitCommand::GetLastPosition()));
EXPECT_EQ(BuildCommitCommand::GetGap() + 1,
- command_.InterpolatePosition(1, BuildCommitCommand::GetLastPosition()));
+ BuildCommitCommand::InterpolatePosition(
+ 1, BuildCommitCommand::GetLastPosition()));
EXPECT_EQ(BuildCommitCommand::GetGap() - 1,
- command_.InterpolatePosition(-1, BuildCommitCommand::GetLastPosition()));
+ BuildCommitCommand::InterpolatePosition(
+ -1, BuildCommitCommand::GetLastPosition()));
EXPECT_EQ(-BuildCommitCommand::GetGap(),
- command_.InterpolatePosition(BuildCommitCommand::GetFirstPosition(), 0));
+ BuildCommitCommand::InterpolatePosition(
+ BuildCommitCommand::GetFirstPosition(), 0));
EXPECT_EQ(-BuildCommitCommand::GetGap() + 1,
- command_.InterpolatePosition(BuildCommitCommand::GetFirstPosition(), 1));
+ BuildCommitCommand::InterpolatePosition(
+ BuildCommitCommand::GetFirstPosition(), 1));
EXPECT_EQ(-BuildCommitCommand::GetGap() - 1,
- command_.InterpolatePosition(BuildCommitCommand::GetFirstPosition(), -1));
+ BuildCommitCommand::InterpolatePosition(
+ BuildCommitCommand::GetFirstPosition(), -1));
EXPECT_EQ(BuildCommitCommand::GetGap() / 8,
- command_.InterpolatePosition(0, BuildCommitCommand::GetGap()));
+ BuildCommitCommand::InterpolatePosition(
+ 0, BuildCommitCommand::GetGap()));
EXPECT_EQ(BuildCommitCommand::GetGap() / 4,
- command_.InterpolatePosition(0, BuildCommitCommand::GetGap()*2));
+ BuildCommitCommand::InterpolatePosition(
+ 0, BuildCommitCommand::GetGap()*2));
EXPECT_EQ(BuildCommitCommand::GetGap(),
- command_.InterpolatePosition(0, BuildCommitCommand::GetGap()*2 + 1));
+ BuildCommitCommand::InterpolatePosition(
+ 0, BuildCommitCommand::GetGap()*2 + 1));
}
} // namespace syncer
diff --git a/sync/engine/commit.cc b/sync/engine/commit.cc
index f2728cd..55fe4f5 100644
--- a/sync/engine/commit.cc
+++ b/sync/engine/commit.cc
@@ -61,20 +61,21 @@ void ClearSyncingBits(syncable::Directory* dir,
// The ClientToServerMessage parameter is an output parameter which will contain
// the commit message which should be sent to the server. It is valid iff the
// return value of this function is true.
-bool PrepareCommitMessage(sessions::SyncSession* session,
- sessions::OrderedCommitSet* commit_set,
- sync_pb::ClientToServerMessage* commit_message) {
+bool PrepareCommitMessage(
+ sessions::SyncSession* session,
+ sessions::OrderedCommitSet* commit_set,
+ sync_pb::ClientToServerMessage* commit_message,
+ ExtensionsActivityMonitor::Records* extensions_activity_buffer) {
TRACE_EVENT0("sync", "PrepareCommitMessage");
commit_set->Clear();
commit_message->Clear();
WriteTransaction trans(FROM_HERE, SYNCER, session->context()->directory());
- sessions::ScopedSetSessionWriteTransaction set_trans(session, &trans);
// Fetch the items to commit.
const size_t batch_size = session->context()->max_commit_batch_size();
- GetCommitIdsCommand get_commit_ids_command(batch_size, commit_set);
+ GetCommitIdsCommand get_commit_ids_command(&trans, batch_size, commit_set);
get_commit_ids_command.Execute(session);
DVLOG(1) << "Commit message will contain " << commit_set->Size() << " items.";
@@ -83,19 +84,30 @@ bool PrepareCommitMessage(sessions::SyncSession* session,
}
// Serialize the message.
- BuildCommitCommand build_commit_command(*commit_set, commit_message);
+ BuildCommitCommand build_commit_command(&trans,
+ *commit_set,
+ commit_message,
+ extensions_activity_buffer);
build_commit_command.Execute(session);
- SetSyncingBits(session->write_transaction(), *commit_set);
+ SetSyncingBits(&trans, *commit_set);
return true;
}
SyncerError BuildAndPostCommitsImpl(Syncer* syncer,
sessions::SyncSession* session,
sessions::OrderedCommitSet* commit_set) {
- sync_pb::ClientToServerMessage commit_message;
- while (!syncer->ExitRequested() &&
- PrepareCommitMessage(session, commit_set, &commit_message)) {
+ while (!syncer->ExitRequested()) {
+ sync_pb::ClientToServerMessage commit_message;
+ ExtensionsActivityMonitor::Records extensions_activity_buffer;
+
+ if (!PrepareCommitMessage(session,
+ commit_set,
+ &commit_message,
+ &extensions_activity_buffer)) {
+ break;
+ }
+
sync_pb::ClientToServerResponse commit_response;
DVLOG(1) << "Sending commit message.";
@@ -104,6 +116,9 @@ SyncerError BuildAndPostCommitsImpl(Syncer* syncer,
&commit_message, &commit_response, session);
TRACE_EVENT_END0("sync", "PostCommit");
+ // TODO(rlarocque): Put all the post-commit logic in one place.
+ // See crbug.com/196338.
+
if (post_result != SYNCER_OK) {
LOG(WARNING) << "Post commit failed";
return post_result;
@@ -130,6 +145,14 @@ SyncerError BuildAndPostCommitsImpl(Syncer* syncer,
process_response_command.Execute(session);
TRACE_EVENT_END0("sync", "ProcessCommitResponse");
+ // If the commit failed, return the data to the ExtensionsActivityMonitor.
+ if (session->status_controller().
+ model_neutral_state().num_successful_bookmark_commits == 0) {
+ ExtensionsActivityMonitor* extensions_activity_monitor =
+ session->context()->extensions_monitor();
+ extensions_activity_monitor->PutRecords(extensions_activity_buffer);
+ }
+
if (processing_result != SYNCER_OK) {
return processing_result;
}
diff --git a/sync/engine/get_commit_ids_command.cc b/sync/engine/get_commit_ids_command.cc
index a84024a..6b2325f 100644
--- a/sync/engine/get_commit_ids_command.cc
+++ b/sync/engine/get_commit_ids_command.cc
@@ -11,11 +11,10 @@
#include "sync/engine/syncer_util.h"
#include "sync/engine/throttled_data_type_tracker.h"
#include "sync/syncable/entry.h"
-#include "sync/syncable/mutable_entry.h"
#include "sync/syncable/nigori_handler.h"
#include "sync/syncable/nigori_util.h"
+#include "sync/syncable/syncable_base_transaction.h"
#include "sync/syncable/syncable_util.h"
-#include "sync/syncable/syncable_write_transaction.h"
#include "sync/util/cryptographer.h"
using std::set;
@@ -28,9 +27,11 @@ using sessions::SyncSession;
using sessions::StatusController;
GetCommitIdsCommand::GetCommitIdsCommand(
+ syncable::BaseTransaction* trans,
const size_t commit_batch_size,
sessions::OrderedCommitSet* commit_set)
- : requested_commit_batch_size_(commit_batch_size),
+ : trans_(trans),
+ requested_commit_batch_size_(commit_batch_size),
commit_set_(commit_set) {
}
@@ -41,17 +42,17 @@ SyncerError GetCommitIdsCommand::ExecuteImpl(SyncSession* session) {
// are not in the correct order for commit.
std::set<int64> ready_unsynced_set;
syncable::Directory::UnsyncedMetaHandles all_unsynced_handles;
- GetUnsyncedEntries(session->write_transaction(),
+ GetUnsyncedEntries(trans_,
&all_unsynced_handles);
ModelTypeSet encrypted_types;
bool passphrase_missing = false;
Cryptographer* cryptographer =
session->context()->
- directory()->GetCryptographer(session->write_transaction());
+ directory()->GetCryptographer(trans_);
if (cryptographer) {
encrypted_types = session->context()->directory()->GetNigoriHandler()->
- GetEncryptedTypes(session->write_transaction());
+ GetEncryptedTypes(trans_);
passphrase_missing = cryptographer->has_pending_keys();
};
@@ -60,14 +61,14 @@ SyncerError GetCommitIdsCommand::ExecuteImpl(SyncSession* session) {
// We filter out all unready entries from the set of unsynced handles. This
// new set of ready and unsynced items (which excludes throttled items as
// well) is then what we use to determine what is a candidate for commit.
- FilterUnreadyEntries(session->write_transaction(),
+ FilterUnreadyEntries(trans_,
throttled_types,
encrypted_types,
passphrase_missing,
all_unsynced_handles,
&ready_unsynced_set);
- BuildCommitIds(session->write_transaction(),
+ BuildCommitIds(trans_,
session->context()->routing_info(),
ready_unsynced_set);
@@ -292,7 +293,7 @@ bool GetCommitIdsCommand::IsCommitBatchFull() const {
}
void GetCommitIdsCommand::AddCreatesAndMoves(
- syncable::WriteTransaction* write_transaction,
+ syncable::BaseTransaction* trans,
const ModelSafeRoutingInfo& routes,
const std::set<int64>& ready_unsynced_set) {
// Add moves and creates, and prepend their uncommitted parents.
@@ -302,7 +303,7 @@ void GetCommitIdsCommand::AddCreatesAndMoves(
if (commit_set_->HaveCommitItem(metahandle))
continue;
- syncable::Entry entry(write_transaction,
+ syncable::Entry entry(trans,
syncable::GET_BY_HANDLE,
metahandle);
if (!entry.Get(syncable::IS_DEL)) {
@@ -310,12 +311,12 @@ void GetCommitIdsCommand::AddCreatesAndMoves(
// dependencies are not in conflict.
OrderedCommitSet item_dependencies(routes);
if (AddUncommittedParentsAndTheirPredecessors(
- write_transaction,
+ trans,
routes,
ready_unsynced_set,
entry,
&item_dependencies) &&
- AddPredecessorsThenItem(write_transaction,
+ AddPredecessorsThenItem(trans,
routes,
ready_unsynced_set,
entry,
@@ -331,7 +332,7 @@ void GetCommitIdsCommand::AddCreatesAndMoves(
}
void GetCommitIdsCommand::AddDeletes(
- syncable::WriteTransaction* write_transaction,
+ syncable::BaseTransaction* trans,
const std::set<int64>& ready_unsynced_set) {
set<syncable::Id> legal_delete_parents;
@@ -341,11 +342,11 @@ void GetCommitIdsCommand::AddDeletes(
if (commit_set_->HaveCommitItem(metahandle))
continue;
- syncable::Entry entry(write_transaction, syncable::GET_BY_HANDLE,
+ syncable::Entry entry(trans, syncable::GET_BY_HANDLE,
metahandle);
if (entry.Get(syncable::IS_DEL)) {
- syncable::Entry parent(write_transaction, syncable::GET_BY_ID,
+ syncable::Entry parent(trans, syncable::GET_BY_ID,
entry.Get(syncable::PARENT_ID));
// If the parent is deleted and unsynced, then any children of that
// parent don't need to be added to the delete queue.
@@ -402,8 +403,8 @@ void GetCommitIdsCommand::AddDeletes(
int64 metahandle = *iter;
if (commit_set_->HaveCommitItem(metahandle))
continue;
- syncable::MutableEntry entry(write_transaction, syncable::GET_BY_HANDLE,
- metahandle);
+ syncable::Entry entry(trans, syncable::GET_BY_HANDLE,
+ metahandle);
if (entry.Get(syncable::IS_DEL)) {
syncable::Id parent_id = entry.Get(syncable::PARENT_ID);
if (legal_delete_parents.count(parent_id)) {
@@ -415,7 +416,7 @@ void GetCommitIdsCommand::AddDeletes(
}
void GetCommitIdsCommand::BuildCommitIds(
- syncable::WriteTransaction* write_transaction,
+ syncable::BaseTransaction* trans,
const ModelSafeRoutingInfo& routes,
const std::set<int64>& ready_unsynced_set) {
// Commits follow these rules:
@@ -428,10 +429,10 @@ void GetCommitIdsCommand::BuildCommitIds(
// delete trees.
// Add moves and creates, and prepend their uncommitted parents.
- AddCreatesAndMoves(write_transaction, routes, ready_unsynced_set);
+ AddCreatesAndMoves(trans, routes, ready_unsynced_set);
// Add all deletes.
- AddDeletes(write_transaction, ready_unsynced_set);
+ AddDeletes(trans, ready_unsynced_set);
}
} // namespace syncer
diff --git a/sync/engine/get_commit_ids_command.h b/sync/engine/get_commit_ids_command.h
index 86abf24..a41dd00 100644
--- a/sync/engine/get_commit_ids_command.h
+++ b/sync/engine/get_commit_ids_command.h
@@ -12,7 +12,6 @@
#include "sync/base/sync_export.h"
#include "sync/engine/syncer_command.h"
#include "sync/engine/syncer_util.h"
-#include "sync/sessions/ordered_commit_set.h"
#include "sync/sessions/sync_session.h"
#include "sync/syncable/directory.h"
@@ -21,6 +20,10 @@ using std::vector;
namespace syncer {
+namespace sessions {
+class OrderedCommitSet;
+}
+
// A class that contains the code used to search the syncable::Directory for
// locally modified items that are ready to be committed to the server.
//
@@ -36,7 +39,8 @@ class SYNC_EXPORT_PRIVATE GetCommitIdsCommand : public SyncerCommand {
// set of items that are ready to commit. Its size shall not exceed the
// provided batch_size. This contents of this "set" will be ordered; see the
// comments in this class' implementation for details.
- GetCommitIdsCommand(const size_t commit_batch_size,
+ GetCommitIdsCommand(syncable::BaseTransaction* trans,
+ const size_t commit_batch_size,
sessions::OrderedCommitSet* ordered_commit_set);
virtual ~GetCommitIdsCommand();
@@ -44,7 +48,7 @@ class SYNC_EXPORT_PRIVATE GetCommitIdsCommand : public SyncerCommand {
virtual SyncerError ExecuteImpl(sessions::SyncSession* session) OVERRIDE;
// Builds a vector of IDs that should be committed.
- void BuildCommitIds(syncable::WriteTransaction* write_transaction,
+ void BuildCommitIds(syncable::BaseTransaction* write_transaction,
const ModelSafeRoutingInfo& routes,
const std::set<int64>& ready_unsynced_set);
@@ -119,13 +123,16 @@ class SYNC_EXPORT_PRIVATE GetCommitIdsCommand : public SyncerCommand {
bool IsCommitBatchFull() const;
- void AddCreatesAndMoves(syncable::WriteTransaction* write_transaction,
+ void AddCreatesAndMoves(syncable::BaseTransaction* write_transaction,
const ModelSafeRoutingInfo& routes,
const std::set<int64>& ready_unsynced_set);
- void AddDeletes(syncable::WriteTransaction* write_transaction,
+ void AddDeletes(syncable::BaseTransaction* write_transaction,
const std::set<int64>& ready_unsynced_set);
+ // A pointer to a valid transaction not owned by this class.
+ syncable::BaseTransaction* trans_;
+
// Input parameter; see constructor comment.
const size_t requested_commit_batch_size_;
diff --git a/sync/engine/process_commit_response_command.cc b/sync/engine/process_commit_response_command.cc
index 49b1a4d..96a82bd 100644
--- a/sync/engine/process_commit_response_command.cc
+++ b/sync/engine/process_commit_response_command.cc
@@ -84,25 +84,6 @@ std::set<ModelSafeGroup> ProcessCommitResponseCommand::GetGroupsToChange(
SyncerError ProcessCommitResponseCommand::ModelChangingExecuteImpl(
SyncSession* session) {
- SyncerError result = ProcessCommitResponse(session);
- ExtensionsActivityMonitor* monitor = session->context()->extensions_monitor();
-
- // This is to be run on one model only: the bookmark model.
- if (session->status_controller().HasBookmarkCommitActivity()) {
- // If the commit failed, return the data to the ExtensionsActivityMonitor.
- if (session->status_controller().
- model_neutral_state().num_successful_bookmark_commits == 0) {
- monitor->PutRecords(session->extensions_activity());
- }
- // Clear our cached data in either case.
- session->mutable_extensions_activity()->clear();
- }
-
- return result;
-}
-
-SyncerError ProcessCommitResponseCommand::ProcessCommitResponse(
- SyncSession* session) {
syncable::Directory* dir = session->context()->directory();
StatusController* status = session->mutable_status_controller();
const CommitResponse& cr = commit_response_.commit();
diff --git a/sync/engine/process_commit_response_command.h b/sync/engine/process_commit_response_command.h
index f7efcfc..155a47d 100644
--- a/sync/engine/process_commit_response_command.h
+++ b/sync/engine/process_commit_response_command.h
@@ -73,9 +73,6 @@ class SYNC_EXPORT_PRIVATE ProcessCommitResponseCommand
const syncable::Id& pre_commit_id,
std::set<syncable::Id>* deleted_folders);
- // Actually does the work of execute.
- SyncerError ProcessCommitResponse(sessions::SyncSession* session);
-
void ProcessSuccessfulCommitResponse(
const sync_pb::SyncEntity& committed_entry,
const sync_pb::CommitResponse_EntryResponse& entry_response,
diff --git a/sync/engine/process_commit_response_command_unittest.cc b/sync/engine/process_commit_response_command_unittest.cc
index 3fb7017..5c96424 100644
--- a/sync/engine/process_commit_response_command_unittest.cc
+++ b/sync/engine/process_commit_response_command_unittest.cc
@@ -349,82 +349,4 @@ TEST_F(ProcessCommitResponseCommandTest, NewFolderCommitKeepsChildOrder) {
<< "Too few or too many children in parent folder after commit.";
}
-// This test fixture runs across a Cartesian product of per-type fail/success
-// possibilities.
-enum {
- TEST_PARAM_BOOKMARK_ENABLE_BIT,
- TEST_PARAM_AUTOFILL_ENABLE_BIT,
- TEST_PARAM_BIT_COUNT
-};
-class MixedResult :
- public ProcessCommitResponseCommandTest,
- public ::testing::WithParamInterface<int> {
- protected:
- bool ShouldFailBookmarkCommit() {
- return (GetParam() & (1 << TEST_PARAM_BOOKMARK_ENABLE_BIT)) == 0;
- }
- bool ShouldFailAutofillCommit() {
- return (GetParam() & (1 << TEST_PARAM_AUTOFILL_ENABLE_BIT)) == 0;
- }
-};
-INSTANTIATE_TEST_CASE_P(ProcessCommitResponse,
- MixedResult,
- testing::Range(0, 1 << TEST_PARAM_BIT_COUNT));
-
-// This test commits 2 items (one bookmark, one autofill) and validates what
-// happens to the extensions activity records. Commits could fail or succeed,
-// depending on the test parameter.
-TEST_P(MixedResult, ExtensionActivity) {
- sessions::OrderedCommitSet commit_set(session()->context()->routing_info());
- sync_pb::ClientToServerMessage request;
- sync_pb::ClientToServerResponse response;
-
- EXPECT_NE(routing_info().find(BOOKMARKS)->second,
- routing_info().find(AUTOFILL)->second)
- << "To not be lame, this test requires more than one active group.";
-
- // Bookmark item setup.
- CreateUnprocessedCommitResult(
- id_factory_.NewServerId(),
- id_factory_.root(), "Some bookmark", false,
- BOOKMARKS, &commit_set, &request, &response);
- if (ShouldFailBookmarkCommit())
- SetLastErrorCode(CommitResponse::TRANSIENT_ERROR, &response);
- // Autofill item setup.
- CreateUnprocessedCommitResult(
- id_factory_.NewServerId(),
- id_factory_.root(), "Some autofill", false,
- AUTOFILL, &commit_set, &request, &response);
- if (ShouldFailAutofillCommit())
- SetLastErrorCode(CommitResponse::TRANSIENT_ERROR, &response);
-
- // Put some extensions activity in the session.
- {
- ExtensionsActivityMonitor::Records* records =
- session()->mutable_extensions_activity();
- (*records)["ABC"].extension_id = "ABC";
- (*records)["ABC"].bookmark_write_count = 2049U;
- (*records)["xyz"].extension_id = "xyz";
- (*records)["xyz"].bookmark_write_count = 4U;
- }
- ProcessCommitResponseCommand command(commit_set, request, response);
- command.ExecuteImpl(session());
- ExpectGroupsToChange(command, GROUP_UI, GROUP_DB);
-
- ExtensionsActivityMonitor::Records final_monitor_records;
- context()->extensions_monitor()->GetAndClearRecords(&final_monitor_records);
-
- if (ShouldFailBookmarkCommit()) {
- ASSERT_EQ(2U, final_monitor_records.size())
- << "Should restore records after unsuccessful bookmark commit.";
- EXPECT_EQ("ABC", final_monitor_records["ABC"].extension_id);
- EXPECT_EQ("xyz", final_monitor_records["xyz"].extension_id);
- EXPECT_EQ(2049U, final_monitor_records["ABC"].bookmark_write_count);
- EXPECT_EQ(4U, final_monitor_records["xyz"].bookmark_write_count);
- } else {
- EXPECT_TRUE(final_monitor_records.empty())
- << "Should not restore records after successful bookmark commit.";
- }
-}
-
} // namespace syncer
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 304dd5a..1bdc13a 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -105,7 +105,6 @@ using syncable::SPECIFICS;
using syncable::SYNCING;
using syncable::UNITTEST;
-using sessions::ScopedSetSessionWriteTransaction;
using sessions::StatusController;
using sessions::SyncSessionContext;
using sessions::SyncSession;
@@ -409,18 +408,16 @@ class SyncerTest : public testing::Test,
const vector<syncable::Id>& expected_id_order) {
for (size_t limit = expected_id_order.size() + 2; limit > 0; --limit) {
WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
- ScopedSetSessionWriteTransaction set_trans(session_.get(), &wtrans);
ModelSafeRoutingInfo routes;
GetModelSafeRoutingInfo(&routes);
sessions::OrderedCommitSet output_set(routes);
- GetCommitIdsCommand command(limit, &output_set);
+ GetCommitIdsCommand command(&wtrans, limit, &output_set);
std::set<int64> ready_unsynced_set;
command.FilterUnreadyEntries(&wtrans, ModelTypeSet(),
ModelTypeSet(), false,
unsynced_handle_view, &ready_unsynced_set);
- command.BuildCommitIds(session_->write_transaction(), routes,
- ready_unsynced_set);
+ command.BuildCommitIds(&wtrans, routes, ready_unsynced_set);
size_t truncated_size = std::min(limit, expected_id_order.size());
ASSERT_EQ(truncated_size, output_set.Size());
for (size_t i = 0; i < truncated_size; ++i) {
@@ -4924,4 +4921,76 @@ TEST_F(SyncerPositionTiebreakingTest, MidLowHigh) {
ExpectLocalOrderIsByServerId();
}
+enum {
+ TEST_PARAM_BOOKMARK_ENABLE_BIT,
+ TEST_PARAM_AUTOFILL_ENABLE_BIT,
+ TEST_PARAM_BIT_COUNT
+};
+
+class MixedResult :
+ public SyncerTest,
+ public ::testing::WithParamInterface<int> {
+ protected:
+ bool ShouldFailBookmarkCommit() {
+ return (GetParam() & (1 << TEST_PARAM_BOOKMARK_ENABLE_BIT)) == 0;
+ }
+ bool ShouldFailAutofillCommit() {
+ return (GetParam() & (1 << TEST_PARAM_AUTOFILL_ENABLE_BIT)) == 0;
+ }
+};
+
+INSTANTIATE_TEST_CASE_P(ExtensionsActivity,
+ MixedResult,
+ testing::Range(0, 1 << TEST_PARAM_BIT_COUNT));
+
+TEST_P(MixedResult, ExtensionsActivity) {
+ {
+ WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
+
+ MutableEntry pref(&wtrans, CREATE, PREFERENCES, wtrans.root_id(), "pref");
+ ASSERT_TRUE(pref.good());
+ pref.Put(syncable::IS_UNSYNCED, true);
+
+ MutableEntry bookmark(
+ &wtrans, CREATE, BOOKMARKS, wtrans.root_id(), "bookmark");
+ ASSERT_TRUE(bookmark.good());
+ bookmark.Put(syncable::IS_UNSYNCED, true);
+
+ if (ShouldFailBookmarkCommit()) {
+ mock_server_->SetTransientErrorId(bookmark.Get(ID));
+ }
+
+ if (ShouldFailAutofillCommit()) {
+ mock_server_->SetTransientErrorId(pref.Get(ID));
+ }
+ }
+
+
+ // Put some extenions activity records into the monitor.
+ {
+ ExtensionsActivityMonitor::Records records;
+ records["ABC"].extension_id = "ABC";
+ records["ABC"].bookmark_write_count = 2049U;
+ records["xyz"].extension_id = "xyz";
+ records["xyz"].bookmark_write_count = 4U;
+ context_->extensions_monitor()->PutRecords(records);
+ }
+
+ SyncShareNudge();
+
+ ExtensionsActivityMonitor::Records final_monitor_records;
+ context_->extensions_monitor()->GetAndClearRecords(&final_monitor_records);
+ if (ShouldFailBookmarkCommit()) {
+ ASSERT_EQ(2U, final_monitor_records.size())
+ << "Should restore records after unsuccessful bookmark commit.";
+ EXPECT_EQ("ABC", final_monitor_records["ABC"].extension_id);
+ EXPECT_EQ("xyz", final_monitor_records["xyz"].extension_id);
+ EXPECT_EQ(2049U, final_monitor_records["ABC"].bookmark_write_count);
+ EXPECT_EQ(4U, final_monitor_records["xyz"].bookmark_write_count);
+ } else {
+ EXPECT_TRUE(final_monitor_records.empty())
+ << "Should not restore records after successful bookmark commit.";
+ }
+}
+
} // namespace syncer
generated by cgit v1.1 at 2025-05-14 14:41:45 +0000