Skip to content

Commit

Permalink
Revert 78749 - sync: don't send SYNC_CONFIGURE_DONE unless it is true
Browse files Browse the repository at this point in the history 
Adds a BLOCKED state to DataTypeManager to handle waiting on encryption.
Removes current_dtc_ from DTM because it's redundant.

(Note that DTMImpl will soon be obsoleted by Impl2)

BUG=76135,76233,76258
TEST=DataTypeManagerImpl[2]Test.*

Review URL: http://codereview.chromium.org/6697039

TBR=tim@chromium.org
Review URL: http://codereview.chromium.org/6713051

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@78757 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
tim@chromium.org committed Mar 18, 2011
1 parent 0b15c55 commit 421f3ae
Show file tree
Hide file tree
Showing 17 changed files with 146 additions and 209 deletions.
2 changes: 0 additions & 2 deletions chrome/browser/sync/glue/data_type_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ class DataTypeManager {
// TODO(tim): Deprecated. Bug 26339.
PAUSE_PENDING, // Waiting for the sync backend to pause.
CONFIGURING, // Data types are being started.
BLOCKED, // We can't move forward with configuration because some
// external action must take place (i.e. passphrase).

// TODO(tim): Deprecated. Bug 26339.
RESUME_PENDING, // Waiting for the sync backend to resume.
Expand Down
76 changes: 26 additions & 50 deletions chrome/browser/sync/glue/data_type_manager_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ DataTypeManagerImpl::DataTypeManagerImpl(
: backend_(backend),
controllers_(controllers),
state_(DataTypeManager::STOPPED),
current_dtc_(NULL),
pause_pending_(false),
syncer_paused_(false),
ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK(backend_);
Expand All @@ -76,25 +76,6 @@ DataTypeManagerImpl::DataTypeManagerImpl(
DataTypeManagerImpl::~DataTypeManagerImpl() {
}

bool DataTypeManagerImpl::GetControllersNeedingStart(
std::vector<DataTypeController*>* needs_start) {
// Add any data type controllers into the needs_start_ list that are
// currently NOT_RUNNING or STOPPING.
bool found_any = false;
for (TypeSet::const_iterator it = last_requested_types_.begin();
it != last_requested_types_.end(); ++it) {
DataTypeController::TypeMap::const_iterator dtc = controllers_.find(*it);
if (dtc != controllers_.end() &&
(dtc->second->state() == DataTypeController::NOT_RUNNING ||
dtc->second->state() == DataTypeController::STOPPING)) {
found_any = true;
if (needs_start)
needs_start->push_back(dtc->second.get());
}
}
return found_any;
}

void DataTypeManagerImpl::Configure(const TypeSet& desired_types) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (state_ == STOPPING) {
Expand All @@ -104,8 +85,19 @@ void DataTypeManagerImpl::Configure(const TypeSet& desired_types) {
}

last_requested_types_ = desired_types;
// Add any data type controllers into the needs_start_ list that are
// currently NOT_RUNNING or STOPPING.
needs_start_.clear();
GetControllersNeedingStart(&needs_start_);
for (TypeSet::const_iterator it = desired_types.begin();
it != desired_types.end(); ++it) {
DataTypeController::TypeMap::const_iterator dtc = controllers_.find(*it);
if (dtc != controllers_.end() &&
(dtc->second->state() == DataTypeController::NOT_RUNNING ||
dtc->second->state() == DataTypeController::STOPPING)) {
needs_start_.push_back(dtc->second.get());
VLOG(1) << "Will start " << dtc->second->name();
}
}
// Sort these according to kStartOrder.
std::sort(needs_start_.begin(),
needs_start_.end(),
Expand Down Expand Up @@ -158,8 +150,8 @@ void DataTypeManagerImpl::Restart() {
<< " configuring.";
return;
}
DCHECK(state_ == STOPPED || state_ == RESTARTING || state_ == CONFIGURED ||
state_ == BLOCKED);
DCHECK(state_ == STOPPED || state_ == RESTARTING || state_ == CONFIGURED);
current_dtc_ = NULL;

// Starting from a "steady state" (stopped or configured) state
// should send a start notification.
Expand Down Expand Up @@ -224,12 +216,6 @@ void DataTypeManagerImpl::DownloadReady() {
return;
}

if (syncer_paused_) {
state_ = CONFIGURING;
StartNextType();
return;
}

// Pause the sync backend before starting the data types.
state_ = PAUSE_PENDING;
PauseSyncer();
Expand All @@ -239,24 +225,16 @@ void DataTypeManagerImpl::StartNextType() {
// If there are any data types left to start, start the one at the
// front of the list.
if (!needs_start_.empty()) {
VLOG(1) << "Starting " << needs_start_[0]->name();
needs_start_[0]->Start(
current_dtc_ = needs_start_[0];
VLOG(1) << "Starting " << current_dtc_->name();
current_dtc_->Start(
NewCallback(this, &DataTypeManagerImpl::TypeStartCallback));
return;
}

// If no more data types need starting, we're done. Resume the sync
// backend to finish.
DCHECK_EQ(state_, CONFIGURING);

// Do a fresh calculation to see if controllers need starting to account for
// things like encryption, which may still need to be sorted out before we
// can announce we're "Done" configuration entirely.
if (GetControllersNeedingStart(NULL)) {
state_ = BLOCKED;
return;
}

// If no more data types need starting, we're done. Resume the sync backend
// to finish.
state_ = RESUME_PENDING;
ResumeSyncer();
}
Expand All @@ -266,6 +244,7 @@ void DataTypeManagerImpl::TypeStartCallback(
// When the data type controller invokes this callback, it must be
// on the UI thread.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK(current_dtc_);

// If configuration changed while this data type was starting, we
// need to reset. Resume the syncer.
Expand All @@ -275,10 +254,11 @@ void DataTypeManagerImpl::TypeStartCallback(
}

// We're done with the data type at the head of the list -- remove it.
DataTypeController* started_dtc = needs_start_[0];
DataTypeController* started_dtc = current_dtc_;
DCHECK(needs_start_.size());
DCHECK_EQ(needs_start_[0], started_dtc);
needs_start_.erase(needs_start_.begin());
current_dtc_ = NULL;

// If we reach this callback while stopping, this means that
// DataTypeManager::Stop() was called while the current data type
Expand Down Expand Up @@ -344,9 +324,7 @@ void DataTypeManagerImpl::Stop() {
// which will synchronously invoke the start callback.
if (state_ == CONFIGURING) {
state_ = STOPPING;

DCHECK_LT(0U, needs_start_.size());
needs_start_[0]->Stop();
current_dtc_->Stop();

// By this point, the datatype should have invoked the start callback,
// triggering FinishStop to be called, and the state to reach STOPPED. If we
Expand Down Expand Up @@ -398,8 +376,7 @@ void DataTypeManagerImpl::FinishStop() {
DCHECK(state_== CONFIGURING ||
state_ == STOPPING ||
state_ == PAUSE_PENDING ||
state_ == RESUME_PENDING ||
state_ == BLOCKED);
state_ == RESUME_PENDING);
// Simply call the Stop() method on all running data types.
for (DataTypeController::TypeMap::const_iterator it = controllers_.begin();
it != controllers_.end(); ++it) {
Expand Down Expand Up @@ -428,7 +405,7 @@ void DataTypeManagerImpl::Observe(NotificationType type,
DCHECK(state_ == PAUSE_PENDING || state_ == RESTARTING);
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
pause_pending_ = false;
syncer_paused_ = true;

RemoveObserver(NotificationType::SYNC_PAUSED);

// If the state changed to RESTARTING while waiting to be
Expand All @@ -444,7 +421,6 @@ void DataTypeManagerImpl::Observe(NotificationType type,
case NotificationType::SYNC_RESUMED:
DCHECK(state_ == RESUME_PENDING || state_ == RESTARTING);
RemoveObserver(NotificationType::SYNC_RESUMED);
syncer_paused_ = false;

// If we are resuming because of a restart, continue the restart.
if (state_ == RESTARTING) {
Expand Down
9 changes: 1 addition & 8 deletions chrome/browser/sync/glue/data_type_manager_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,6 @@ class DataTypeManagerImpl : public DataTypeManager,
void FinishStop();
void FinishStopAndNotify(ConfigureResult result);

// Returns true if any last_requested_types_ currently needs to start model
// association. If non-null, fills |needs_start| with all such controllers.
bool GetControllersNeedingStart(
std::vector<DataTypeController*>* needs_start);

void Restart();
void DownloadReady();
void AddObserver(NotificationType type);
Expand All @@ -78,6 +73,7 @@ class DataTypeManagerImpl : public DataTypeManager,
// This list is determined at startup by various command line flags.
const DataTypeController::TypeMap controllers_;
State state_;
DataTypeController* current_dtc_;
std::map<syncable::ModelType, int> start_order_;
TypeSet last_requested_types_;
std::vector<DataTypeController*> needs_start_;
Expand All @@ -88,9 +84,6 @@ class DataTypeManagerImpl : public DataTypeManager,
// PAUSE_PENDING state. See http://crbug.com/73218.
bool pause_pending_;

// Whether we've observed a SYNC_PAUSED but not SYNC_RESUMED.
bool syncer_paused_;

NotificationRegistrar notification_registrar_;
ScopedRunnableMethodFactory<DataTypeManagerImpl> method_factory_;

Expand Down
63 changes: 24 additions & 39 deletions chrome/browser/sync/glue/data_type_manager_impl2.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ DataTypeManagerImpl2::DataTypeManagerImpl2(SyncBackendHost* backend,
: backend_(backend),
controllers_(controllers),
state_(DataTypeManager::STOPPED),
current_dtc_(NULL),
method_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
DCHECK(backend_);
// Ensure all data type controllers are stopped.
Expand All @@ -74,25 +75,6 @@ DataTypeManagerImpl2::DataTypeManagerImpl2(SyncBackendHost* backend,

DataTypeManagerImpl2::~DataTypeManagerImpl2() {}

bool DataTypeManagerImpl2::GetControllersNeedingStart(
std::vector<DataTypeController*>* needs_start) {
// Add any data type controllers into the needs_start_ list that are
// currently NOT_RUNNING or STOPPING.
bool found_any = false;
for (TypeSet::const_iterator it = last_requested_types_.begin();
it != last_requested_types_.end(); ++it) {
DataTypeController::TypeMap::const_iterator dtc = controllers_.find(*it);
if (dtc != controllers_.end() &&
(dtc->second->state() == DataTypeController::NOT_RUNNING ||
dtc->second->state() == DataTypeController::STOPPING)) {
found_any = true;
if (needs_start)
needs_start->push_back(dtc->second.get());
}
}
return found_any;
}

void DataTypeManagerImpl2::Configure(const TypeSet& desired_types) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (state_ == STOPPING) {
Expand All @@ -102,8 +84,19 @@ void DataTypeManagerImpl2::Configure(const TypeSet& desired_types) {
}

last_requested_types_ = desired_types;
// Add any data type controllers into the needs_start_ list that are
// currently NOT_RUNNING or STOPPING.
needs_start_.clear();
GetControllersNeedingStart(&needs_start_);
for (TypeSet::const_iterator it = desired_types.begin();
it != desired_types.end(); ++it) {
DataTypeController::TypeMap::const_iterator dtc = controllers_.find(*it);
if (dtc != controllers_.end() &&
(dtc->second->state() == DataTypeController::NOT_RUNNING ||
dtc->second->state() == DataTypeController::STOPPING)) {
needs_start_.push_back(dtc->second.get());
VLOG(1) << "Will start " << dtc->second->name();
}
}
// Sort these according to kStartOrder.
std::sort(needs_start_.begin(),
needs_start_.end(),
Expand Down Expand Up @@ -150,8 +143,8 @@ void DataTypeManagerImpl2::Restart() {
return;
}

DCHECK(state_ == STOPPED || state_ == RESTARTING || state_ == CONFIGURED ||
state_ == BLOCKED);
DCHECK(state_ == STOPPED || state_ == RESTARTING || state_ == CONFIGURED);
current_dtc_ = NULL;

// Starting from a "steady state" (stopped or configured) state
// should send a start notification.
Expand Down Expand Up @@ -194,23 +187,15 @@ void DataTypeManagerImpl2::StartNextType() {
// If there are any data types left to start, start the one at the
// front of the list.
if (!needs_start_.empty()) {
VLOG(1) << "Starting " << needs_start_[0]->name();
needs_start_[0]->Start(
current_dtc_ = needs_start_[0];
VLOG(1) << "Starting " << current_dtc_->name();
current_dtc_->Start(
NewCallback(this, &DataTypeManagerImpl2::TypeStartCallback));
return;
}

DCHECK_EQ(state_, CONFIGURING);

// Do a fresh calculation to see if controllers need starting to account for
// things like encryption, which may still need to be sorted out before we
// can announce we're "Done" configuration entirely.
if (GetControllersNeedingStart(NULL)) {
state_ = BLOCKED;
return;
}

// If no more data types need starting, we're done.
DCHECK_EQ(state_, CONFIGURING);
state_ = CONFIGURED;
NotifyDone(OK);
}
Expand All @@ -220,6 +205,7 @@ void DataTypeManagerImpl2::TypeStartCallback(
// When the data type controller invokes this callback, it must be
// on the UI thread.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK(current_dtc_);

if (state_ == RESTARTING) {
// If configuration changed while this data type was starting, we
Expand All @@ -241,10 +227,11 @@ void DataTypeManagerImpl2::TypeStartCallback(
}

// We're done with the data type at the head of the list -- remove it.
DataTypeController* started_dtc = needs_start_[0];
DataTypeController* started_dtc = current_dtc_;
DCHECK(needs_start_.size());
DCHECK_EQ(needs_start_[0], started_dtc);
needs_start_.erase(needs_start_.begin());
current_dtc_ = NULL;

// If the type started normally, continue to the next type.
// If the type is waiting for the cryptographer, continue to the next type.
Expand Down Expand Up @@ -288,9 +275,7 @@ void DataTypeManagerImpl2::Stop() {
// which will synchronously invoke the start callback.
if (state_ == CONFIGURING) {
state_ = STOPPING;

DCHECK_LT(0U, needs_start_.size());
needs_start_[0]->Stop();
current_dtc_->Stop();

// By this point, the datatype should have invoked the start callback,
// triggering FinishStop to be called, and the state to reach STOPPED. If we
Expand All @@ -314,7 +299,7 @@ void DataTypeManagerImpl2::Stop() {
}

void DataTypeManagerImpl2::FinishStop() {
DCHECK(state_== CONFIGURING || state_ == STOPPING || state_ == BLOCKED);
DCHECK(state_== CONFIGURING || state_ == STOPPING);
// Simply call the Stop() method on all running data types.
for (DataTypeController::TypeMap::const_iterator it = controllers_.begin();
it != controllers_.end(); ++it) {
Expand Down
6 changes: 1 addition & 5 deletions chrome/browser/sync/glue/data_type_manager_impl2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ class DataTypeManagerImpl2 : public DataTypeManager {
void FinishStop();
void FinishStopAndNotify(ConfigureResult result);

// Returns true if any last_requested_types_ currently needs to start model
// association. If non-null, fills |needs_start| with all such controllers.
bool GetControllersNeedingStart(
std::vector<DataTypeController*>* needs_start);

void Restart();
void DownloadReady();
void NotifyStart();
Expand All @@ -59,6 +54,7 @@ class DataTypeManagerImpl2 : public DataTypeManager {
// This list is determined at startup by various command line flags.
const DataTypeController::TypeMap controllers_;
State state_;
DataTypeController* current_dtc_;
std::map<syncable::ModelType, int> start_order_;
TypeSet last_requested_types_;
std::vector<DataTypeController*> needs_start_;
Expand Down
Loading

0 comments on commit 421f3ae

Please sign in to comment.