Skip to content

GitLab

T
tangled
Commit cb0a4762 authored by zea@chromium.org's avatar zea@chromium.org
Browse files
Options

Reland 148792 

Relanding after rebasing.

[Sync] Remove CleanupDisabledTypes command and move purge logic into SyncManager."

Original codereview at http://codereview.chromium.org/10541079/

BUG=131433, 90868
TBR=tim@chromium.org


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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149052 0039d316-1c4b-4281-b951-d872f2087c98
parent d8083825
Show whitespace changes
Inline Side-by-side
Showing with 185 additions and 331 deletions
sync/engine/cleanup_disabled_types_command.cc deleted 100644 → 0
View file @ d8083825
// Copyright (c) 2012 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 "sync/engine/cleanup_disabled_types_command.h"
#include <algorithm>
#include "sync/internal_api/public/base/model_type.h"
#include "sync/sessions/sync_session.h"
#include "sync/sessions/sync_session_context.h"
#include "sync/syncable/directory.h"
namespace syncer {
CleanupDisabledTypesCommand::CleanupDisabledTypesCommand() {}
CleanupDisabledTypesCommand::~CleanupDisabledTypesCommand() {}
SyncerError CleanupDisabledTypesCommand::ExecuteImpl(
sessions::SyncSession* session) {
// Because a full directory purge is slow, we avoid purging
// undesired types unless we have reason to believe they were
// previously enabled. Because purging could theoretically fail on
// the first sync session (when there's no previous routing info) we
// pay the full directory scan price once and do a "deep clean" of
// types that may potentially need cleanup so that we converge to
// the correct state.
//
// in_previous | !in_previous
// |
// initial_sync_ended should clean | may have attempted cleanup
// !initial_sync_ended should clean | may have never been enabled, or
// | could have been disabled before
// | initial sync ended and cleanup
// | may not have happened yet
// | (failure, browser restart
// | before another sync session,..)
const ModelTypeSet enabled_types =
GetRoutingInfoTypes(session->routing_info());
const ModelTypeSet previous_enabled_types =
GetRoutingInfoTypes(
session->context()->previous_session_routing_info());
ModelTypeSet to_cleanup = Difference(ModelTypeSet::All(), enabled_types);
// If |previous_enabled_types| is non-empty (i.e., not the first
// sync session), set |to_cleanup| to its intersection with
// |previous_enabled_types|.
if (!previous_enabled_types.Empty()) {
to_cleanup.RetainAll(previous_enabled_types);
}
DVLOG(1) << "enabled_types = " << ModelTypeSetToString(enabled_types)
<< ", previous_enabled_types = "
<< ModelTypeSetToString(previous_enabled_types)
<< ", to_cleanup = " << ModelTypeSetToString(to_cleanup);
if (to_cleanup.Empty())
return SYNCER_OK;
session->context()->directory()->PurgeEntriesWithTypeIn(to_cleanup);
return SYNCER_OK;
}
} // namespace syncer
sync/engine/cleanup_disabled_types_command.h deleted 100644 → 0
View file @ d8083825
// Copyright (c) 2012 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.
#ifndef SYNC_ENGINE_CLEANUP_DISABLED_TYPES_COMMAND_H_
#define SYNC_ENGINE_CLEANUP_DISABLED_TYPES_COMMAND_H_
#include "base/compiler_specific.h"
#include "sync/engine/syncer_command.h"
namespace syncer {
// A syncer command that purges (from memory and disk) entries belonging to
// a ModelType or ServerModelType that the user has not elected to sync.
//
// This is done as part of a session to 1) ensure it does not block the UI,
// and 2) avoid complicated races that could arise between a) deleting
// things b) a sync session trying to use these things c) and the potential
// re-enabling of the data type by the user before some scheduled deletion
// took place. Here, we are safe to perform I/O synchronously and we know it
// is a safe time to delete as we are in the only active session.
//
// The removal from memory is done synchronously, while the disk purge is left
// to an asynchronous SaveChanges operation. However, all the updates for
// meta data fields (such as initial_sync_ended) as well as the actual entry
// deletions will be committed in a single sqlite transaction. Thus it is
// possible that disabled types re-appear (in the sync db) after a reboot,
// but things will remain in a consistent state. This kind of error case is
// cared for in this command by retrying; see ExecuteImpl.
class CleanupDisabledTypesCommand : public SyncerCommand {
public:
CleanupDisabledTypesCommand();
virtual ~CleanupDisabledTypesCommand();
// SyncerCommand implementation.
virtual SyncerError ExecuteImpl(sessions::SyncSession* session) OVERRIDE;
private:
DISALLOW_COPY_AND_ASSIGN(CleanupDisabledTypesCommand);
};
} // namespace syncer
#endif // SYNC_ENGINE_CLEANUP_DISABLED_TYPES_COMMAND_H_
sync/engine/cleanup_disabled_types_command_unittest.cc deleted 100644 → 0
View file @ d8083825
// Copyright (c) 2012 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 <vector>
#include "sync/engine/cleanup_disabled_types_command.h"
#include "sync/internal_api/public/base/model_type_test_util.h"
#include "sync/sessions/sync_session.h"
#include "sync/test/engine/syncer_command_test.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace syncer {
namespace {
using testing::_;
class CleanupDisabledTypesCommandTest : public MockDirectorySyncerCommandTest {
public:
CleanupDisabledTypesCommandTest() {}
virtual void SetUp() {
mutable_routing_info()->clear();
(*mutable_routing_info())[BOOKMARKS] = GROUP_PASSIVE;
MockDirectorySyncerCommandTest::SetUp();
}
};
// TODO(tim): Add syncer test to verify previous routing info is set.
TEST_F(CleanupDisabledTypesCommandTest, NoPreviousRoutingInfo) {
CleanupDisabledTypesCommand command;
ModelTypeSet expected = ModelTypeSet::All();
expected.Remove(BOOKMARKS);
EXPECT_CALL(*mock_directory(),
PurgeEntriesWithTypeIn(HasModelTypes(expected)));
command.ExecuteImpl(session());
}
TEST_F(CleanupDisabledTypesCommandTest, NoPurge) {
CleanupDisabledTypesCommand command;
EXPECT_CALL(*mock_directory(), PurgeEntriesWithTypeIn(_)).Times(0);
ModelSafeRoutingInfo prev(routing_info());
session()->context()->set_previous_session_routing_info(prev);
(*mutable_routing_info())[AUTOFILL] = GROUP_PASSIVE;
command.ExecuteImpl(session());
prev = routing_info();
command.ExecuteImpl(session());
}
TEST_F(CleanupDisabledTypesCommandTest, TypeDisabled) {
CleanupDisabledTypesCommand command;
(*mutable_routing_info())[AUTOFILL] = GROUP_PASSIVE;
(*mutable_routing_info())[THEMES] = GROUP_PASSIVE;
(*mutable_routing_info())[EXTENSIONS] = GROUP_PASSIVE;
ModelSafeRoutingInfo prev(routing_info());
prev[PASSWORDS] = GROUP_PASSIVE;
prev[PREFERENCES] = GROUP_PASSIVE;
session()->context()->set_previous_session_routing_info(prev);
const ModelTypeSet expected(PASSWORDS, PREFERENCES);
EXPECT_CALL(*mock_directory(),
PurgeEntriesWithTypeIn(HasModelTypes(expected)));
command.ExecuteImpl(session());
}
} // namespace
} // namespace syncer
sync/engine/sync_scheduler_impl.cc
View file @ cb0a4762
......@@ -132,7 +132,6 @@ const char* SyncSchedulerImpl::SyncSessionJob::GetPurposeString(
ENUM_CASE(POLL);
ENUM_CASE(NUDGE);
ENUM_CASE(CONFIGURATION);
ENUM_CASE(CLEANUP_DISABLED_TYPES);
}
NOTREACHED();
return "";
......@@ -354,17 +353,6 @@ bool SyncSchedulerImpl::ScheduleConfiguration(
&restricted_workers);
session_context_->set_routing_info(params.routing_info);
// We rely on this not failing, so don't need to worry about checking for
// success. In addition, this will be removed as part of crbug.com/131433.
SyncSessionJob cleanup_job(
SyncSessionJob::CLEANUP_DISABLED_TYPES,
TimeTicks::Now(),
make_linked_ptr(CreateSyncSession(SyncSourceInfo())),
false,
ConfigurationParams(),
FROM_HERE);
DoSyncSessionJob(cleanup_job);
if (params.keystore_key_status == ConfigurationParams::KEYSTORE_KEY_NEEDED) {
// TODO(zea): implement in such a way that we can handle failures and the
// subsequent retrys the scheduler might perform. See crbug.com/129665.
......@@ -411,7 +399,6 @@ SyncSchedulerImpl::DecideWhileInWaitInterval(
const SyncSessionJob& job) {
DCHECK_EQ(MessageLoop::current(), sync_loop_);
DCHECK(wait_interval_.get());
DCHECK_NE(job.purpose, SyncSessionJob::CLEANUP_DISABLED_TYPES);
SDVLOG(2) << "DecideWhileInWaitInterval with WaitInterval mode "
<< WaitInterval::GetModeString(wait_interval_->mode)
......@@ -444,8 +431,6 @@ SyncSchedulerImpl::DecideWhileInWaitInterval(
SyncSchedulerImpl::JobProcessDecision SyncSchedulerImpl::DecideOnJob(
const SyncSessionJob& job) {
DCHECK_EQ(MessageLoop::current(), sync_loop_);
if (job.purpose == SyncSessionJob::CLEANUP_DISABLED_TYPES)
return CONTINUE;
// See if our type is throttled.
ModelTypeSet throttled_types =
......@@ -539,8 +524,6 @@ bool SyncSchedulerImpl::ShouldRunJob(const SyncSessionJob& job) {
void SyncSchedulerImpl::SaveJob(const SyncSessionJob& job) {
DCHECK_EQ(MessageLoop::current(), sync_loop_);
// TODO(sync): Should we also check that job.purpose !=
// CLEANUP_DISABLED_TYPES? (See http://crbug.com/90868.)
if (job.purpose == SyncSessionJob::NUDGE) {
SDVLOG(2) << "Saving a nudge job";
InitOrCoalescePendingJob(job);
......@@ -697,10 +680,6 @@ void SyncSchedulerImpl::SetSyncerStepsForPurpose(
*start = SYNCER_BEGIN;
*end = SYNCER_END;
return;
case SyncSessionJob::CLEANUP_DISABLED_TYPES:
*start = CLEANUP_DISABLED_TYPES;
*end = CLEANUP_DISABLED_TYPES;
return;
default:
NOTREACHED();
*start = SYNCER_END;
......@@ -805,27 +784,6 @@ void SyncSchedulerImpl::DoSyncSessionJob(const SyncSessionJob& job) {
FinishSyncSessionJob(job);
}
void SyncSchedulerImpl::UpdateCarryoverSessionState(
const SyncSessionJob& old_job) {
DCHECK_EQ(MessageLoop::current(), sync_loop_);
if (old_job.purpose == SyncSessionJob::CONFIGURATION) {
// Whatever types were part of a configuration task will have had updates
// downloaded. For that reason, we make sure they get recorded in the
// event that they get disabled at a later time.
ModelSafeRoutingInfo r(session_context_->previous_session_routing_info());
if (!r.empty()) {
ModelSafeRoutingInfo temp_r;
ModelSafeRoutingInfo old_info(old_job.session->routing_info());
std::set_union(r.begin(), r.end(), old_info.begin(), old_info.end(),
std::insert_iterator<ModelSafeRoutingInfo>(temp_r, temp_r.begin()));
session_context_->set_previous_session_routing_info(temp_r);
}
} else {
session_context_->set_previous_session_routing_info(
old_job.session->routing_info());
}
}
void SyncSchedulerImpl::FinishSyncSessionJob(const SyncSessionJob& job) {
DCHECK_EQ(MessageLoop::current(), sync_loop_);
// Update timing information for how often datatypes are triggering nudges.
......@@ -855,7 +813,6 @@ void SyncSchedulerImpl::FinishSyncSessionJob(const SyncSessionJob& job) {
ServerConnectionManager* scm = session_context_->connection_manager();
UpdateServerConnectionManagerStatus(scm->server_status());
UpdateCarryoverSessionState(job);
if (IsSyncingCurrentlySilenced()) {
SDVLOG(2) << "We are currently throttled; not scheduling the next sync.";
// TODO(sync): Investigate whether we need to check job.purpose
......
sync/engine/sync_scheduler_impl.h
View file @ cb0a4762
......@@ -99,12 +99,8 @@ class SyncSchedulerImpl : public SyncScheduler {
// a sync. The source is inferable from |session.source()|.
NUDGE,
// Typically used for fetching updates for a subset of the enabled types
// during initial sync or reconfiguration. We don't run all steps of
// the sync cycle for these (e.g. CleanupDisabledTypes is skipped).
// during initial sync or reconfiguration.
CONFIGURATION,
// The user disabled some types and we have to clean up the data
// for those.
CLEANUP_DISABLED_TYPES,
};
SyncSessionJob();
SyncSessionJob(SyncSessionJobPurpose purpose, base::TimeTicks start,
......@@ -220,10 +216,6 @@ class SyncSchedulerImpl : public SyncScheduler {
// reset our state.
void FinishSyncSessionJob(const SyncSessionJob& job);
// Record important state that might be needed in future syncs, such as which
// data types may require cleanup.
void UpdateCarryoverSessionState(const SyncSessionJob& old_job);
// Helper to FinishSyncSessionJob to schedule the next sync operation.
void ScheduleNextSync(const SyncSessionJob& old_job);
......
sync/engine/sync_scheduler_unittest.cc
View file @ cb0a4762
......@@ -301,9 +301,7 @@ TEST_F(SyncSchedulerTest, Config) {
SyncShareRecords records;
const ModelTypeSet model_types(BOOKMARKS);
EXPECT_CALL(*syncer(),
SyncShare(_,_,_))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess))
EXPECT_CALL(*syncer(), SyncShare(_,_,_))
.WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
WithArg<0>(RecordSyncShare(&records))));
......@@ -334,9 +332,7 @@ TEST_F(SyncSchedulerTest, ConfigWithBackingOff) {
SyncShareRecords records;
const ModelTypeSet model_types(BOOKMARKS);
EXPECT_CALL(*syncer(),
SyncShare(_,_,_))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess))
EXPECT_CALL(*syncer(), SyncShare(_,_,_))
.WillOnce(DoAll(Invoke(sessions::test_util::SimulateCommitFailed),
WithArg<0>(RecordSyncShare(&records))))
.WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
......@@ -710,19 +706,14 @@ TEST_F(SyncSchedulerTest, HasMoreToSyncThenFails) {
EXPECT_TRUE(RunAndGetBackoff());
}
// Test that no syncing occurs when throttled (although CleanupDisabledTypes
// is allowed).
// Test that no syncing occurs when throttled.
TEST_F(SyncSchedulerTest, ThrottlingDoesThrottle) {
const ModelTypeSet types(BOOKMARKS);
TimeDelta poll(TimeDelta::FromMilliseconds(5));
TimeDelta throttle(TimeDelta::FromMinutes(10));
scheduler()->OnReceivedLongPollIntervalUpdate(poll);
EXPECT_CALL(*syncer(),
SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess));
EXPECT_CALL(*syncer(), SyncShare(_,Not(CLEANUP_DISABLED_TYPES),
Not(CLEANUP_DISABLED_TYPES)))
EXPECT_CALL(*syncer(), SyncShare(_,_,_))
.WillOnce(WithArg<0>(sessions::test_util::SimulateThrottled(throttle)))
.WillRepeatedly(AddFailureAndQuitLoopNow());
......@@ -775,7 +766,6 @@ TEST_F(SyncSchedulerTest, ConfigurationMode) {
SyncShareRecords records;
scheduler()->OnReceivedLongPollIntervalUpdate(poll);
EXPECT_CALL(*syncer(), SyncShare(_,_,_))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess))
.WillOnce(DoAll(Invoke(sessions::test_util::SimulateSuccess),
WithArg<0>(RecordSyncShare(&records))));
......@@ -900,10 +890,6 @@ TEST_F(SyncSchedulerTest, BackoffDropsJobs) {
EXPECT_EQ(GetUpdatesCallerInfo::LOCAL,
r.snapshots[1].source().updates_source);
// Cleanup is not affected by backoff, but it should not relieve it either.
EXPECT_CALL(*syncer(),
SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess));
EXPECT_CALL(*delay(), GetDelay(_)).Times(0);
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
......@@ -1077,10 +1063,7 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
StopSyncScheduler();
Mock::VerifyAndClearExpectations(syncer());
// Configuration (always includes a cleanup disabled types).
EXPECT_CALL(*syncer(),
SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess));
// Configuration.
EXPECT_CALL(*syncer(), SyncShare(_, DOWNLOAD_UPDATES, APPLY_UPDATES))
.WillOnce(Invoke(sessions::test_util::SimulateSuccess));
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
......@@ -1099,25 +1082,6 @@ TEST_F(SyncSchedulerTest, SyncerSteps) {
StopSyncScheduler();
Mock::VerifyAndClearExpectations(syncer());
// Cleanup disabled types. Because no types are being configured, we just
// perform the cleanup.
EXPECT_CALL(*syncer(),
SyncShare(_, CLEANUP_DISABLED_TYPES, CLEANUP_DISABLED_TYPES)).
WillOnce(Invoke(sessions::test_util::SimulateSuccess));
StartSyncScheduler(SyncScheduler::CONFIGURATION_MODE);
CallbackCounter counter2;
ConfigurationParams params2(
GetUpdatesCallerInfo::RECONFIGURATION,
ModelTypeSet(),
ModelSafeRoutingInfo(),
ConfigurationParams::KEYSTORE_KEY_UNNECESSARY,
base::Bind(&CallbackCounter::Callback, base::Unretained(&counter2)));
ASSERT_TRUE(scheduler()->ScheduleConfiguration(params2));
ASSERT_EQ(1, counter2.times_called());
StopSyncScheduler();
Mock::VerifyAndClearExpectations(syncer());
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
// Poll.
......@@ -1163,24 +1127,4 @@ TEST_F(SyncSchedulerTest, StartWhenNotConnected) {
MessageLoop::current()->RunAllPending();
}
TEST_F(SyncSchedulerTest, SetsPreviousRoutingInfo) {
ModelSafeRoutingInfo info;
EXPECT_TRUE(info == context()->previous_session_routing_info());
ModelSafeRoutingInfo expected(context()->routing_info());
ASSERT_FALSE(expected.empty());
EXPECT_CALL(*syncer(), SyncShare(_,_,_)).Times(1);
StartSyncScheduler(SyncScheduler::NORMAL_MODE);
scheduler()->ScheduleNudgeAsync(
zero(), NUDGE_SOURCE_LOCAL, ModelTypeSet(), FROM_HERE);
PumpLoop();
// Pump again to run job.
PumpLoop();
StopSyncScheduler();
EXPECT_TRUE(expected == context()->previous_session_routing_info());
}
} // namespace syncer
sync/engine/syncer.cc
View file @ cb0a4762
......@@ -12,7 +12,6 @@
#include "build/build_config.h"
#include "sync/engine/apply_updates_command.h"
#include "sync/engine/build_commit_command.h"
#include "sync/engine/cleanup_disabled_types_command.h"
#include "sync/engine/commit.h"
#include "sync/engine/conflict_resolver.h"
#include "sync/engine/download_updates_command.h"
......@@ -53,7 +52,6 @@ const char* SyncerStepToString(const SyncerStep step)
{
switch (step) {
ENUM_CASE(SYNCER_BEGIN);
ENUM_CASE(CLEANUP_DISABLED_TYPES);
ENUM_CASE(DOWNLOAD_UPDATES);
ENUM_CASE(PROCESS_CLIENT_COMMAND);
ENUM_CASE(VERIFY_UPDATES);
......@@ -106,14 +104,8 @@ void Syncer::SyncShare(sessions::SyncSession* session,
PruneUnthrottledTypes(base::TimeTicks::Now());
session->SendEventNotification(SyncEngineEvent::SYNC_CYCLE_BEGIN);
next_step = CLEANUP_DISABLED_TYPES;
break;
case CLEANUP_DISABLED_TYPES: {
CleanupDisabledTypesCommand cleanup;
cleanup.Execute(session);
next_step = DOWNLOAD_UPDATES;
break;
}
case DOWNLOAD_UPDATES: {
// TODO(akalin): We may want to propagate this switch up
// eventually.
......
sync/engine/syncer.h
View file @ cb0a4762
......@@ -26,7 +26,6 @@ class MutableEntry;
enum SyncerStep {
SYNCER_BEGIN,
CLEANUP_DISABLED_TYPES,
DOWNLOAD_UPDATES,
PROCESS_CLIENT_COMMAND,
VERIFY_UPDATES,
......
sync/internal_api/sync_manager_impl.cc
View file @ cb0a4762
......@@ -338,6 +338,19 @@ void SyncManagerImpl::ConfigureSyncer(
DCHECK(!ready_task.is_null());
DCHECK(!retry_task.is_null());
// Cleanup any types that might have just been disabled.
ModelTypeSet previous_types = ModelTypeSet::All();
if (!session_context_->routing_info().empty())
previous_types = GetRoutingInfoTypes(session_context_->routing_info());
if (!PurgeDisabledTypes(previous_types,
GetRoutingInfoTypes(new_routing_info))) {
// We failed to cleanup the types. Invoke the ready task without actually
// configuring any types. The caller should detect this as a configuration
// failure and act appropriately.
ready_task.Run();
return;
}
// TODO(zea): set this based on whether cryptographer has keystore
// encryption key or not (requires opening a transaction). crbug.com/129665.
ConfigurationParams::KeystoreKeyStatus keystore_key_status =
......@@ -689,6 +702,19 @@ bool SyncManagerImpl::PurgePartiallySyncedTypes() {
return directory()->PurgeEntriesWithTypeIn(partially_synced_types);
}
bool SyncManagerImpl::PurgeDisabledTypes(
ModelTypeSet previously_enabled_types,
ModelTypeSet currently_enabled_types) {
ModelTypeSet disabled_types = Difference(previously_enabled_types,
currently_enabled_types);
if (disabled_types.Empty())
return true;
DVLOG(1) << "Purging disabled types "
<< ModelTypeSetToString(disabled_types);
return directory()->PurgeEntriesWithTypeIn(disabled_types);
}
void SyncManagerImpl::UpdateCredentials(
const SyncCredentials& credentials) {
DCHECK(thread_checker_.CalledOnValidThread());
......
sync/internal_api/sync_manager_impl.h
View file @ cb0a4762
......@@ -179,6 +179,7 @@ class SyncManagerImpl : public SyncManager,
FRIEND_TEST_ALL_PREFIXES(SyncManagerTest, NudgeDelayTest);
FRIEND_TEST_ALL_PREFIXES(SyncManagerTest, OnNotificationStateChange);
FRIEND_TEST_ALL_PREFIXES(SyncManagerTest, OnIncomingNotification);
FRIEND_TEST_ALL_PREFIXES(SyncManagerTest, PurgeDisabledTypes);
struct NotificationInfo {
NotificationInfo();
......
sync/internal_api/syncapi_unittest.cc
View file @ cb0a4762
......@@ -919,6 +919,22 @@ class SyncManagerTest : public testing::Test,
REMOTE_NOTIFICATION);
}
void SetProgressMarkerForType(ModelType type, bool set) {
if (set) {
sync_pb::DataTypeProgressMarker marker;
marker.set_token("token");
marker.set_data_type_id(GetSpecificsFieldNumberFromModelType(type));
sync_manager_.directory()->SetDownloadProgress(type, marker);
} else {
sync_pb::DataTypeProgressMarker marker;
sync_manager_.directory()->SetDownloadProgress(type, marker);
}
}
void SetInitialSyncEndedForType(ModelType type, bool value) {
sync_manager_.directory()->set_initial_sync_ended_for_type(type, value);
}
private:
// Needed by |sync_manager_|.
MessageLoop message_loop_;
......@@ -2502,18 +2518,24 @@ class MockSyncScheduler : public FakeSyncScheduler {
class ComponentsFactory : public TestInternalComponentsFactory {
public:
ComponentsFactory(SyncScheduler* scheduler_to_use)
ComponentsFactory(SyncScheduler* scheduler_to_use,
sessions::SyncSessionContext** session_context)
: TestInternalComponentsFactory(
TestInternalComponentsFactory::IN_MEMORY),
scheduler_to_use_(scheduler_to_use) {}
scheduler_to_use_(scheduler_to_use),
session_context_(session_context) {}
virtual ~ComponentsFactory() {}
virtual scoped_ptr<SyncScheduler> BuildScheduler(
const std::string& name,
sessions::SyncSessionContext* context) OVERRIDE {
*session_context_ = context;
return scheduler_to_use_.Pass();
}
private:
scoped_ptr<SyncScheduler> scheduler_to_use_;
sessions::SyncSessionContext** session_context_;
};
class SyncManagerTestWithMockScheduler : public SyncManagerTest {
......@@ -2521,24 +2543,42 @@ class SyncManagerTestWithMockScheduler : public SyncManagerTest {
SyncManagerTestWithMockScheduler() : scheduler_(NULL) {}
virtual InternalComponentsFactory* GetFactory() OVERRIDE {
scheduler_ = new MockSyncScheduler();
return new ComponentsFactory(scheduler_);
return new ComponentsFactory(scheduler_, &session_context_);
}
MockSyncScheduler* scheduler() { return scheduler_; }
sessions::SyncSessionContext* session_context() {
return session_context_;
}
private:
MockSyncScheduler* scheduler_;
sessions::SyncSessionContext* session_context_;
};
// Test that the configuration params are properly created and sent to
// ScheduleConfigure. No callback should be invoked.
// ScheduleConfigure. No callback should be invoked. Any disabled datatypes
// should be purged.
TEST_F(SyncManagerTestWithMockScheduler, BasicConfiguration) {
ConfigureReason reason = CONFIGURE_REASON_RECONFIGURATION;
ModelTypeSet types_to_download(BOOKMARKS, PREFERENCES);
ModelSafeRoutingInfo new_routing_info;
GetModelSafeRoutingInfo(&new_routing_info);
ModelTypeSet enabled_types = GetRoutingInfoTypes(new_routing_info);
ModelTypeSet disabled_types = Difference(ModelTypeSet::All(), enabled_types);
ConfigurationParams params;
EXPECT_CALL(*scheduler_, Start(SyncScheduler::CONFIGURATION_MODE));
EXPECT_CALL(*scheduler_, ScheduleConfiguration(_)).
EXPECT_CALL(*scheduler(), Start(SyncScheduler::CONFIGURATION_MODE));
EXPECT_CALL(*scheduler(), ScheduleConfiguration(_)).
WillOnce(DoAll(SaveArg<0>(&params), Return(true)));
// Set data for all types.
for (ModelTypeSet::Iterator iter = ModelTypeSet::All().First(); iter.Good();
iter.Inc()) {
SetProgressMarkerForType(iter.Get(), true);
SetInitialSyncEndedForType(iter.Get(), true);
}
CallbackCounter ready_task_counter, retry_task_counter;
sync_manager_.ConfigureSyncer(
reason,
......@@ -2554,6 +2594,70 @@ TEST_F(SyncManagerTestWithMockScheduler, BasicConfiguration) {
params.source);
EXPECT_TRUE(types_to_download.Equals(params.types_to_download));
EXPECT_EQ(new_routing_info, params.routing_info);
// Verify all the disabled types were purged.
EXPECT_TRUE(sync_manager_.InitialSyncEndedTypes().Equals(
enabled_types));
EXPECT_TRUE(sync_manager_.GetTypesWithEmptyProgressMarkerToken(
ModelTypeSet::All()).Equals(disabled_types));
}
// Test that on a reconfiguration (configuration where the session context
// already has routing info), only those recently disabled types are purged.
TEST_F(SyncManagerTestWithMockScheduler, ReConfiguration) {
ConfigureReason reason = CONFIGURE_REASON_RECONFIGURATION;
ModelTypeSet types_to_download(BOOKMARKS, PREFERENCES);
ModelTypeSet disabled_types = ModelTypeSet(THEMES, SESSIONS);
ModelSafeRoutingInfo old_routing_info;
ModelSafeRoutingInfo new_routing_info;
GetModelSafeRoutingInfo(&old_routing_info);
new_routing_info = old_routing_info;
new_routing_info.erase(THEMES);
new_routing_info.erase(SESSIONS);
ModelTypeSet enabled_types = GetRoutingInfoTypes(new_routing_info);
ConfigurationParams params;
EXPECT_CALL(*scheduler(), Start(SyncScheduler::CONFIGURATION_MODE));
EXPECT_CALL(*scheduler(), ScheduleConfiguration(_)).
WillOnce(DoAll(SaveArg<0>(&params), Return(true)));
// Set data for all types except those recently disabled (so we can verify
// only those recently disabled are purged) .
for (ModelTypeSet::Iterator iter = ModelTypeSet::All().First(); iter.Good();
iter.Inc()) {
if (!disabled_types.Has(iter.Get())) {
SetProgressMarkerForType(iter.Get(), true);
SetInitialSyncEndedForType(iter.Get(), true);
} else {
SetProgressMarkerForType(iter.Get(), false);
SetInitialSyncEndedForType(iter.Get(), false);
}
}
// Set the context to have the old routing info.
session_context()->set_routing_info(old_routing_info);
CallbackCounter ready_task_counter, retry_task_counter;
sync_manager_.ConfigureSyncer(
reason,
types_to_download,
new_routing_info,
base::Bind(&CallbackCounter::Callback,
base::Unretained(&ready_task_counter)),
base::Bind(&CallbackCounter::Callback,
base::Unretained(&retry_task_counter)));
EXPECT_EQ(0, ready_task_counter.times_called());
EXPECT_EQ(0, retry_task_counter.times_called());
EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::RECONFIGURATION,
params.source);
EXPECT_TRUE(types_to_download.Equals(params.types_to_download));
EXPECT_EQ(new_routing_info, params.routing_info);
// Verify only the recently disabled types were purged.
EXPECT_TRUE(sync_manager_.InitialSyncEndedTypes().Equals(
Difference(ModelTypeSet::All(), disabled_types)));
EXPECT_TRUE(sync_manager_.GetTypesWithEmptyProgressMarkerToken(
ModelTypeSet::All()).Equals(disabled_types));
}
// Test that the retry callback is invoked on configuration failure.
......@@ -2564,8 +2668,8 @@ TEST_F(SyncManagerTestWithMockScheduler, ConfigurationRetry) {
GetModelSafeRoutingInfo(&new_routing_info);
ConfigurationParams params;
EXPECT_CALL(*scheduler_, Start(SyncScheduler::CONFIGURATION_MODE));
EXPECT_CALL(*scheduler_, ScheduleConfiguration(_)).
EXPECT_CALL(*scheduler(), Start(SyncScheduler::CONFIGURATION_MODE));
EXPECT_CALL(*scheduler(), ScheduleConfiguration(_)).
WillOnce(DoAll(SaveArg<0>(&params), Return(false)));
CallbackCounter ready_task_counter, retry_task_counter;
......@@ -2627,4 +2731,44 @@ TEST_F(SyncManagerTest, PurgePartiallySyncedTypes) {
EXPECT_FALSE(partial_types.Has(PREFERENCES));
}
// Test CleanipDisabledTypes properly purges all disabled types as specified
// by the previous and current enabled params. Enabled partial types should not
// be purged.
TEST_F(SyncManagerTest, PurgeDisabledTypes) {
ModelSafeRoutingInfo routing_info;
GetModelSafeRoutingInfo(&routing_info);
ModelTypeSet enabled_types = GetRoutingInfoTypes(routing_info);
ModelTypeSet disabled_types = Difference(ModelTypeSet::All(), enabled_types);
ModelTypeSet partial_enabled_types(PASSWORDS);
// Set data for all non-partial types.
for (ModelTypeSet::Iterator iter = ModelTypeSet::All().First(); iter.Good();
iter.Inc()) {
SetProgressMarkerForType(iter.Get(), true);
if (!partial_enabled_types.Has(iter.Get()))
SetInitialSyncEndedForType(iter.Get(), true);
}
// Verify all the enabled types remain after cleanup, and all the disabled
// types were purged.
sync_manager_.PurgeDisabledTypes(ModelTypeSet::All(), enabled_types);
EXPECT_TRUE(enabled_types.Equals(
Union(sync_manager_.InitialSyncEndedTypes(), partial_enabled_types)));
EXPECT_TRUE(disabled_types.Equals(
sync_manager_.GetTypesWithEmptyProgressMarkerToken(ModelTypeSet::All())));
// Disable some more types.
disabled_types.Put(BOOKMARKS);
disabled_types.Put(PREFERENCES);
ModelTypeSet new_enabled_types =
Difference(ModelTypeSet::All(), disabled_types);
// Verify only the non-disabled types remain after cleanup.
sync_manager_.PurgeDisabledTypes(enabled_types, new_enabled_types);
EXPECT_TRUE(new_enabled_types.Equals(
Union(sync_manager_.InitialSyncEndedTypes(), partial_enabled_types)));
EXPECT_TRUE(disabled_types.Equals(
sync_manager_.GetTypesWithEmptyProgressMarkerToken(ModelTypeSet::All())));
}
} // namespace
sync/sessions/sync_session_context.h
View file @ cb0a4762
......@@ -109,14 +109,6 @@ class SyncSessionContext {
}
int32 max_commit_batch_size() const { return max_commit_batch_size_; }
const ModelSafeRoutingInfo& previous_session_routing_info() const {
return previous_session_routing_info_;
}
void set_previous_session_routing_info(const ModelSafeRoutingInfo& info) {
previous_session_routing_info_ = info;
}
void NotifyListeners(const SyncEngineEvent& event) {
FOR_EACH_OBSERVER(SyncEngineEventListener, listeners_,
OnSyncEngineEvent(event));
......@@ -162,10 +154,6 @@ class SyncSessionContext {
// The server limits the number of items a client can commit in one batch.
int max_commit_batch_size_;
// Some routing info history to help us clean up types that get disabled
// by the user.
ModelSafeRoutingInfo previous_session_routing_info_;
ThrottledDataTypeTracker* throttled_data_type_tracker_;
// We use this to get debug info to send to the server for debugging
......
sync/sync.gyp
View file @ cb0a4762
......@@ -76,8 +76,6 @@
'engine/apply_updates_command.h',
'engine/build_commit_command.cc',
'engine/build_commit_command.h',
'engine/cleanup_disabled_types_command.cc',
'engine/cleanup_disabled_types_command.h',
'engine/commit.cc',
'engine/commit.h',
'engine/conflict_resolver.cc',
......@@ -561,7 +559,6 @@
'internal_api/public/util/immutable_unittest.cc',
'engine/apply_updates_command_unittest.cc',
'engine/build_commit_command_unittest.cc',
'engine/cleanup_disabled_types_command_unittest.cc',
'engine/download_updates_command_unittest.cc',
'engine/model_changing_syncer_command_unittest.cc',
'engine/process_commit_response_command_unittest.cc',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment