summaryrefslogtreecommitdiffstats
path: root/chrome/browser/sync
diff options
context:
space:
mode:
authorakalin@chromium.org <akalin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-12-03 03:11:51 +0000
committerakalin@chromium.org <akalin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-12-03 03:11:51 +0000
commit9de8fb77690b7752953bc9ff8c60c94182a65453 (patch)
treebaa6ab5fcf154908f17ad73faaf5b746359f7aa4 /chrome/browser/sync
parent6540fe376db91459f3d3d03d36a2b7dbd3b0c6a2 (diff)
downloadchromium_src-9de8fb77690b7752953bc9ff8c60c94182a65453.zip
chromium_src-9de8fb77690b7752953bc9ff8c60c94182a65453.tar.gz
chromium_src-9de8fb77690b7752953bc9ff8c60c94182a65453.tar.bz2
Revert 112815 - [Sync] Make syncer commands avoid posting tasks on threads with no work to do
Add abstract GetGroupsToChange() method to ModelChangingSyncerCommand. Use that to figure out which worker threads to post work on (instead of posting on all of them). Implement GetGroupsToChange() for each ModelChangingSyncerCommand. Add GetEnabledGroups() and GetEnabledGroupsWithConflicts() functions to SyncSession. Key unapplied updates index by type for ApplyUpdatesCommand. Make the abstract methods of ModelChangingSyncerCommand protected. This patch includes a speculative fix for the perf regression introduced by the last time this was landed (112178). BUG=97832 TEST= Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112178 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112743 Review URL: http://codereview.chromium.org/8637006 TBR=akalin@chromium.org Review URL: http://codereview.chromium.org/8785015 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@112855 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/sync')
-rw-r--r--chrome/browser/sync/engine/apply_updates_command.cc56
-rw-r--r--chrome/browser/sync/engine/apply_updates_command.h3
-rw-r--r--chrome/browser/sync/engine/apply_updates_command_unittest.cc14
-rw-r--r--chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc5
-rw-r--r--chrome/browser/sync/engine/build_and_process_conflict_sets_command.h3
-rw-r--r--chrome/browser/sync/engine/build_and_process_conflict_sets_command_unittest.cc52
-rw-r--r--chrome/browser/sync/engine/model_changing_syncer_command.cc23
-rw-r--r--chrome/browser/sync/engine/model_changing_syncer_command.h17
-rw-r--r--chrome/browser/sync/engine/model_changing_syncer_command_unittest.cc74
-rw-r--r--chrome/browser/sync/engine/process_commit_response_command.cc22
-rw-r--r--chrome/browser/sync/engine/process_commit_response_command.h6
-rw-r--r--chrome/browser/sync/engine/process_commit_response_command_unittest.cc6
-rw-r--r--chrome/browser/sync/engine/process_updates_command.cc10
-rw-r--r--chrome/browser/sync/engine/process_updates_command.h6
-rw-r--r--chrome/browser/sync/engine/process_updates_command_unittest.cc59
-rw-r--r--chrome/browser/sync/engine/resolve_conflicts_command.cc5
-rw-r--r--chrome/browser/sync/engine/resolve_conflicts_command.h3
-rw-r--r--chrome/browser/sync/engine/resolve_conflicts_command_unittest.cc51
-rw-r--r--chrome/browser/sync/engine/sync_scheduler.cc2
-rw-r--r--chrome/browser/sync/engine/syncer_unittest.cc4
-rw-r--r--chrome/browser/sync/engine/update_applicator.cc10
-rw-r--r--chrome/browser/sync/engine/verify_updates_command.cc15
-rw-r--r--chrome/browser/sync/engine/verify_updates_command.h5
-rw-r--r--chrome/browser/sync/engine/verify_updates_command_unittest.cc5
-rw-r--r--chrome/browser/sync/sessions/status_controller.cc12
-rw-r--r--chrome/browser/sync/sessions/status_controller.h4
-rw-r--r--chrome/browser/sync/sessions/sync_session.cc92
-rw-r--r--chrome/browser/sync/sessions/sync_session.h18
-rw-r--r--chrome/browser/sync/sessions/sync_session_unittest.cc142
-rw-r--r--chrome/browser/sync/syncable/syncable.cc116
-rw-r--r--chrome/browser/sync/syncable/syncable.h18
-rw-r--r--chrome/browser/sync/syncable/syncable_unittest.cc17
-rw-r--r--chrome/browser/sync/test/engine/fake_model_safe_worker_registrar.cc2
-rw-r--r--chrome/browser/sync/test/engine/syncer_command_test.h41
34 files changed, 104 insertions, 814 deletions
diff --git a/chrome/browser/sync/engine/apply_updates_command.cc b/chrome/browser/sync/engine/apply_updates_command.cc
index 57472b5..8b5a6b8 100644
--- a/chrome/browser/sync/engine/apply_updates_command.cc
+++ b/chrome/browser/sync/engine/apply_updates_command.cc
@@ -17,35 +17,6 @@ using sessions::SyncSession;
ApplyUpdatesCommand::ApplyUpdatesCommand() {}
ApplyUpdatesCommand::~ApplyUpdatesCommand() {}
-std::set<ModelSafeGroup> ApplyUpdatesCommand::GetGroupsToChange(
- const sessions::SyncSession& session) const {
- std::set<ModelSafeGroup> groups_with_unapplied_updates;
-
- syncable::ModelTypeBitSet server_types_with_unapplied_updates;
- {
- syncable::ScopedDirLookup dir(session.context()->directory_manager(),
- session.context()->account_name());
- if (!dir.good()) {
- LOG(ERROR) << "Scoped dir lookup failed!";
- return groups_with_unapplied_updates;
- }
-
- syncable::ReadTransaction trans(FROM_HERE, dir);
- server_types_with_unapplied_updates =
- dir->GetServerTypesWithUnappliedUpdates(&trans);
- }
-
- for (int i = 0; i < syncable::MODEL_TYPE_COUNT; ++i) {
- const syncable::ModelType type = syncable::ModelTypeFromInt(i);
- if (server_types_with_unapplied_updates.test(type)) {
- groups_with_unapplied_updates.insert(
- GetGroupForModelType(type, session.routing_info()));
- }
- }
-
- return groups_with_unapplied_updates;
-}
-
void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
syncable::ScopedDirLookup dir(session->context()->directory_manager(),
session->context()->account_name());
@@ -53,27 +24,9 @@ void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
LOG(ERROR) << "Scoped dir lookup failed!";
return;
}
-
syncable::WriteTransaction trans(FROM_HERE, syncable::SYNCER, dir);
-
- // Compute server types with unapplied updates that fall under our
- // group restriction.
- const syncable::ModelTypeBitSet server_types_with_unapplied_updates =
- dir->GetServerTypesWithUnappliedUpdates(&trans);
- syncable::ModelTypeBitSet server_type_restriction;
- for (int i = 0; i < syncable::MODEL_TYPE_COUNT; ++i) {
- const syncable::ModelType server_type = syncable::ModelTypeFromInt(i);
- if (server_types_with_unapplied_updates.test(server_type)) {
- if (GetGroupForModelType(server_type, session->routing_info()) ==
- session->status_controller().group_restriction()) {
- server_type_restriction.set(server_type);
- }
- }
- }
-
syncable::Directory::UnappliedUpdateMetaHandles handles;
- dir->GetUnappliedUpdateMetaHandles(
- &trans, server_type_restriction, &handles);
+ dir->GetUnappliedUpdateMetaHandles(&trans, &handles);
UpdateApplicator applicator(
session->context()->resolver(),
@@ -89,6 +42,13 @@ void ApplyUpdatesCommand::ModelChangingExecuteImpl(SyncSession* session) {
// some subset of the currently synced datatypes.
const sessions::StatusController& status(session->status_controller());
if (status.ServerSaysNothingMoreToDownload()) {
+ syncable::ScopedDirLookup dir(session->context()->directory_manager(),
+ session->context()->account_name());
+ if (!dir.good()) {
+ LOG(ERROR) << "Scoped dir lookup failed!";
+ return;
+ }
+
for (int i = syncable::FIRST_REAL_MODEL_TYPE;
i < syncable::MODEL_TYPE_COUNT; ++i) {
syncable::ModelType model_type = syncable::ModelTypeFromInt(i);
diff --git a/chrome/browser/sync/engine/apply_updates_command.h b/chrome/browser/sync/engine/apply_updates_command.h
index d75754f..a6c9d2b 100644
--- a/chrome/browser/sync/engine/apply_updates_command.h
+++ b/chrome/browser/sync/engine/apply_updates_command.h
@@ -16,10 +16,7 @@ class ApplyUpdatesCommand : public ModelChangingSyncerCommand {
ApplyUpdatesCommand();
virtual ~ApplyUpdatesCommand();
- protected:
// ModelChangingSyncerCommand implementation.
- virtual std::set<ModelSafeGroup> GetGroupsToChange(
- const sessions::SyncSession& session) const OVERRIDE;
virtual void ModelChangingExecuteImpl(
sessions::SyncSession* session) OVERRIDE;
diff --git a/chrome/browser/sync/engine/apply_updates_command_unittest.cc b/chrome/browser/sync/engine/apply_updates_command_unittest.cc
index 734f9207..b951e4f 100644
--- a/chrome/browser/sync/engine/apply_updates_command_unittest.cc
+++ b/chrome/browser/sync/engine/apply_updates_command_unittest.cc
@@ -57,11 +57,12 @@ class ApplyUpdatesCommandTest : public SyncerCommandTest {
make_scoped_refptr(new FakeModelWorker(GROUP_UI)));
workers()->push_back(
make_scoped_refptr(new FakeModelWorker(GROUP_PASSWORD)));
+ workers()->push_back(
+ make_scoped_refptr(new FakeModelWorker(GROUP_PASSIVE)));
(*mutable_routing_info())[syncable::BOOKMARKS] = GROUP_UI;
(*mutable_routing_info())[syncable::PASSWORDS] = GROUP_PASSWORD;
(*mutable_routing_info())[syncable::NIGORI] = GROUP_PASSIVE;
SyncerCommandTest::SetUp();
- ExpectNoGroupsToChange(apply_updates_command_);
}
// Create a new unapplied folder node with a parent.
@@ -159,7 +160,6 @@ TEST_F(ApplyUpdatesCommandTest, Simple) {
DefaultBookmarkSpecifics(),
"parent");
- ExpectGroupToChange(apply_updates_command_, GROUP_UI);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
@@ -195,7 +195,6 @@ TEST_F(ApplyUpdatesCommandTest, UpdateWithChildrenBeforeParents) {
DefaultBookmarkSpecifics(),
"parent");
- ExpectGroupToChange(apply_updates_command_, GROUP_UI);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
@@ -219,7 +218,6 @@ TEST_F(ApplyUpdatesCommandTest, NestedItemsWithUnknownParent) {
DefaultBookmarkSpecifics(),
"some_item");
- ExpectGroupToChange(apply_updates_command_, GROUP_UI);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
@@ -256,7 +254,6 @@ TEST_F(ApplyUpdatesCommandTest, ItemsBothKnownAndUnknown) {
DefaultBookmarkSpecifics(),
root_server_id);
- ExpectGroupToChange(apply_updates_command_, GROUP_UI);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
@@ -295,7 +292,6 @@ TEST_F(ApplyUpdatesCommandTest, DecryptablePassword) {
specifics.MutableExtension(sync_pb::password)->mutable_encrypted());
CreateUnappliedNewItem("item", specifics, false);
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSWORD);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
@@ -324,7 +320,6 @@ TEST_F(ApplyUpdatesCommandTest, UndecryptableData) {
encrypted_password.MutableExtension(sync_pb::password);
CreateUnappliedNewItem("item3", encrypted_password, false);
- ExpectGroupsToChange(apply_updates_command_, GROUP_UI, GROUP_PASSWORD);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
@@ -399,7 +394,6 @@ TEST_F(ApplyUpdatesCommandTest, SomeUndecryptablePassword) {
CreateUnappliedNewItem("item2", specifics, false);
}
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSWORD);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
@@ -454,7 +448,6 @@ TEST_F(ApplyUpdatesCommandTest, NigoriUpdate) {
specifics, true);
EXPECT_FALSE(cryptographer->has_pending_keys());
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
@@ -506,7 +499,6 @@ TEST_F(ApplyUpdatesCommandTest, NigoriUpdateForDisabledTypes) {
specifics, true);
EXPECT_FALSE(cryptographer->has_pending_keys());
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
@@ -594,7 +586,6 @@ TEST_F(ApplyUpdatesCommandTest, EncryptUnsyncedChanges) {
EXPECT_EQ(2*batch_s+1, handles.size());
}
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
@@ -697,7 +688,6 @@ TEST_F(ApplyUpdatesCommandTest, CannotEncryptUnsyncedChanges) {
EXPECT_EQ(2*batch_s+1, handles.size());
}
- ExpectGroupToChange(apply_updates_command_, GROUP_PASSIVE);
apply_updates_command_.ExecuteImpl(session());
sessions::StatusController* status = session()->mutable_status_controller();
diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
index 065f021..9810225 100644
--- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
+++ b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.cc
@@ -30,11 +30,6 @@ using std::vector;
BuildAndProcessConflictSetsCommand::BuildAndProcessConflictSetsCommand() {}
BuildAndProcessConflictSetsCommand::~BuildAndProcessConflictSetsCommand() {}
-std::set<ModelSafeGroup> BuildAndProcessConflictSetsCommand::GetGroupsToChange(
- const sessions::SyncSession& session) const {
- return session.GetEnabledGroupsWithConflicts();
-}
-
void BuildAndProcessConflictSetsCommand::ModelChangingExecuteImpl(
SyncSession* session) {
session->mutable_status_controller()->update_conflict_sets_built(
diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h
index 8ef7b8d..a446e7e 100644
--- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h
+++ b/chrome/browser/sync/engine/build_and_process_conflict_sets_command.h
@@ -35,10 +35,7 @@ class BuildAndProcessConflictSetsCommand : public ModelChangingSyncerCommand {
BuildAndProcessConflictSetsCommand();
virtual ~BuildAndProcessConflictSetsCommand();
- protected:
// ModelChangingSyncerCommand implementation.
- virtual std::set<ModelSafeGroup> GetGroupsToChange(
- const sessions::SyncSession& session) const OVERRIDE;
virtual void ModelChangingExecuteImpl(
sessions::SyncSession* session) OVERRIDE;
diff --git a/chrome/browser/sync/engine/build_and_process_conflict_sets_command_unittest.cc b/chrome/browser/sync/engine/build_and_process_conflict_sets_command_unittest.cc
deleted file mode 100644
index 3152484..0000000
--- a/chrome/browser/sync/engine/build_and_process_conflict_sets_command_unittest.cc
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright (c) 2011 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 "base/basictypes.h"
-#include "base/memory/ref_counted.h"
-#include "chrome/browser/sync/engine/build_and_process_conflict_sets_command.h"
-#include "chrome/browser/sync/sessions/session_state.h"
-#include "chrome/browser/sync/sessions/sync_session.h"
-#include "chrome/browser/sync/syncable/model_type.h"
-#include "chrome/browser/sync/syncable/syncable_id.h"
-#include "chrome/browser/sync/test/engine/fake_model_worker.h"
-#include "chrome/browser/sync/test/engine/syncer_command_test.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace browser_sync {
-
-namespace {
-
-class BuildAndProcessConflictSetsCommandTest : public SyncerCommandTest {
- protected:
- BuildAndProcessConflictSetsCommandTest() {}
- virtual ~BuildAndProcessConflictSetsCommandTest() {}
-
- virtual void SetUp() {
- workers()->push_back(
- make_scoped_refptr(new FakeModelWorker(GROUP_UI)));
- workers()->push_back(
- make_scoped_refptr(new FakeModelWorker(GROUP_PASSWORD)));
- (*mutable_routing_info())[syncable::BOOKMARKS] = GROUP_UI;
- (*mutable_routing_info())[syncable::PASSWORDS] = GROUP_PASSWORD;
- SyncerCommandTest::SetUp();
- }
-
- BuildAndProcessConflictSetsCommand command_;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(BuildAndProcessConflictSetsCommandTest);
-};
-
-TEST_F(BuildAndProcessConflictSetsCommandTest, GetGroupsToChange) {
- ExpectNoGroupsToChange(command_);
- // Put GROUP_UI in conflict.
- session()->mutable_status_controller()->
- GetUnrestrictedMutableConflictProgressForTest(GROUP_UI)->
- AddConflictingItemById(syncable::Id());
- ExpectGroupToChange(command_, GROUP_UI);
-}
-
-} // namespace
-
-} // namespace browser_sync
diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.cc b/chrome/browser/sync/engine/model_changing_syncer_command.cc
index 2ea2e71..a8df65a 100644
--- a/chrome/browser/sync/engine/model_changing_syncer_command.cc
+++ b/chrome/browser/sync/engine/model_changing_syncer_command.cc
@@ -6,6 +6,7 @@
#include "base/basictypes.h"
#include "base/callback_old.h"
+#include "chrome/browser/sync/engine/model_safe_worker.h"
#include "chrome/browser/sync/sessions/status_controller.h"
#include "chrome/browser/sync/sessions/sync_session.h"
#include "chrome/browser/sync/util/unrecoverable_error_info.h"
@@ -18,13 +19,27 @@ void ModelChangingSyncerCommand::ExecuteImpl(sessions::SyncSession* session) {
return;
}
- const std::set<ModelSafeGroup>& groups_to_change =
- GetGroupsToChange(*work_session_);
+ // Project the list of active types (i.e., types in the routing
+ // info) to a list of groups.
+ //
+ // TODO(akalin): Make this overrideable by subclasses (who might be
+ // working on a subset of |active_groups|). (See
+ // http://crbug.com/97832.)
+ std::set<ModelSafeGroup> active_groups;
+ const ModelSafeRoutingInfo& routing_info = session->routing_info();
+ for (ModelSafeRoutingInfo::const_iterator it = routing_info.begin();
+ it != routing_info.end(); ++it) {
+ active_groups.insert(it->second);
+ }
+ // Always work on GROUP_PASSIVE, since that's the group that
+ // top-level folders map to.
+ active_groups.insert(GROUP_PASSIVE);
+
for (size_t i = 0; i < session->workers().size(); ++i) {
- ModelSafeWorker* worker = work_session_->workers()[i];
+ ModelSafeWorker* worker = session->workers()[i];
ModelSafeGroup group = worker->GetModelSafeGroup();
// Skip workers whose group isn't active.
- if (groups_to_change.count(group) == 0u) {
+ if (active_groups.find(group) == active_groups.end()) {
DVLOG(2) << "Skipping worker for group "
<< ModelSafeGroupToString(group);
continue;
diff --git a/chrome/browser/sync/engine/model_changing_syncer_command.h b/chrome/browser/sync/engine/model_changing_syncer_command.h
index e6f6ca1..9a2311e 100644
--- a/chrome/browser/sync/engine/model_changing_syncer_command.h
+++ b/chrome/browser/sync/engine/model_changing_syncer_command.h
@@ -7,7 +7,6 @@
#pragma once
#include "base/compiler_specific.h"
-#include "chrome/browser/sync/engine/model_safe_worker.h"
#include "chrome/browser/sync/engine/syncer_command.h"
#include "chrome/browser/sync/util/unrecoverable_error_info.h"
@@ -42,22 +41,6 @@ class ModelChangingSyncerCommand : public SyncerCommand {
return UnrecoverableErrorInfo();
}
- std::set<ModelSafeGroup> GetGroupsToChangeForTest(
- const sessions::SyncSession& session) const {
- return GetGroupsToChange(session);
- }
-
- protected:
- // This should return the set of groups in |session| that need to be
- // changed. The returned set should be a subset of
- // session.GetEnabledGroups(). Subclasses can guarantee this either
- // by calling one of the session.GetEnabledGroups*() functions and
- // filtering that, or using GetGroupForModelType() (which handles
- // top-level/unspecified nodes) to project from model types to
- // groups.
- virtual std::set<ModelSafeGroup> GetGroupsToChange(
- const sessions::SyncSession& session) const = 0;
-
// Sometimes, a command has work to do that needs to touch global state
// belonging to multiple ModelSafeGroups, but in a way that is known to be
// safe. This will be called once, prior to ModelChangingExecuteImpl,
diff --git a/chrome/browser/sync/engine/model_changing_syncer_command_unittest.cc b/chrome/browser/sync/engine/model_changing_syncer_command_unittest.cc
deleted file mode 100644
index 5f07d8a..0000000
--- a/chrome/browser/sync/engine/model_changing_syncer_command_unittest.cc
+++ /dev/null
@@ -1,74 +0,0 @@
-// Copyright (c) 2011 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 "base/basictypes.h"
-#include "base/memory/ref_counted.h"
-#include "chrome/browser/sync/engine/model_changing_syncer_command.h"
-#include "chrome/browser/sync/sessions/sync_session.h"
-#include "chrome/browser/sync/syncable/model_type.h"
-#include "chrome/browser/sync/test/engine/fake_model_worker.h"
-#include "chrome/browser/sync/test/engine/syncer_command_test.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace browser_sync {
-
-namespace {
-
-class FakeModelChangingSyncerCommand : public ModelChangingSyncerCommand {
- public:
- FakeModelChangingSyncerCommand() {}
- virtual ~FakeModelChangingSyncerCommand() {}
-
- const std::set<ModelSafeGroup>& changed_groups() const {
- return changed_groups_;
- }
-
- protected:
- virtual std::set<ModelSafeGroup> GetGroupsToChange(
- const sessions::SyncSession& session) const {
- return session.GetEnabledGroups();
- }
-
- virtual void ModelChangingExecuteImpl(sessions::SyncSession* session) {
- changed_groups_.insert(session->status_controller().group_restriction());
- }
-
- private:
- std::set<ModelSafeGroup> changed_groups_;
-
- DISALLOW_COPY_AND_ASSIGN(FakeModelChangingSyncerCommand);
-};
-
-class ModelChangingSyncerCommandTest : public SyncerCommandTest {
- protected:
- ModelChangingSyncerCommandTest() {}
- virtual ~ModelChangingSyncerCommandTest() {}
-
- virtual void SetUp() {
- workers()->push_back(
- make_scoped_refptr(new FakeModelWorker(GROUP_UI)));
- workers()->push_back(
- make_scoped_refptr(new FakeModelWorker(GROUP_PASSWORD)));
- (*mutable_routing_info())[syncable::BOOKMARKS] = GROUP_UI;
- (*mutable_routing_info())[syncable::PASSWORDS] = GROUP_PASSWORD;
- SyncerCommandTest::SetUp();
- }
-
- FakeModelChangingSyncerCommand command_;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(ModelChangingSyncerCommandTest);
-};
-
-TEST_F(ModelChangingSyncerCommandTest, Basic) {
- ExpectGroupsToChange(command_, GROUP_UI, GROUP_PASSWORD, GROUP_PASSIVE);
- EXPECT_TRUE(command_.changed_groups().empty());
- command_.ExecuteImpl(session());
- EXPECT_EQ(command_.GetGroupsToChangeForTest(*session()),
- command_.changed_groups());
-}
-
-} // namespace
-
-} // namespace browser_sync
diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc
index a4c3291..83e0d30 100644
--- a/chrome/browser/sync/engine/process_commit_response_command.cc
+++ b/chrome/browser/sync/engine/process_commit_response_command.cc
@@ -4,7 +4,6 @@
#include "chrome/browser/sync/engine/process_commit_response_command.h"
-#include <cstddef>
#include <set>
#include <string>
#include <vector>
@@ -60,27 +59,6 @@ void ResetErrorCounters(StatusController* status) {
ProcessCommitResponseCommand::ProcessCommitResponseCommand() {}
ProcessCommitResponseCommand::~ProcessCommitResponseCommand() {}
-std::set<ModelSafeGroup> ProcessCommitResponseCommand::GetGroupsToChange(
- const sessions::SyncSession& session) const {
- std::set<ModelSafeGroup> groups_with_commits;
- syncable::ScopedDirLookup dir(session.context()->directory_manager(),
- session.context()->account_name());
- if (!dir.good()) {
- LOG(ERROR) << "Scoped dir lookup failed!";
- return groups_with_commits;
- }
-
- syncable::ReadTransaction trans(FROM_HERE, dir);
- const StatusController& status = session.status_controller();
- for (size_t i = 0; i < status.commit_ids().size(); ++i) {
- groups_with_commits.insert(
- GetGroupForModelType(status.GetUnrestrictedCommitModelTypeAt(i),
- session.routing_info()));
- }
-
- return groups_with_commits;
-}
-
bool ProcessCommitResponseCommand::ModelNeutralExecuteImpl(
sessions::SyncSession* session) {
ScopedDirLookup dir(session->context()->directory_manager(),
diff --git a/chrome/browser/sync/engine/process_commit_response_command.h b/chrome/browser/sync/engine/process_commit_response_command.h
index bac2a31..dd13806 100644
--- a/chrome/browser/sync/engine/process_commit_response_command.h
+++ b/chrome/browser/sync/engine/process_commit_response_command.h
@@ -28,12 +28,8 @@ class ProcessCommitResponseCommand : public ModelChangingSyncerCommand {
ProcessCommitResponseCommand();
virtual ~ProcessCommitResponseCommand();
- protected:
// ModelChangingSyncerCommand implementation.
- virtual std::set<ModelSafeGroup> GetGroupsToChange(
- const sessions::SyncSession& session) const OVERRIDE;
- virtual bool ModelNeutralExecuteImpl(
- sessions::SyncSession* session) OVERRIDE;
+ virtual bool ModelNeutralExecuteImpl(sessions::SyncSession* session) OVERRIDE;
virtual void ModelChangingExecuteImpl(
sessions::SyncSession* session) OVERRIDE;
diff --git a/chrome/browser/sync/engine/process_commit_response_command_unittest.cc b/chrome/browser/sync/engine/process_commit_response_command_unittest.cc
index b22963c..83b8885 100644
--- a/chrome/browser/sync/engine/process_commit_response_command_unittest.cc
+++ b/chrome/browser/sync/engine/process_commit_response_command_unittest.cc
@@ -53,9 +53,6 @@ class ProcessCommitResponseCommandTestWithParam
commit_set_.reset(new sessions::OrderedCommitSet(routing_info()));
SyncerCommandTestWithParam<T>::SetUp();
- // Need to explicitly use this-> to avoid obscure template
- // warning.
- this->ExpectNoGroupsToChange(command_);
}
protected:
@@ -236,7 +233,6 @@ TEST_F(ProcessCommitResponseCommandTest, MultipleCommitIdProjections) {
CreateUnprocessedCommitResult(autofill_id2, id_factory_.root(),
"Autofill 2", syncable::AUTOFILL);
- ExpectGroupsToChange(command_, GROUP_UI, GROUP_DB);
command_.ExecuteImpl(session());
ScopedDirLookup dir(syncdb()->manager(), syncdb()->name());
@@ -328,7 +324,6 @@ TEST_F(ProcessCommitResponseCommandTest, NewFolderCommitKeepsChildOrder) {
// 25 items. This should apply the values indicated by
// each CommitResponse_EntryResponse to the syncable Entries. All new
// items in the commit batch should have their IDs changed to server IDs.
- ExpectGroupToChange(command_, GROUP_UI);
command_.ExecuteImpl(session());
ScopedDirLookup dir(syncdb()->manager(), syncdb()->name());
@@ -433,7 +428,6 @@ TEST_P(MixedResult, ExtensionActivity) {
(*records)["xyz"].extension_id = "xyz";
(*records)["xyz"].bookmark_write_count = 4U;
}
- ExpectGroupsToChange(command_, GROUP_UI, GROUP_DB);
command_.ExecuteImpl(session());
ExtensionsActivityMonitor::Records final_monitor_records;
diff --git a/chrome/browser/sync/engine/process_updates_command.cc b/chrome/browser/sync/engine/process_updates_command.cc
index 08cba00..e6763bd 100644
--- a/chrome/browser/sync/engine/process_updates_command.cc
+++ b/chrome/browser/sync/engine/process_updates_command.cc
@@ -22,20 +22,10 @@ namespace browser_sync {
using sessions::SyncSession;
using sessions::StatusController;
-using sessions::UpdateProgress;
ProcessUpdatesCommand::ProcessUpdatesCommand() {}
ProcessUpdatesCommand::~ProcessUpdatesCommand() {}
-std::set<ModelSafeGroup> ProcessUpdatesCommand::GetGroupsToChange(
- const sessions::SyncSession& session) const {
- // TODO(akalin): Figure out why using
- // GetEnabledGroupsWithVerifiedUpdates() regresses the perf tests
- // for delete_typed_urls, add_autofill_profiles, and
- // delete_autofill_profiles.
- return session.GetEnabledGroups();
-}
-
bool ProcessUpdatesCommand::ModelNeutralExecuteImpl(SyncSession* session) {
const GetUpdatesResponse& updates =
session->status_controller().updates_response().get_updates();
diff --git a/chrome/browser/sync/engine/process_updates_command.h b/chrome/browser/sync/engine/process_updates_command.h
index 1f12706..26e4993 100644
--- a/chrome/browser/sync/engine/process_updates_command.h
+++ b/chrome/browser/sync/engine/process_updates_command.h
@@ -34,12 +34,8 @@ class ProcessUpdatesCommand : public ModelChangingSyncerCommand {
ProcessUpdatesCommand();
virtual ~ProcessUpdatesCommand();
- protected:
// ModelChangingSyncerCommand implementation.
- virtual std::set<ModelSafeGroup> GetGroupsToChange(
- const sessions::SyncSession& session) const OVERRIDE;
- virtual bool ModelNeutralExecuteImpl(
- sessions::SyncSession* session) OVERRIDE;
+ virtual bool ModelNeutralExecuteImpl(sessions::SyncSession* session) OVERRIDE;
virtual void ModelChangingExecuteImpl(
sessions::SyncSession* session) OVERRIDE;
diff --git a/chrome/browser/sync/engine/process_updates_command_unittest.cc b/chrome/browser/sync/engine/process_updates_command_unittest.cc
deleted file mode 100644
index e421b3a..0000000
--- a/chrome/browser/sync/engine/process_updates_command_unittest.cc
+++ /dev/null
@@ -1,59 +0,0 @@
-// Copyright (c) 2011 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 "base/basictypes.h"
-#include "base/memory/ref_counted.h"
-#include "chrome/browser/sync/engine/process_updates_command.h"
-#include "chrome/browser/sync/sessions/session_state.h"
-#include "chrome/browser/sync/sessions/sync_session.h"
-#include "chrome/browser/sync/syncable/model_type.h"
-#include "chrome/browser/sync/syncable/syncable_id.h"
-#include "chrome/browser/sync/test/engine/fake_model_worker.h"
-#include "chrome/browser/sync/test/engine/syncer_command_test.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace browser_sync {
-
-namespace {
-
-class ProcessUpdatesCommandTest : public SyncerCommandTest {
- protected:
- ProcessUpdatesCommandTest() {}
- virtual ~ProcessUpdatesCommandTest() {}
-
- virtual void SetUp() {
- workers()->push_back(
- make_scoped_refptr(new FakeModelWorker(GROUP_UI)));
- workers()->push_back(
- make_scoped_refptr(new FakeModelWorker(GROUP_DB)));
- (*mutable_routing_info())[syncable::BOOKMARKS] = GROUP_UI;
- (*mutable_routing_info())[syncable::AUTOFILL] = GROUP_DB;
- SyncerCommandTest::SetUp();
- }
-
- ProcessUpdatesCommand command_;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(ProcessUpdatesCommandTest);
-};
-
-TEST_F(ProcessUpdatesCommandTest, GetGroupsToChange) {
- // TODO(akalin): Expect no groups to change here after we change
- // ProcessUpdatesCommand back to use
- // GetEnabledGroupsWithVerifiedUpdates().
-
- // Add a verified update for GROUP_DB.
- session()->mutable_status_controller()->
- GetUnrestrictedMutableUpdateProgressForTest(GROUP_DB)->
- AddVerifyResult(VerifyResult(), sync_pb::SyncEntity());
-
- // TODO(akalin): Change this to just expect GROUP_DB after we change
- // ProcessUpdatesCommand back to use
- // GetEnabledGroupsWithVerifiedUpdates().
- ExpectGroupsToChange(command_, GROUP_DB, GROUP_UI, GROUP_PASSIVE);
-}
-
-} // namespace
-
-} // namespace browser_sync
diff --git a/chrome/browser/sync/engine/resolve_conflicts_command.cc b/chrome/browser/sync/engine/resolve_conflicts_command.cc
index b55f9ad..71351c4 100644
--- a/chrome/browser/sync/engine/resolve_conflicts_command.cc
+++ b/chrome/browser/sync/engine/resolve_conflicts_command.cc
@@ -14,11 +14,6 @@ namespace browser_sync {
ResolveConflictsCommand::ResolveConflictsCommand() {}
ResolveConflictsCommand::~ResolveConflictsCommand() {}
-std::set<ModelSafeGroup> ResolveConflictsCommand::GetGroupsToChange(
- const sessions::SyncSession& session) const {
- return session.GetEnabledGroupsWithConflicts();
-}
-
void ResolveConflictsCommand::ModelChangingExecuteImpl(
sessions::SyncSession* session) {
ConflictResolver* resolver = session->context()->resolver();
diff --git a/chrome/browser/sync/engine/resolve_conflicts_command.h b/chrome/browser/sync/engine/resolve_conflicts_command.h
index 2407593..1fcdec9 100644
--- a/chrome/browser/sync/engine/resolve_conflicts_command.h
+++ b/chrome/browser/sync/engine/resolve_conflicts_command.h
@@ -17,10 +17,7 @@ class ResolveConflictsCommand : public ModelChangingSyncerCommand {
ResolveConflictsCommand();
virtual ~ResolveConflictsCommand();
- protected:
// ModelChangingSyncerCommand implementation.
- virtual std::set<ModelSafeGroup> GetGroupsToChange(
- const sessions::SyncSession& session) const OVERRIDE;
virtual void ModelChangingExecuteImpl(
sessions::SyncSession* session) OVERRIDE;
diff --git a/chrome/browser/sync/engine/resolve_conflicts_command_unittest.cc b/chrome/browser/sync/engine/resolve_conflicts_command_unittest.cc
deleted file mode 100644
index a36d5e2..0000000
--- a/chrome/browser/sync/engine/resolve_conflicts_command_unittest.cc
+++ /dev/null
@@ -1,51 +0,0 @@
-// Copyright (c) 2011 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 "base/basictypes.h"
-#include "base/memory/ref_counted.h"
-#include "chrome/browser/sync/engine/resolve_conflicts_command.h"
-#include "chrome/browser/sync/sessions/sync_session.h"
-#include "chrome/browser/sync/syncable/model_type.h"
-#include "chrome/browser/sync/syncable/syncable_id.h"
-#include "chrome/browser/sync/test/engine/fake_model_worker.h"
-#include "chrome/browser/sync/test/engine/syncer_command_test.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace browser_sync {
-
-namespace {
-
-class ResolveConflictsCommandTest : public SyncerCommandTest {
- protected:
- ResolveConflictsCommandTest() {}
- virtual ~ResolveConflictsCommandTest() {}
-
- virtual void SetUp() {
- workers()->push_back(
- make_scoped_refptr(new FakeModelWorker(GROUP_UI)));
- workers()->push_back(
- make_scoped_refptr(new FakeModelWorker(GROUP_PASSWORD)));
- (*mutable_routing_info())[syncable::BOOKMARKS] = GROUP_UI;
- (*mutable_routing_info())[syncable::PASSWORDS] = GROUP_PASSWORD;
- SyncerCommandTest::SetUp();
- }
-
- ResolveConflictsCommand command_;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(ResolveConflictsCommandTest);
-};
-
-TEST_F(ResolveConflictsCommandTest, GetGroupsToChange) {
- ExpectNoGroupsToChange(command_);
- // Put GROUP_PASSWORD in conflict.
- session()->mutable_status_controller()->
- GetUnrestrictedMutableConflictProgressForTest(GROUP_PASSWORD)->
- AddConflictingItemById(syncable::Id());
- ExpectGroupToChange(command_, GROUP_PASSWORD);
-}
-
-} // namespace
-
-} // namespace browser_sync
diff --git a/chrome/browser/sync/engine/sync_scheduler.cc b/chrome/browser/sync/engine/sync_scheduler.cc
index c21749b..ce05fb1 100644
--- a/chrome/browser/sync/engine/sync_scheduler.cc
+++ b/chrome/browser/sync/engine/sync_scheduler.cc
@@ -762,7 +762,7 @@ void SyncScheduler::DoSyncSessionJob(const SyncSessionJob& job) {
// and update any disabled or modified entries in the job.
scoped_ptr<SyncSession> session(CreateSyncSession(job.session->source()));
- job.session->RebaseRoutingInfoWithLatest(*session);
+ job.session->RebaseRoutingInfoWithLatest(session.get());
}
SDVLOG(2) << "DoSyncSessionJob with "
<< SyncSessionJob::GetPurposeString(job.purpose) << " job";
diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc
index 50c30fc..582655e 100644
--- a/chrome/browser/sync/engine/syncer_unittest.cc
+++ b/chrome/browser/sync/engine/syncer_unittest.cc
@@ -2162,8 +2162,6 @@ TEST_F(SyncerTest, CommitsUpdateDoesntAlterEntry) {
}
TEST_F(SyncerTest, ParentAndChildBothMatch) {
- syncable::ModelTypeBitSet all_types;
- all_types.set();
ScopedDirLookup dir(syncdb_.manager(), syncdb_.name());
CHECK(dir.good());
syncable::Id parent_id = ids_.NewServerId();
@@ -2200,7 +2198,7 @@ TEST_F(SyncerTest, ParentAndChildBothMatch) {
dir->GetChildHandlesById(&trans, parent_id, &children);
EXPECT_EQ(1u, children.size());
Directory::UnappliedUpdateMetaHandles unapplied;
- dir->GetUnappliedUpdateMetaHandles(&trans, all_types, &unapplied);
+ dir->GetUnappliedUpdateMetaHandles(&trans, &unapplied);
EXPECT_EQ(0u, unapplied.size());
syncable::Directory::UnsyncedMetaHandles unsynced;
dir->GetUnsyncedMetaHandles(&trans, &unsynced);
diff --git a/chrome/browser/sync/engine/update_applicator.cc b/chrome/browser/sync/engine/update_applicator.cc
index e44083c..6246836 100644
--- a/chrome/browser/sync/engine/update_applicator.cc
+++ b/chrome/browser/sync/engine/update_applicator.cc
@@ -97,12 +97,12 @@ void UpdateApplicator::Advance() {
bool UpdateApplicator::SkipUpdate(const syncable::Entry& entry) {
syncable::ModelType type = entry.GetServerModelType();
ModelSafeGroup g = GetGroupForModelType(type, routing_info_);
- // The set of updates passed to the UpdateApplicator should already
- // be group-filtered.
- if (g != group_filter_) {
- NOTREACHED();
+ // The extra routing_info count check here is to support GetUpdateses for
+ // a subset of the globally enabled types, and not attempt to update items
+ // if their type isn't permitted in the current run. These would typically
+ // be unapplied items from a previous sync.
+ if (g != group_filter_)
return true;
- }
if (g == GROUP_PASSIVE &&
!routing_info_.count(type) &&
type != syncable::UNSPECIFIED &&
diff --git a/chrome/browser/sync/engine/verify_updates_command.cc b/chrome/browser/sync/engine/verify_updates_command.cc
index 96dd4e7..81f1de2 100644
--- a/chrome/browser/sync/engine/verify_updates_command.cc
+++ b/chrome/browser/sync/engine/verify_updates_command.cc
@@ -27,21 +27,6 @@ using syncable::SYNCER;
VerifyUpdatesCommand::VerifyUpdatesCommand() {}
VerifyUpdatesCommand::~VerifyUpdatesCommand() {}
-std::set<ModelSafeGroup> VerifyUpdatesCommand::GetGroupsToChange(
- const sessions::SyncSession& session) const {
- std::set<ModelSafeGroup> groups_with_updates;
-
- const GetUpdatesResponse& updates =
- session.status_controller().updates_response().get_updates();
- for (int i = 0; i < updates.entries().size(); i++) {
- groups_with_updates.insert(
- GetGroupForModelType(syncable::GetModelType(updates.entries(i)),
- session.routing_info()));
- }
-
- return groups_with_updates;
-}
-
void VerifyUpdatesCommand::ModelChangingExecuteImpl(
sessions::SyncSession* session) {
DVLOG(1) << "Beginning Update Verification";
diff --git a/chrome/browser/sync/engine/verify_updates_command.h b/chrome/browser/sync/engine/verify_updates_command.h
index 26b0b37..b612e18 100644
--- a/chrome/browser/sync/engine/verify_updates_command.h
+++ b/chrome/browser/sync/engine/verify_updates_command.h
@@ -26,10 +26,7 @@ class VerifyUpdatesCommand : public ModelChangingSyncerCommand {
VerifyUpdatesCommand();
virtual ~VerifyUpdatesCommand();
- protected:
- // ModelChangingSyncerCommand implementation.
- virtual std::set<ModelSafeGroup> GetGroupsToChange(
- const sessions::SyncSession& session) const OVERRIDE;
+ // SyncerCommand implementation.
virtual void ModelChangingExecuteImpl(
sessions::SyncSession* session) OVERRIDE;
diff --git a/chrome/browser/sync/engine/verify_updates_command_unittest.cc b/chrome/browser/sync/engine/verify_updates_command_unittest.cc
index 7de5cf2..20aab01 100644
--- a/chrome/browser/sync/engine/verify_updates_command_unittest.cc
+++ b/chrome/browser/sync/engine/verify_updates_command_unittest.cc
@@ -5,7 +5,6 @@
#include "base/location.h"
#include "chrome/browser/sync/engine/verify_updates_command.h"
#include "chrome/browser/sync/protocol/bookmark_specifics.pb.h"
-#include "chrome/browser/sync/sessions/session_state.h"
#include "chrome/browser/sync/sessions/sync_session.h"
#include "chrome/browser/sync/syncable/directory_manager.h"
#include "chrome/browser/sync/syncable/syncable.h"
@@ -82,8 +81,6 @@ TEST_F(VerifyUpdatesCommandTest, AllVerified) {
CreateLocalItem("p1", root, syncable::PREFERENCES);
CreateLocalItem("a1", root, syncable::AUTOFILL);
- ExpectNoGroupsToChange(command_);
-
GetUpdatesResponse* updates = session()->mutable_status_controller()->
mutable_updates_response()->mutable_get_updates();
AddUpdate(updates, "b1", root, syncable::BOOKMARKS);
@@ -91,8 +88,6 @@ TEST_F(VerifyUpdatesCommandTest, AllVerified) {
AddUpdate(updates, "p1", root, syncable::PREFERENCES);
AddUpdate(updates, "a1", root, syncable::AUTOFILL);
- ExpectGroupsToChange(command_, GROUP_UI, GROUP_DB);
-
command_.ExecuteImpl(session());
StatusController* status = session()->mutable_status_controller();
diff --git a/chrome/browser/sync/sessions/status_controller.cc b/chrome/browser/sync/sessions/status_controller.cc
index 4b82aa8..26f5c62 100644
--- a/chrome/browser/sync/sessions/status_controller.cc
+++ b/chrome/browser/sync/sessions/status_controller.cc
@@ -62,12 +62,6 @@ const ConflictProgress* StatusController::GetUnrestrictedConflictProgress(
return state ? &state->conflict_progress : NULL;
}
-ConflictProgress*
- StatusController::GetUnrestrictedMutableConflictProgressForTest(
- ModelSafeGroup group) {
- return &GetOrCreateModelSafeGroupState(false, group)->conflict_progress;
-}
-
const UpdateProgress* StatusController::GetUnrestrictedUpdateProgress(
ModelSafeGroup group) const {
const PerModelSafeGroupState* state =
@@ -75,12 +69,6 @@ const UpdateProgress* StatusController::GetUnrestrictedUpdateProgress(
return state ? &state->update_progress : NULL;
}
-UpdateProgress*
- StatusController::GetUnrestrictedMutableUpdateProgressForTest(
- ModelSafeGroup group) {
- return &GetOrCreateModelSafeGroupState(false, group)->update_progress;
-}
-
const PerModelSafeGroupState* StatusController::GetModelSafeGroupState(
bool restrict, ModelSafeGroup group) const {
DCHECK_EQ(restrict, group_restriction_in_effect_);
diff --git a/chrome/browser/sync/sessions/status_controller.h b/chrome/browser/sync/sessions/status_controller.h
index 70aed96..c0782cb 100644
--- a/chrome/browser/sync/sessions/status_controller.h
+++ b/chrome/browser/sync/sessions/status_controller.h
@@ -63,12 +63,8 @@ class StatusController {
UpdateProgress* mutable_update_progress();
const ConflictProgress* GetUnrestrictedConflictProgress(
ModelSafeGroup group) const;
- ConflictProgress* GetUnrestrictedMutableConflictProgressForTest(
- ModelSafeGroup group);
const UpdateProgress* GetUnrestrictedUpdateProgress(
ModelSafeGroup group) const;
- UpdateProgress* GetUnrestrictedMutableUpdateProgressForTest(
- ModelSafeGroup group);
// ClientToServer messages.
const ClientToServerMessage& commit_message() {
diff --git a/chrome/browser/sync/sessions/sync_session.cc b/chrome/browser/sync/sessions/sync_session.cc
index 94bcd85..f399d7d 100644
--- a/chrome/browser/sync/sessions/sync_session.cc
+++ b/chrome/browser/sync/sessions/sync_session.cc
@@ -6,48 +6,12 @@
#include <algorithm>
-#include "base/logging.h"
#include "chrome/browser/sync/syncable/directory_manager.h"
#include "chrome/browser/sync/syncable/model_type.h"
namespace browser_sync {
namespace sessions {
-namespace {
-
-std::set<ModelSafeGroup> ComputeEnabledGroups(
- const ModelSafeRoutingInfo& routing_info,
- const std::vector<ModelSafeWorker*>& workers) {
- std::set<ModelSafeGroup> enabled_groups;
- // Project the list of enabled types (i.e., types in the routing
- // info) to a list of enabled groups.
- for (ModelSafeRoutingInfo::const_iterator it = routing_info.begin();
- it != routing_info.end(); ++it) {
- enabled_groups.insert(it->second);
- }
- // GROUP_PASSIVE is always enabled, since that's the group that
- // top-level folders map to.
- enabled_groups.insert(GROUP_PASSIVE);
- if (DCHECK_IS_ON()) {
- // We sometimes create dummy SyncSession objects (see
- // SyncScheduler::InitialSnapshot) so don't check in that case.
- if (!routing_info.empty() || !workers.empty()) {
- std::set<ModelSafeGroup> groups_with_workers;
- for (std::vector<ModelSafeWorker*>::const_iterator it = workers.begin();
- it != workers.end(); ++it) {
- groups_with_workers.insert((*it)->GetModelSafeGroup());
- }
- // All enabled groups should have a corresponding worker.
- DCHECK(std::includes(
- groups_with_workers.begin(), groups_with_workers.end(),
- enabled_groups.begin(), enabled_groups.end()));
- }
- }
- return enabled_groups;
-}
-
-} // namesepace
-
SyncSession::SyncSession(SyncSessionContext* context, Delegate* delegate,
const SyncSourceInfo& source,
const ModelSafeRoutingInfo& routing_info,
@@ -57,8 +21,7 @@ SyncSession::SyncSession(SyncSessionContext* context, Delegate* delegate,
write_transaction_(NULL),
delegate_(delegate),
workers_(workers),
- routing_info_(routing_info),
- enabled_groups_(ComputeEnabledGroups(routing_info_, workers_)) {
+ routing_info_(routing_info) {
status_controller_.reset(new StatusController(routing_info_));
std::sort(workers_.begin(), workers_.end());
}
@@ -90,18 +53,15 @@ void SyncSession::Coalesce(const SyncSession& session) {
++it) {
routing_info_[it->first] = it->second;
}
-
- // Now update enabled groups.
- enabled_groups_ = ComputeEnabledGroups(routing_info_, workers_);
}
-void SyncSession::RebaseRoutingInfoWithLatest(const SyncSession& session) {
+void SyncSession::RebaseRoutingInfoWithLatest(SyncSession* session) {
ModelSafeRoutingInfo temp_routing_info;
// Take the intersecion and also set the routing info(it->second) from the
// passed in session.
for (ModelSafeRoutingInfo::const_iterator it =
- session.routing_info_.begin(); it != session.routing_info_.end();
+ session->routing_info_.begin(); it != session->routing_info_.end();
++it) {
if (routing_info_.find(it->first) != routing_info_.end()) {
temp_routing_info[it->first] = it->second;
@@ -112,17 +72,14 @@ void SyncSession::RebaseRoutingInfoWithLatest(const SyncSession& session) {
routing_info_.swap(temp_routing_info);
// Now update the payload map.
- PurgeStalePayload(&source_.types, session.routing_info_);
+ PurgeStalePayload(&source_.types, session->routing_info_);
// Now update the workers.
std::vector<ModelSafeWorker*> temp;
std::set_intersection(workers_.begin(), workers_.end(),
- session.workers_.begin(), session.workers_.end(),
+ session->workers_.begin(), session->workers_.end(),
std::back_inserter(temp));
workers_.swap(temp);
-
- // Now update enabled groups.
- enabled_groups_ = ComputeEnabledGroups(routing_info_, workers_);
}
void SyncSession::ResetTransientState() {
@@ -184,46 +141,7 @@ bool SyncSession::HasMoreToSync() const {
status->conflicts_resolved();
// Or, we have conflicting updates, but we're making progress on
// resolving them...
-}
-
-const std::set<ModelSafeGroup>& SyncSession::GetEnabledGroups() const {
- return enabled_groups_;
-}
-
-std::set<ModelSafeGroup> SyncSession::GetEnabledGroupsWithConflicts() const {
- const std::set<ModelSafeGroup>& enabled_groups = GetEnabledGroups();
- std::set<ModelSafeGroup> enabled_groups_with_conflicts;
- for (std::set<ModelSafeGroup>::const_iterator it =
- enabled_groups.begin(); it != enabled_groups.end(); ++it) {
- const sessions::ConflictProgress* conflict_progress =
- status_controller_->GetUnrestrictedConflictProgress(*it);
- if (conflict_progress &&
- (conflict_progress->ConflictingItemsBegin() !=
- conflict_progress->ConflictingItemsEnd())) {
- enabled_groups_with_conflicts.insert(*it);
- }
}
- return enabled_groups_with_conflicts;
-}
-
-std::set<ModelSafeGroup>
- SyncSession::GetEnabledGroupsWithVerifiedUpdates() const {
- const std::set<ModelSafeGroup>& enabled_groups = GetEnabledGroups();
- std::set<ModelSafeGroup> enabled_groups_with_verified_updates;
- for (std::set<ModelSafeGroup>::const_iterator it =
- enabled_groups.begin(); it != enabled_groups.end(); ++it) {
- const UpdateProgress* update_progress =
- status_controller_->GetUnrestrictedUpdateProgress(*it);
- if (update_progress &&
- (update_progress->VerifiedUpdatesBegin() !=
- update_progress->VerifiedUpdatesEnd())) {
- enabled_groups_with_verified_updates.insert(*it);
- }
- }
-
- return enabled_groups_with_verified_updates;
-}
-
} // namespace sessions
} // namespace browser_sync
diff --git a/chrome/browser/sync/sessions/sync_session.h b/chrome/browser/sync/sessions/sync_session.h
index 135e6d7..2762057 100644
--- a/chrome/browser/sync/sessions/sync_session.h
+++ b/chrome/browser/sync/sessions/sync_session.h
@@ -16,7 +16,6 @@
#pragma once
#include <map>
-#include <set>
#include <string>
#include <utility>
#include <vector>
@@ -120,7 +119,7 @@ class SyncSession {
// session. Purges types from the above 3 which are not in session. Useful
// to update the sync session when the user has disabled some types from
// syncing.
- void RebaseRoutingInfoWithLatest(const SyncSession& session);
+ void RebaseRoutingInfoWithLatest(SyncSession* session);
// Should be called any time |this| is being re-used in a new call to
// SyncShare (e.g., HasMoreToSync returned true).
@@ -153,15 +152,6 @@ class SyncSession {
const ModelSafeRoutingInfo& routing_info() const { return routing_info_; }
const SyncSourceInfo& source() const { return source_; }
- // Returns the set of groups which have enabled types.
- const std::set<ModelSafeGroup>& GetEnabledGroups() const;
-
- // Returns the set of enabled groups that have conflicts.
- std::set<ModelSafeGroup> GetEnabledGroupsWithConflicts() const;
-
- // Returns the set of enabled groups that have verified updates.
- std::set<ModelSafeGroup> GetEnabledGroupsWithVerifiedUpdates() const;
-
private:
// Extend the encapsulation boundary to utilities for internal member
// assignments. This way, the scope of these actions is explicit, they can't
@@ -181,7 +171,7 @@ class SyncSession {
syncable::WriteTransaction* write_transaction_;
// The delegate for this session, must never be NULL.
- Delegate* const delegate_;
+ Delegate* delegate_;
// Our controller for various status and error counters.
scoped_ptr<StatusController> status_controller_;
@@ -195,10 +185,6 @@ class SyncSession {
// on those datatypes.
ModelSafeRoutingInfo routing_info_;
- // The set of groups with enabled types. Computed from
- // |routing_info_|.
- std::set<ModelSafeGroup> enabled_groups_;
-
DISALLOW_COPY_AND_ASSIGN(SyncSession);
};
diff --git a/chrome/browser/sync/sessions/sync_session_unittest.cc b/chrome/browser/sync/sessions/sync_session_unittest.cc
index 64c2cec..d914b76 100644
--- a/chrome/browser/sync/sessions/sync_session_unittest.cc
+++ b/chrome/browser/sync/sessions/sync_session_unittest.cc
@@ -10,12 +10,9 @@
#include "base/message_loop.h"
#include "chrome/browser/sync/engine/conflict_resolver.h"
#include "chrome/browser/sync/engine/syncer_types.h"
-#include "chrome/browser/sync/sessions/session_state.h"
-#include "chrome/browser/sync/sessions/status_controller.h"
#include "chrome/browser/sync/syncable/directory_manager.h"
#include "chrome/browser/sync/syncable/model_type.h"
#include "chrome/browser/sync/syncable/syncable.h"
-#include "chrome/browser/sync/syncable/syncable_id.h"
#include "chrome/browser/sync/test/engine/fake_model_worker.h"
#include "chrome/browser/sync/test/engine/test_directory_setter_upper.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -30,13 +27,13 @@ class SyncSessionTest : public testing::Test,
public SyncSession::Delegate,
public ModelSafeWorkerRegistrar {
public:
- SyncSessionTest() : controller_invocations_allowed_(false) {}
+ SyncSessionTest() : controller_invocations_allowed_(false) {
+ GetModelSafeRoutingInfo(&routes_);
+ }
SyncSession* MakeSession() {
- std::vector<ModelSafeWorker*> workers;
- GetWorkers(&workers);
- return new SyncSession(context_.get(), this, SyncSourceInfo(),
- routes_, workers);
+ return new SyncSession(context_.get(), this, SyncSourceInfo(), routes_,
+ std::vector<ModelSafeWorker*>());
}
virtual void SetUp() {
@@ -44,17 +41,7 @@ class SyncSessionTest : public testing::Test,
std::vector<SyncEngineEventListener*>(), NULL));
routes_.clear();
routes_[syncable::BOOKMARKS] = GROUP_UI;
- routes_[syncable::AUTOFILL] = GROUP_DB;
- scoped_refptr<ModelSafeWorker> passive_worker(
- new FakeModelWorker(GROUP_PASSIVE));
- scoped_refptr<ModelSafeWorker> ui_worker(
- new FakeModelWorker(GROUP_UI));
- scoped_refptr<ModelSafeWorker> db_worker(
- new FakeModelWorker(GROUP_DB));
- workers_.clear();
- workers_.push_back(passive_worker);
- workers_.push_back(ui_worker);
- workers_.push_back(db_worker);
+ routes_[syncable::AUTOFILL] = GROUP_UI;
session_.reset(MakeSession());
}
virtual void TearDown() {
@@ -90,15 +77,9 @@ class SyncSessionTest : public testing::Test,
}
// ModelSafeWorkerRegistrar implementation.
- virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) OVERRIDE {
- out->clear();
- for (std::vector<scoped_refptr<ModelSafeWorker> >::const_iterator it =
- workers_.begin(); it != workers_.end(); ++it) {
- out->push_back(it->get());
- }
- }
+ virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) OVERRIDE {}
virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) OVERRIDE {
- *out = routes_;
+ out->swap(routes_);
}
StatusController* status() { return session_->mutable_status_controller(); }
@@ -125,49 +106,9 @@ class SyncSessionTest : public testing::Test,
bool controller_invocations_allowed_;
scoped_ptr<SyncSession> session_;
scoped_ptr<SyncSessionContext> context_;
- std::vector<scoped_refptr<ModelSafeWorker> > workers_;
ModelSafeRoutingInfo routes_;
};
-TEST_F(SyncSessionTest, EnabledGroupsEmpty) {
- routes_.clear();
- workers_.clear();
- scoped_ptr<SyncSession> session(MakeSession());
- std::set<ModelSafeGroup> expected_enabled_groups;
- expected_enabled_groups.insert(GROUP_PASSIVE);
- EXPECT_EQ(expected_enabled_groups, session->GetEnabledGroups());
-}
-
-TEST_F(SyncSessionTest, EnabledGroups) {
- scoped_ptr<SyncSession> session(MakeSession());
- std::set<ModelSafeGroup> expected_enabled_groups;
- expected_enabled_groups.insert(GROUP_PASSIVE);
- expected_enabled_groups.insert(GROUP_UI);
- expected_enabled_groups.insert(GROUP_DB);
- EXPECT_EQ(expected_enabled_groups, session->GetEnabledGroups());
-}
-
-TEST_F(SyncSessionTest, EnabledGroupsWithConflictsEmpty) {
- scoped_ptr<SyncSession> session(MakeSession());
- // Auto-create conflict progress. This shouldn't put that group in
- // conflict.
- session->mutable_status_controller()->
- GetUnrestrictedMutableConflictProgressForTest(GROUP_PASSIVE);
- EXPECT_TRUE(session->GetEnabledGroupsWithConflicts().empty());
-}
-
-TEST_F(SyncSessionTest, EnabledGroupsWithConflicts) {
- scoped_ptr<SyncSession> session(MakeSession());
- // Put GROUP_UI in conflict.
- session->mutable_status_controller()->
- GetUnrestrictedMutableConflictProgressForTest(GROUP_UI)->
- AddConflictingItemById(syncable::Id());
- std::set<ModelSafeGroup> expected_enabled_groups_with_conflicts;
- expected_enabled_groups_with_conflicts.insert(GROUP_UI);
- EXPECT_EQ(expected_enabled_groups_with_conflicts,
- session->GetEnabledGroupsWithConflicts());
-}
-
TEST_F(SyncSessionTest, ScopedContextHelpers) {
ConflictResolver resolver;
EXPECT_FALSE(context_->resolver());
@@ -347,13 +288,9 @@ TEST_F(SyncSessionTest, Coalesce) {
SyncSourceInfo source_one(sync_pb::GetUpdatesCallerInfo::PERIODIC, one_type);
SyncSourceInfo source_two(sync_pb::GetUpdatesCallerInfo::LOCAL, all_types);
- scoped_refptr<ModelSafeWorker> passive_worker(
- new FakeModelWorker(GROUP_PASSIVE));
scoped_refptr<ModelSafeWorker> db_worker(new FakeModelWorker(GROUP_DB));
scoped_refptr<ModelSafeWorker> ui_worker(new FakeModelWorker(GROUP_UI));
- workers_one.push_back(passive_worker);
workers_one.push_back(db_worker);
- workers_two.push_back(passive_worker);
workers_two.push_back(db_worker);
workers_two.push_back(ui_worker);
routes_one[syncable::AUTOFILL] = GROUP_DB;
@@ -362,23 +299,8 @@ TEST_F(SyncSessionTest, Coalesce) {
SyncSession one(context_.get(), this, source_one, routes_one, workers_one);
SyncSession two(context_.get(), this, source_two, routes_two, workers_two);
- std::set<ModelSafeGroup> expected_enabled_groups_one;
- expected_enabled_groups_one.insert(GROUP_PASSIVE);
- expected_enabled_groups_one.insert(GROUP_DB);
-
- std::set<ModelSafeGroup> expected_enabled_groups_two;
- expected_enabled_groups_two.insert(GROUP_PASSIVE);
- expected_enabled_groups_two.insert(GROUP_DB);
- expected_enabled_groups_two.insert(GROUP_UI);
-
- EXPECT_EQ(expected_enabled_groups_one, one.GetEnabledGroups());
- EXPECT_EQ(expected_enabled_groups_two, two.GetEnabledGroups());
-
one.Coalesce(two);
- EXPECT_EQ(expected_enabled_groups_two, one.GetEnabledGroups());
- EXPECT_EQ(expected_enabled_groups_two, two.GetEnabledGroups());
-
EXPECT_EQ(two.source().updates_source, one.source().updates_source);
EXPECT_EQ(all_types, one.source().types);
std::vector<ModelSafeWorker*>::const_iterator it_db =
@@ -404,37 +326,18 @@ TEST_F(SyncSessionTest, RebaseRoutingInfoWithLatestRemoveOneType) {
SyncSourceInfo source_one(sync_pb::GetUpdatesCallerInfo::PERIODIC, one_type);
SyncSourceInfo source_two(sync_pb::GetUpdatesCallerInfo::LOCAL, all_types);
- scoped_refptr<ModelSafeWorker> passive_worker(
- new FakeModelWorker(GROUP_PASSIVE));
scoped_refptr<ModelSafeWorker> db_worker(new FakeModelWorker(GROUP_DB));
scoped_refptr<ModelSafeWorker> ui_worker(new FakeModelWorker(GROUP_UI));
- workers_one.push_back(passive_worker);
workers_one.push_back(db_worker);
- workers_two.push_back(passive_worker);
workers_two.push_back(db_worker);
workers_two.push_back(ui_worker);
routes_one[syncable::AUTOFILL] = GROUP_DB;
- routes_two[syncable::AUTOFILL] = GROUP_DB;
+ routes_two[syncable::AUTOFILL] = GROUP_UI;
routes_two[syncable::BOOKMARKS] = GROUP_UI;
SyncSession one(context_.get(), this, source_one, routes_one, workers_one);
SyncSession two(context_.get(), this, source_two, routes_two, workers_two);
- std::set<ModelSafeGroup> expected_enabled_groups_one;
- expected_enabled_groups_one.insert(GROUP_PASSIVE);
- expected_enabled_groups_one.insert(GROUP_DB);
-
- std::set<ModelSafeGroup> expected_enabled_groups_two;
- expected_enabled_groups_two.insert(GROUP_PASSIVE);
- expected_enabled_groups_two.insert(GROUP_DB);
- expected_enabled_groups_two.insert(GROUP_UI);
-
- EXPECT_EQ(expected_enabled_groups_one, one.GetEnabledGroups());
- EXPECT_EQ(expected_enabled_groups_two, two.GetEnabledGroups());
-
- two.RebaseRoutingInfoWithLatest(one);
-
- EXPECT_EQ(expected_enabled_groups_one, one.GetEnabledGroups());
- EXPECT_EQ(expected_enabled_groups_one, two.GetEnabledGroups());
+ two.RebaseRoutingInfoWithLatest(&one);
// Make sure the source has not been touched.
EXPECT_EQ(two.source().updates_source,
@@ -450,7 +353,7 @@ TEST_F(SyncSessionTest, RebaseRoutingInfoWithLatestRemoveOneType) {
std::find(two.workers().begin(), two.workers().end(), ui_worker);
EXPECT_NE(it_db, two.workers().end());
EXPECT_EQ(it_ui, two.workers().end());
- EXPECT_EQ(two.workers().size(), 2U);
+ EXPECT_EQ(two.workers().size(), 1U);
// Make sure the model safe routing info is reduced to one type.
ModelSafeRoutingInfo::const_iterator it =
@@ -472,14 +375,10 @@ TEST_F(SyncSessionTest, RebaseRoutingInfoWithLatestWithSameType) {
SyncSourceInfo source_second(sync_pb::GetUpdatesCallerInfo::LOCAL,
all_types);
- scoped_refptr<ModelSafeWorker> passive_worker(
- new FakeModelWorker(GROUP_PASSIVE));
scoped_refptr<FakeModelWorker> db_worker(new FakeModelWorker(GROUP_DB));
scoped_refptr<FakeModelWorker> ui_worker(new FakeModelWorker(GROUP_UI));
- workers_first.push_back(passive_worker);
workers_first.push_back(db_worker);
workers_first.push_back(ui_worker);
- workers_second.push_back(passive_worker);
workers_second.push_back(db_worker);
workers_second.push_back(ui_worker);
routes_first[syncable::AUTOFILL] = GROUP_DB;
@@ -491,18 +390,7 @@ TEST_F(SyncSessionTest, RebaseRoutingInfoWithLatestWithSameType) {
SyncSession second(context_.get(), this, source_second, routes_second,
workers_second);
- std::set<ModelSafeGroup> expected_enabled_groups;
- expected_enabled_groups.insert(GROUP_PASSIVE);
- expected_enabled_groups.insert(GROUP_DB);
- expected_enabled_groups.insert(GROUP_UI);
-
- EXPECT_EQ(expected_enabled_groups, first.GetEnabledGroups());
- EXPECT_EQ(expected_enabled_groups, second.GetEnabledGroups());
-
- second.RebaseRoutingInfoWithLatest(first);
-
- EXPECT_EQ(expected_enabled_groups, first.GetEnabledGroups());
- EXPECT_EQ(expected_enabled_groups, second.GetEnabledGroups());
+ second.RebaseRoutingInfoWithLatest(&first);
// Make sure the source has not been touched.
EXPECT_EQ(second.source().updates_source,
@@ -512,17 +400,13 @@ TEST_F(SyncSessionTest, RebaseRoutingInfoWithLatestWithSameType) {
EXPECT_EQ(all_types, second.source().types);
// Make sure the workers are still the same.
- std::vector<ModelSafeWorker*>::const_iterator it_passive =
- std::find(second.workers().begin(), second.workers().end(),
- passive_worker);
std::vector<ModelSafeWorker*>::const_iterator it_db =
std::find(second.workers().begin(), second.workers().end(), db_worker);
std::vector<ModelSafeWorker*>::const_iterator it_ui =
std::find(second.workers().begin(), second.workers().end(), ui_worker);
- EXPECT_NE(it_passive, second.workers().end());
EXPECT_NE(it_db, second.workers().end());
EXPECT_NE(it_ui, second.workers().end());
- EXPECT_EQ(second.workers().size(), 3U);
+ EXPECT_EQ(second.workers().size(), 2U);
// Make sure the model safe routing info is reduced to first type.
ModelSafeRoutingInfo::const_iterator it1 =
diff --git a/chrome/browser/sync/syncable/syncable.cc b/chrome/browser/sync/syncable/syncable.cc
index 6a931f3..0050fbf 100644
--- a/chrome/browser/sync/syncable/syncable.cc
+++ b/chrome/browser/sync/syncable/syncable.cc
@@ -258,20 +258,6 @@ EntryKernel::EntryKernel() : dirty_(false) {
EntryKernel::~EntryKernel() {}
-syncable::ModelType EntryKernel::GetServerModelType() const {
- ModelType specifics_type = GetModelTypeFromSpecifics(ref(SERVER_SPECIFICS));
- if (specifics_type != UNSPECIFIED)
- return specifics_type;
- if (ref(ID).IsRoot())
- return TOP_LEVEL_FOLDER;
- // Loose check for server-created top-level folders that aren't
- // bound to a particular model type.
- if (!ref(UNIQUE_SERVER_TAG).empty() && ref(SERVER_IS_DIR))
- return TOP_LEVEL_FOLDER;
-
- return UNSPECIFIED;
-}
-
bool EntryKernel::ContainsString(const std::string& lowercase_query) const {
// TODO(lipalani) - figure out what to do if the node is encrypted.
const sync_pb::EntitySpecifics& specifics = ref(SPECIFICS);
@@ -342,7 +328,6 @@ StringValue* IdToValue(const Id& id) {
DictionaryValue* EntryKernel::ToValue() const {
DictionaryValue* kernel_info = new DictionaryValue();
kernel_info->SetBoolean("isDirty", is_dirty());
- kernel_info->Set("serverModelType", ModelTypeToValue(GetServerModelType()));
// Int64 fields.
SetFieldValues(*this, kernel_info,
@@ -448,6 +433,7 @@ Directory::Kernel::Kernel(
ids_index(new Directory::IdsIndex),
parent_id_child_index(new Directory::ParentIdChildIndex),
client_tag_index(new Directory::ClientTagIndex),
+ unapplied_update_metahandles(new MetahandleSet),
unsynced_metahandles(new MetahandleSet),
dirty_metahandles(new MetahandleSet),
metahandles_to_purge(new MetahandleSet),
@@ -473,6 +459,7 @@ void Directory::Kernel::Release() {
Directory::Kernel::~Kernel() {
CHECK_EQ(0, refcount);
delete unsynced_metahandles;
+ delete unapplied_update_metahandles;
delete dirty_metahandles;
delete metahandles_to_purge;
delete parent_id_child_index;
@@ -509,13 +496,10 @@ void Directory::InitializeIndices() {
kernel_->parent_id_child_index);
InitializeIndexEntry<IdIndexer>(entry, kernel_->ids_index);
InitializeIndexEntry<ClientTagIndexer>(entry, kernel_->client_tag_index);
- const int64 metahandle = entry->ref(META_HANDLE);
if (entry->ref(IS_UNSYNCED))
- kernel_->unsynced_metahandles->insert(metahandle);
- if (entry->ref(IS_UNAPPLIED_UPDATE)) {
- const ModelType type = entry->GetServerModelType();
- kernel_->unapplied_update_metahandles[type].insert(metahandle);
- }
+ kernel_->unsynced_metahandles->insert(entry->ref(META_HANDLE));
+ if (entry->ref(IS_UNAPPLIED_UPDATE))
+ kernel_->unapplied_update_metahandles->insert(entry->ref(META_HANDLE));
DCHECK(!entry->is_dirty());
}
}
@@ -718,12 +702,11 @@ bool Directory::SafeToPurgeFromMemory(const EntryKernel* const entry) const {
!entry->ref(IS_UNSYNCED);
if (safe) {
- const int64 handle = entry->ref(META_HANDLE);
- const ModelType type = entry->GetServerModelType();
+ int64 handle = entry->ref(META_HANDLE);
CHECK_EQ(kernel_->dirty_metahandles->count(handle), 0U);
// TODO(tim): Bug 49278.
CHECK(!kernel_->unsynced_metahandles->count(handle));
- CHECK(!kernel_->unapplied_update_metahandles[type].count(handle));
+ CHECK(!kernel_->unapplied_update_metahandles->count(handle));
}
return safe;
@@ -854,8 +837,7 @@ void Directory::PurgeEntriesWithTypeIn(const std::set<ModelType>& types) {
DCHECK_EQ(entry->ref(UNIQUE_CLIENT_TAG).empty(), !num_erased);
num_erased = kernel_->unsynced_metahandles->erase(handle);
DCHECK_EQ(entry->ref(IS_UNSYNCED), num_erased > 0);
- num_erased =
- kernel_->unapplied_update_metahandles[server_type].erase(handle);
+ num_erased = kernel_->unapplied_update_metahandles->erase(handle);
DCHECK_EQ(entry->ref(IS_UNAPPLIED_UPDATE), num_erased > 0);
num_erased = kernel_->parent_id_child_index->erase(entry);
DCHECK_EQ(entry->ref(IS_DEL), !num_erased);
@@ -1025,33 +1007,13 @@ int64 Directory::unsynced_entity_count() const {
return kernel_->unsynced_metahandles->size();
}
-syncable::ModelTypeBitSet
- Directory::GetServerTypesWithUnappliedUpdates(
- BaseTransaction* trans) const {
- syncable::ModelTypeBitSet server_types;
- ScopedKernelLock lock(this);
- for (int i = 0; i < MODEL_TYPE_COUNT; ++i) {
- if (!kernel_->unapplied_update_metahandles[i].empty()) {
- server_types.set(i);
- }
- }
- return server_types;
-}
-
-void Directory::GetUnappliedUpdateMetaHandles(
- BaseTransaction* trans,
- syncable::ModelTypeBitSet server_types,
+void Directory::GetUnappliedUpdateMetaHandles(BaseTransaction* trans,
UnappliedUpdateMetaHandles* result) {
result->clear();
ScopedKernelLock lock(this);
- for (int i = 0; i < MODEL_TYPE_COUNT; ++i) {
- const ModelType type = ModelTypeFromInt(i);
- if (server_types.test(type)) {
- std::copy(kernel_->unapplied_update_metahandles[type].begin(),
- kernel_->unapplied_update_metahandles[type].end(),
- back_inserter(*result));
- }
- }
+ copy(kernel_->unapplied_update_metahandles->begin(),
+ kernel_->unapplied_update_metahandles->end(),
+ back_inserter(*result));
}
@@ -1408,6 +1370,8 @@ DictionaryValue* Entry::ToValue() const {
entry_info->SetBoolean("good", good());
if (good()) {
entry_info->Set("kernel", kernel_->ToValue());
+ entry_info->Set("serverModelType",
+ ModelTypeToValue(GetServerModelTypeHelper()));
entry_info->Set("modelType",
ModelTypeToValue(GetModelType()));
entry_info->SetBoolean("existsOnClientBecauseNameIsNonEmpty",
@@ -1423,7 +1387,7 @@ const string& Entry::Get(StringField field) const {
}
syncable::ModelType Entry::GetServerModelType() const {
- ModelType specifics_type = kernel_->GetServerModelType();
+ ModelType specifics_type = GetServerModelTypeHelper();
if (specifics_type != UNSPECIFIED)
return specifics_type;
@@ -1439,6 +1403,20 @@ syncable::ModelType Entry::GetServerModelType() const {
return UNSPECIFIED;
}
+syncable::ModelType Entry::GetServerModelTypeHelper() const {
+ ModelType specifics_type = GetModelTypeFromSpecifics(Get(SERVER_SPECIFICS));
+ if (specifics_type != UNSPECIFIED)
+ return specifics_type;
+ if (IsRoot())
+ return TOP_LEVEL_FOLDER;
+ // Loose check for server-created top-level folders that aren't
+ // bound to a particular model type.
+ if (!Get(UNIQUE_SERVER_TAG).empty() && Get(SERVER_IS_DIR))
+ return TOP_LEVEL_FOLDER;
+
+ return UNSPECIFIED;
+}
+
syncable::ModelType Entry::GetModelType() const {
ModelType specifics_type = GetModelTypeFromSpecifics(Get(SPECIFICS));
if (specifics_type != UNSPECIFIED)
@@ -1625,32 +1603,8 @@ bool MutableEntry::Put(ProtoField field,
// TODO(ncarter): This is unfortunately heavyweight. Can we do
// better?
if (kernel_->ref(field).SerializeAsString() != value.SerializeAsString()) {
- const bool update_unapplied_updates_index =
- (field == SERVER_SPECIFICS) && kernel_->ref(IS_UNAPPLIED_UPDATE);
- if (update_unapplied_updates_index) {
- // Remove ourselves from unapplied_update_metahandles with our
- // old server type.
- const syncable::ModelType old_server_type =
- kernel_->GetServerModelType();
- const int64 metahandle = kernel_->ref(META_HANDLE);
- size_t erase_count =
- dir()->kernel_->unapplied_update_metahandles[old_server_type]
- .erase(metahandle);
- DCHECK_EQ(erase_count, 1u);
- }
-
kernel_->put(field, value);
kernel_->mark_dirty(dir()->kernel_->dirty_metahandles);
-
- if (update_unapplied_updates_index) {
- // Add ourselves back into unapplied_update_metahandles with our
- // new server type.
- const syncable::ModelType new_server_type =
- kernel_->GetServerModelType();
- const int64 metahandle = kernel_->ref(META_HANDLE);
- dir()->kernel_->unapplied_update_metahandles[new_server_type]
- .insert(metahandle);
- }
}
return true;
}
@@ -1713,16 +1667,10 @@ bool MutableEntry::Put(IndexedBitField field, bool value) {
DCHECK(kernel_);
if (kernel_->ref(field) != value) {
MetahandleSet* index;
- if (IS_UNSYNCED == field) {
+ if (IS_UNSYNCED == field)
index = dir()->kernel_->unsynced_metahandles;
- } else {
- // Use kernel_->GetServerModelType() instead of
- // GetServerModelType() as we may trigger some DCHECKs in the
- // latter.
- index =
- &dir()->kernel_->unapplied_update_metahandles[
- kernel_->GetServerModelType()];
- }
+ else
+ index = dir()->kernel_->unapplied_update_metahandles;
ScopedKernelLock lock(dir());
if (value)
diff --git a/chrome/browser/sync/syncable/syncable.h b/chrome/browser/sync/syncable/syncable.h
index 73e71d5..64c518b 100644
--- a/chrome/browser/sync/syncable/syncable.h
+++ b/chrome/browser/sync/syncable/syncable.h
@@ -371,8 +371,6 @@ struct EntryKernel {
return id_fields[field - ID_FIELDS_BEGIN];
}
- syncable::ModelType GetServerModelType() const;
-
// Does a case in-sensitive search for a given string, which must be
// lower case.
bool ContainsString(const std::string& lowercase_query) const;
@@ -489,6 +487,9 @@ class Entry {
EntryKernel* kernel_;
private:
+ // Like GetServerModelType() but without the DCHECKs.
+ ModelType GetServerModelTypeHelper() const;
+
DISALLOW_COPY_AND_ASSIGN(Entry);
};
@@ -968,17 +969,9 @@ class Directory {
void GetUnsyncedMetaHandles(BaseTransaction* trans,
UnsyncedMetaHandles* result);
- // Returns all server types with unapplied updates. A subset of
- // those types can then be passed into
- // GetUnappliedUpdateMetaHandles() below.
- syncable::ModelTypeBitSet GetServerTypesWithUnappliedUpdates(
- BaseTransaction* trans) const;
-
- // Get all the metahandles for unapplied updates for a given set of
- // server types.
+ // Get all the metahandles for unapplied updates
typedef std::vector<int64> UnappliedUpdateMetaHandles;
void GetUnappliedUpdateMetaHandles(BaseTransaction* trans,
- syncable::ModelTypeBitSet server_types,
UnappliedUpdateMetaHandles* result);
// Checks tree metadata consistency.
@@ -1110,8 +1103,7 @@ class Directory {
EntryKernel needle;
// 3 in-memory indices on bits used extremely frequently by the syncer.
- // |unapplied_update_metahandles| is keyed by the server model type.
- MetahandleSet unapplied_update_metahandles[MODEL_TYPE_COUNT];
+ MetahandleSet* const unapplied_update_metahandles;
MetahandleSet* const unsynced_metahandles;
// Contains metahandles that are most likely dirty (though not
// necessarily). Dirtyness is confirmed in TakeSnapshotForSaveChanges().
diff --git a/chrome/browser/sync/syncable/syncable_unittest.cc b/chrome/browser/sync/syncable/syncable_unittest.cc
index b7fcb6e..1b6fc38 100644
--- a/chrome/browser/sync/syncable/syncable_unittest.cc
+++ b/chrome/browser/sync/syncable/syncable_unittest.cc
@@ -47,8 +47,8 @@ TEST_F(SyncableKernelTest, ToValue) {
if (value.get()) {
// Not much to check without repeating the ToValue() code.
EXPECT_TRUE(value->HasKey("isDirty"));
- // The extra +2 is for "isDirty" and "serverModelType".
- EXPECT_EQ(BIT_TEMPS_END - BEGIN_FIELDS + 2,
+ // The extra +1 is for "isDirty".
+ EXPECT_EQ(BIT_TEMPS_END - BEGIN_FIELDS + 1,
static_cast<int>(value->size()));
} else {
ADD_FAILURE();
@@ -370,6 +370,7 @@ TEST_F(SyncableGeneralTest, ToValue) {
scoped_ptr<DictionaryValue> value(me.ToValue());
ExpectDictBooleanValue(true, *value, "good");
EXPECT_TRUE(value->HasKey("kernel"));
+ ExpectDictStringValue("Unspecified", *value, "serverModelType");
ExpectDictStringValue("Unspecified", *value, "modelType");
ExpectDictBooleanValue(true, *value, "existsOnClientBecauseNameIsNonEmpty");
ExpectDictBooleanValue(false, *value, "isRoot");
@@ -879,12 +880,10 @@ TEST_F(SyncableDirectoryTest, TestGetUnsynced) {
TEST_F(SyncableDirectoryTest, TestGetUnappliedUpdates) {
Directory::UnappliedUpdateMetaHandles handles;
int64 handle1, handle2;
- syncable::ModelTypeBitSet all_types;
- all_types.set();
{
WriteTransaction trans(FROM_HERE, UNITTEST, dir_.get());
- dir_->GetUnappliedUpdateMetaHandles(&trans, all_types, &handles);
+ dir_->GetUnappliedUpdateMetaHandles(&trans, &handles);
ASSERT_TRUE(0 == handles.size());
MutableEntry e1(&trans, CREATE, trans.root_id(), "abba");
@@ -906,7 +905,7 @@ TEST_F(SyncableDirectoryTest, TestGetUnappliedUpdates) {
{
WriteTransaction trans(FROM_HERE, UNITTEST, dir_.get());
- dir_->GetUnappliedUpdateMetaHandles(&trans, all_types, &handles);
+ dir_->GetUnappliedUpdateMetaHandles(&trans, &handles);
ASSERT_TRUE(0 == handles.size());
MutableEntry e3(&trans, GET_BY_HANDLE, handle1);
@@ -916,7 +915,7 @@ TEST_F(SyncableDirectoryTest, TestGetUnappliedUpdates) {
dir_->SaveChanges();
{
WriteTransaction trans(FROM_HERE, UNITTEST, dir_.get());
- dir_->GetUnappliedUpdateMetaHandles(&trans, all_types, &handles);
+ dir_->GetUnappliedUpdateMetaHandles(&trans, &handles);
ASSERT_TRUE(1 == handles.size());
ASSERT_TRUE(handle1 == handles[0]);
@@ -927,7 +926,7 @@ TEST_F(SyncableDirectoryTest, TestGetUnappliedUpdates) {
dir_->SaveChanges();
{
WriteTransaction trans(FROM_HERE, UNITTEST, dir_.get());
- dir_->GetUnappliedUpdateMetaHandles(&trans, all_types, &handles);
+ dir_->GetUnappliedUpdateMetaHandles(&trans, &handles);
ASSERT_TRUE(2 == handles.size());
if (handle1 == handles[0]) {
ASSERT_TRUE(handle2 == handles[1]);
@@ -943,7 +942,7 @@ TEST_F(SyncableDirectoryTest, TestGetUnappliedUpdates) {
dir_->SaveChanges();
{
WriteTransaction trans(FROM_HERE, UNITTEST, dir_.get());
- dir_->GetUnappliedUpdateMetaHandles(&trans, all_types, &handles);
+ dir_->GetUnappliedUpdateMetaHandles(&trans, &handles);
ASSERT_TRUE(1 == handles.size());
ASSERT_TRUE(handle2 == handles[0]);
}
diff --git a/chrome/browser/sync/test/engine/fake_model_safe_worker_registrar.cc b/chrome/browser/sync/test/engine/fake_model_safe_worker_registrar.cc
index 21ab061..15a4d45f9 100644
--- a/chrome/browser/sync/test/engine/fake_model_safe_worker_registrar.cc
+++ b/chrome/browser/sync/test/engine/fake_model_safe_worker_registrar.cc
@@ -15,8 +15,6 @@ FakeModelSafeWorkerRegistrar::FakeModelSafeWorkerRegistrar(
it != routes_.end(); ++it) {
groups.insert(it->second);
}
- // Sessions always expect a passive worker to be present.
- groups.insert(GROUP_PASSIVE);
for (std::set<ModelSafeGroup>::const_iterator it = groups.begin();
it != groups.end(); ++it) {
diff --git a/chrome/browser/sync/test/engine/syncer_command_test.h b/chrome/browser/sync/test/engine/syncer_command_test.h
index 2b4e17a..c6fccbb 100644
--- a/chrome/browser/sync/test/engine/syncer_command_test.h
+++ b/chrome/browser/sync/test/engine/syncer_command_test.h
@@ -11,15 +11,12 @@
#include <vector>
#include "base/compiler_specific.h"
-#include "base/memory/ref_counted.h"
#include "base/message_loop.h"
-#include "chrome/browser/sync/engine/model_changing_syncer_command.h"
#include "chrome/browser/sync/engine/model_safe_worker.h"
#include "chrome/browser/sync/sessions/debug_info_getter.h"
#include "chrome/browser/sync/sessions/sync_session.h"
#include "chrome/browser/sync/sessions/sync_session_context.h"
#include "chrome/browser/sync/test/engine/mock_connection_manager.h"
-#include "chrome/browser/sync/test/engine/fake_model_worker.h"
#include "chrome/browser/sync/test/engine/test_directory_setter_upper.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -96,9 +93,6 @@ class SyncerCommandTestWithParam : public testing::TestWithParam<T>,
virtual void SetUp() {
syncdb_->SetUp();
ResetContext();
- // The session always expects there to be a passive worker.
- workers()->push_back(
- make_scoped_refptr(new FakeModelWorker(GROUP_PASSIVE)));
}
virtual void TearDown() {
syncdb_->TearDown();
@@ -163,41 +157,6 @@ class SyncerCommandTestWithParam : public testing::TestWithParam<T>,
return &mock_debug_info_getter_;
}
- // Helper functions to check command.GetGroupsToChange().
-
- void ExpectNoGroupsToChange(const ModelChangingSyncerCommand& command) {
- EXPECT_TRUE(command.GetGroupsToChangeForTest(*session()).empty());
- }
-
- void ExpectGroupToChange(
- const ModelChangingSyncerCommand& command, ModelSafeGroup group) {
- std::set<ModelSafeGroup> expected_groups_to_change;
- expected_groups_to_change.insert(group);
- EXPECT_EQ(expected_groups_to_change,
- command.GetGroupsToChangeForTest(*session()));
- }
-
- void ExpectGroupsToChange(
- const ModelChangingSyncerCommand& command,
- ModelSafeGroup group1, ModelSafeGroup group2) {
- std::set<ModelSafeGroup> expected_groups_to_change;
- expected_groups_to_change.insert(group1);
- expected_groups_to_change.insert(group2);
- EXPECT_EQ(expected_groups_to_change,
- command.GetGroupsToChangeForTest(*session()));
- }
-
- void ExpectGroupsToChange(
- const ModelChangingSyncerCommand& command,
- ModelSafeGroup group1, ModelSafeGroup group2, ModelSafeGroup group3) {
- std::set<ModelSafeGroup> expected_groups_to_change;
- expected_groups_to_change.insert(group1);
- expected_groups_to_change.insert(group2);
- expected_groups_to_change.insert(group3);
- EXPECT_EQ(expected_groups_to_change,
- command.GetGroupsToChangeForTest(*session()));
- }
-
private:
MessageLoop message_loop_;
scoped_ptr<TestDirectorySetterUpper> syncdb_;