r137482 - in branches/1132/src/chrome/browser: sync ui/webui

[atwi...@chromium.org]

Author: atwi...@chromium.org
Date: Wed May 16 12:29:59 2012
New Revision: 137482

Log:
Merge 136934 - Notify sync observers whenever preferred types change

Changed ProfileSyncService so any call to OnUserChoseDatatypes() will result in observers being notified even if configuration is aborted due to passphrase-required errors.

Also updated sync config UI so the "invalid passphrase" error is generated even for blank passphrases.

BUG=122734
TEST=Turn on explicit passphrase, setup sync, turn off passwords, click OK. Then go into settings and turn passwords on again, click OK, then cancel out of the dialog - wrench menu should have "passphrase error" badge.


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

TBR=atwi...@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10387158

Modified:
branches/1132/src/chrome/browser/sync/profile_sync_service.cc
branches/1132/src/chrome/browser/sync/profile_sync_service_harness.cc
branches/1132/src/chrome/browser/sync/profile_sync_service_harness.h
branches/1132/src/chrome/browser/ui/webui/sync_setup_handler.cc

Modified: branches/1132/src/chrome/browser/sync/profile_sync_service.cc
==============================================================================
--- branches/1132/src/chrome/browser/sync/profile_sync_service.cc (original)
+++ branches/1132/src/chrome/browser/sync/profile_sync_service.cc Wed May 16 12:29:59 2012
@@ -463,6 +463,10 @@
this,
chrome::NOTIFICATION_SYNC_CONFIGURE_DONE,
content::Source<DataTypeManager>(data_type_manager_.get()));
+ registrar_.Remove(
+ this,
+ chrome::NOTIFICATION_SYNC_CONFIGURE_BLOCKED,
+ content::Source<DataTypeManager>(data_type_manager_.get()));
data_type_manager_.reset();
}

@@ -1128,6 +1132,7 @@
failed_datatypes_handler_.OnUserChoseDatatypes();
ChangePreferredDataTypes(chosen_types);
AcknowledgeSyncedTypes();
+ NotifyObservers();
}

void ProfileSyncService::ChangePreferredDataTypes(
@@ -1193,6 +1198,9 @@
registrar_.Add(this,
chrome::NOTIFICATION_SYNC_CONFIGURE_DONE,
content::Source<DataTypeManager>(data_type_manager_.get()));
+ registrar_.Add(this,
+ chrome::NOTIFICATION_SYNC_CONFIGURE_BLOCKED,
+ content::Source<DataTypeManager>(data_type_manager_.get()));

// We create the migrator at the same time.
migrator_.reset(
@@ -1209,6 +1217,7 @@
// until we receive an OnPassphraseAccepted (which triggers a configure).
DVLOG(1) << "ProfileSyncService::ConfigureDataTypeManager bailing out "
<< "because a passphrase required";
+ NotifyObservers();
return;
}
sync_api::ConfigureReason reason = sync_api::CONFIGURE_REASON_UNKNOWN;
@@ -1390,11 +1399,11 @@
const content::NotificationSource& source,
const content::NotificationDetails& details) {
switch (type) {
- case chrome::NOTIFICATION_SYNC_CONFIGURE_START: {
+ case chrome::NOTIFICATION_SYNC_CONFIGURE_START:
+ case chrome::NOTIFICATION_SYNC_CONFIGURE_BLOCKED:
NotifyObservers();
// TODO(sync): Maybe toast?
break;
- }
case chrome::NOTIFICATION_SYNC_CONFIGURE_DONE: {
// We should have cleared our cached passphrase before we get here (in
// OnBackendInitialized()).

Modified: branches/1132/src/chrome/browser/sync/profile_sync_service_harness.cc
==============================================================================
--- branches/1132/src/chrome/browser/sync/profile_sync_service_harness.cc (original)
+++ branches/1132/src/chrome/browser/sync/profile_sync_service_harness.cc Wed May 16 12:29:59 2012
@@ -108,7 +108,8 @@
timestamp_match_partner_(NULL),
username_(username),
password_(password),
- profile_debug_name_(profile->GetDebugName()) {
+ profile_debug_name_(profile->GetDebugName()),
+ waiting_for_status_change_(false) {
if (IsSyncAlreadySetup()) {
service_ = ProfileSyncServiceFactory::GetInstance()->GetForProfile(
profile_);
@@ -262,7 +263,8 @@
}

void ProfileSyncServiceHarness::SignalStateComplete() {
- MessageLoop::current()->Quit();
+ if (waiting_for_status_change_)
+ MessageLoop::current()->Quit();
}

bool ProfileSyncServiceHarness::RunStateChangeMachine() {
@@ -730,6 +732,9 @@
scoped_refptr<StateChangeTimeoutEvent> timeout_signal(
new StateChangeTimeoutEvent(this, reason));
{
+ // Set the flag to tell SignalStateComplete() that it's OK to quit out of
+ // the MessageLoop if we hit a state transition.
+ waiting_for_status_change_ = true;
MessageLoop* loop = MessageLoop::current();
MessageLoop::ScopedNestableTaskAllower allow(loop);
loop->PostDelayedTask(
@@ -738,6 +743,7 @@
timeout_signal.get()),
base::TimeDelta::FromMilliseconds(timeout_milliseconds));
loop->Run();
+ waiting_for_status_change_ = false;
}

if (timeout_signal->Abort()) {
@@ -991,7 +997,7 @@
os << "has_more_to_sync: "
<< snap.has_more_to_sync()
<< ", has_unsynced_items: "
- << service()->HasUnsyncedItems()
+ << (service()->sync_initialized() ? service()->HasUnsyncedItems() : 0)
<< ", unsynced_count: "
<< snap.unsynced_count()
<< ", encryption conflicts: "

Modified: branches/1132/src/chrome/browser/sync/profile_sync_service_harness.h
==============================================================================
--- branches/1132/src/chrome/browser/sync/profile_sync_service_harness.h (original)
+++ branches/1132/src/chrome/browser/sync/profile_sync_service_harness.h Wed May 16 12:29:59 2012
@@ -359,6 +359,11 @@
// related bookkeeping information for verification.
browser_sync::RetryVerifier retry_verifier_;

+ // Flag set to true when we're waiting for a status change to happen. Used to
+ // avoid triggering internal state machine logic on unexpected sync observer
+ // callbacks.
+ bool waiting_for_status_change_;
+
DISALLOW_COPY_AND_ASSIGN(ProfileSyncServiceHarness);
};


Modified: branches/1132/src/chrome/browser/ui/webui/sync_setup_handler.cc
==============================================================================
--- branches/1132/src/chrome/browser/ui/webui/sync_setup_handler.cc (original)
+++ branches/1132/src/chrome/browser/ui/webui/sync_setup_handler.cc Wed May 16 12:29:59 2012
@@ -720,16 +720,31 @@
}
}

+ bool user_was_prompted_for_passphrase =
+ service->IsPassphraseRequiredForDecryption();
service->OnUserChoseDatatypes(configuration.sync_everything,
configuration.data_types);

// Need to call IsPassphraseRequiredForDecryption() *after* calling
// OnUserChoseDatatypes() because the user may have just disabled the
- // encrypted datatypes.
+ // encrypted datatypes (in which case we just want to exit, not prompt the
+ // user for a passphrase).
if (passphrase_failed || service->IsPassphraseRequiredForDecryption()) {
// We need a passphrase, or the user's attempt to set a passphrase failed -
- // prompt them again.
- DisplayConfigureSync(true, passphrase_failed);
+ // prompt them again. This covers a few subtle cases:
+ // 1) The user enters an incorrect passphrase *and* disabled the encrypted
+ // data types. In that case we want to notify the user that the
+ // passphrase was incorrect even though there are no longer any encrypted
+ // types enabled (IsPassphraseRequiredForDecryption() == false).
+ // 2) The user doesn't enter any passphrase. In this case, we won't call
+ // SetDecryptionPassphrase() (passphrase_failed == false), but we still
+ // want to display an error message to let the user know that their
+ // blank passphrase entry is not acceptable.
+ // 3) The user just enabled an encrypted data type - in this case we don't
+ // want to display an "invalid passphrase" error, since it's the first
+ // time the user is seeing the prompt.
+ DisplayConfigureSync(
+ true, passphrase_failed || user_was_prompted_for_passphrase);
} else {
// No passphrase is required from the user so mark the configuration as
// complete and close the sync setup overlay.