summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/sync/engine/syncapi.cc2
-rw-r--r--chrome/browser/sync/glue/sync_backend_host.cc33
-rw-r--r--chrome/browser/sync/glue/sync_backend_host.h10
-rw-r--r--chrome/browser/sync/profile_sync_service.cc70
-rw-r--r--chrome/browser/sync/profile_sync_service.h11
-rw-r--r--chrome/browser/sync/profile_sync_service_password_unittest.cc2
6 files changed, 85 insertions, 43 deletions
diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc
index f91d0f8..dabb926 100644
--- a/chrome/browser/sync/engine/syncapi.cc
+++ b/chrome/browser/sync/engine/syncapi.cc
@@ -1863,7 +1863,7 @@ void SyncManager::SyncInternal::HandleChannelEvent(const SyncerEvent& event) {
sync_api::ReadTransaction trans(GetUserShare());
sync_api::ReadNode node(&trans);
if (!node.InitByTagLookup(kNigoriTag)) {
- NOTREACHED();
+ DCHECK(!event.snapshot->is_share_usable);
return;
}
const sync_pb::NigoriSpecifics& nigori = node.GetNigoriSpecifics();
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index 74a8c47..05cbd7b 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -22,6 +22,8 @@
#include "chrome/browser/sync/glue/http_bridge.h"
#include "chrome/browser/sync/glue/password_model_worker.h"
#include "chrome/browser/sync/sessions/session_state.h"
+// TODO(tim): Remove this! We should have a syncapi pass-thru instead.
+#include "chrome/browser/sync/syncable/directory_manager.h" // Cryptographer.
#include "chrome/common/chrome_switches.h"
#include "chrome/common/chrome_version_info.h"
#include "chrome/common/net/gaia/gaia_constants.h"
@@ -115,12 +117,13 @@ void SyncBackendHost::Initialize(
registrar_.routing_info[(*it)] = GROUP_PASSIVE;
}
- // TODO(tim): This should be encryption-specific instead of passwords
- // specific. For now we have to do this to avoid NIGORI node lookups when
- // we haven't downloaded that node.
- if (profile_->GetPrefs()->GetBoolean(prefs::kSyncPasswords)) {
+ // TODO(tim): Remove this special case once NIGORI is populated by
+ // default. We piggy back off of the passwords flag for now to not
+ // require both encryption and passwords flags.
+ bool enable_encryption = CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kEnableSyncPasswords) || types.count(syncable::PASSWORDS);
+ if (enable_encryption)
registrar_.routing_info[syncable::NIGORI] = GROUP_PASSIVE;
- }
InitCore(Core::DoInitializeOptions(
sync_service_url,
@@ -146,6 +149,20 @@ std::string SyncBackendHost::RestoreEncryptionBootstrapToken() {
return token;
}
+bool SyncBackendHost::IsNigoriEnabled() const {
+ AutoLock lock(registrar_lock_);
+ // Note that NIGORI is only ever added/removed from routing_info once,
+ // during initialization / first configuration, so there is no real 'race'
+ // possible here or possibility of stale return value.
+ return registrar_.routing_info.find(syncable::NIGORI) !=
+ registrar_.routing_info.end();
+}
+
+bool SyncBackendHost::IsCryptographerReady() const {
+ return syncapi_initialized_ &&
+ GetUserShareHandle()->dir_manager->cryptographer()->is_ready();
+}
+
sync_api::HttpPostProviderFactory* SyncBackendHost::MakeHttpBridgeFactory(
URLRequestContextGetter* getter) {
return new HttpBridgeFactory(getter);
@@ -170,6 +187,12 @@ void SyncBackendHost::StartSyncingWithServer() {
}
void SyncBackendHost::SetPassphrase(const std::string& passphrase) {
+ if (!IsNigoriEnabled()) {
+ LOG(WARNING) << "Silently dropping SetPassphrase request.";
+ return;
+ }
+
+ // If encryption is enabled and we've got a SetPassphrase
core_thread_.message_loop()->PostTask(FROM_HERE,
NewRunnableMethod(core_.get(), &SyncBackendHost::Core::DoSetPassphrase,
passphrase));
diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index d18bf26d..26d060a 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -191,6 +191,14 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
// ONLY CALL THIS IF OnInitializationComplete was called!
bool HasUnsyncedItems() const;
+ // Whether or not we are syncing encryption keys.
+ bool IsNigoriEnabled() const;
+
+ // True if the cryptographer has any keys available to attempt decryption.
+ // Could mean we've downloaded and loaded Nigori objects, or we bootstrapped
+ // using a token previously received.
+ bool IsCryptographerReady() const;
+
protected:
// The real guts of SyncBackendHost, to keep the public client API clean.
class Core : public base::RefCountedThreadSafe<SyncBackendHost::Core>,
@@ -463,7 +471,7 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
// pointer value", and then invoke methods), because lifetimes are managed on
// the UI thread. Of course, this comment only applies to ModelSafeWorker
// impls that are themselves thread-safe, such as UIModelWorker.
- Lock registrar_lock_;
+ mutable Lock registrar_lock_;
// The frontend which we serve (and are owned by).
SyncFrontend* frontend_;
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 9788f2c..01f03bb 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -23,16 +23,13 @@
#include "chrome/browser/prefs/pref_service.h"
#include "chrome/browser/profile.h"
#include "chrome/browser/net/gaia/token_service.h"
-#include "chrome/browser/sync/engine/syncapi.h"
#include "chrome/browser/sync/glue/change_processor.h"
#include "chrome/browser/sync/glue/data_type_controller.h"
#include "chrome/browser/sync/glue/data_type_manager.h"
#include "chrome/browser/sync/glue/session_data_type_controller.h"
#include "chrome/browser/sync/profile_sync_factory.h"
#include "chrome/browser/sync/signin_manager.h"
-#include "chrome/browser/sync/syncable/directory_manager.h"
#include "chrome/browser/sync/token_migrator.h"
-#include "chrome/browser/sync/util/user_settings.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/net/gaia/gaia_constants.h"
#include "chrome/common/notification_details.h"
@@ -73,6 +70,7 @@ ProfileSyncService::ProfileSyncService(ProfileSyncFactory* factory,
unrecoverable_error_detected_(false),
ALLOW_THIS_IN_INITIALIZER_LIST(scoped_runnable_method_factory_(this)),
token_migrator_(NULL),
+ observed_passphrase_required_(false),
clear_server_data_state_(CLEAR_NOT_STARTED) {
DCHECK(factory);
DCHECK(profile);
@@ -307,7 +305,7 @@ void ProfileSyncService::RegisterPreferences() {
#endif
pref_service->RegisterBooleanPref(prefs::kSyncBookmarks, true);
- pref_service->RegisterBooleanPref(prefs::kSyncPasswords, false);
+ pref_service->RegisterBooleanPref(prefs::kSyncPasswords, enable_by_default);
pref_service->RegisterBooleanPref(prefs::kSyncPreferences, enable_by_default);
pref_service->RegisterBooleanPref(prefs::kSyncAutofill, enable_by_default);
pref_service->RegisterBooleanPref(prefs::kSyncThemes, enable_by_default);
@@ -408,26 +406,40 @@ void ProfileSyncService::StartUp() {
void ProfileSyncService::Shutdown(bool sync_disabled) {
// Stop all data type controllers, if needed.
- if (data_type_manager_.get() &&
- data_type_manager_->state() != DataTypeManager::STOPPED) {
- data_type_manager_->Stop();
- }
+ if (data_type_manager_.get()) {
+ if (data_type_manager_->state() != DataTypeManager::STOPPED) {
+ data_type_manager_->Stop();
+ }
- data_type_manager_.reset();
+ registrar_.Remove(this,
+ NotificationType::SYNC_CONFIGURE_START,
+ Source<DataTypeManager>(data_type_manager_.get()));
+ registrar_.Remove(this,
+ NotificationType::SYNC_CONFIGURE_DONE,
+ Source<DataTypeManager>(data_type_manager_.get()));
+ data_type_manager_.reset();
+ }
// Move aside the backend so nobody else tries to use it while we are
// shutting it down.
scoped_ptr<SyncBackendHost> doomed_backend(backend_.release());
-
- if (doomed_backend.get())
+ if (doomed_backend.get()) {
doomed_backend->Shutdown(sync_disabled);
- doomed_backend.reset();
+ registrar_.Remove(this,
+ NotificationType::SYNC_PASSPHRASE_REQUIRED,
+ Source<SyncBackendHost>(doomed_backend.get()));
+ registrar_.Remove(this,
+ NotificationType::SYNC_PASSPHRASE_ACCEPTED,
+ Source<SyncBackendHost>(doomed_backend.get()));
+ doomed_backend.reset();
+ }
// Clear various flags.
is_auth_in_progress_ = false;
backend_initialized_ = false;
+ observed_passphrase_required_ = false;
last_attempted_user_email_.clear();
}
@@ -819,24 +831,7 @@ void ProfileSyncService::GetRegisteredDataTypes(
}
bool ProfileSyncService::IsCryptographerReady() const {
- return backend_.get() && backend_initialized_ &&
- backend_->GetUserShareHandle()->dir_manager->cryptographer()->is_ready();
-}
-
-void ProfileSyncService::SetPassphrase(const std::string& passphrase) {
- // TODO(tim): This should be encryption-specific instead of passwords
- // specific. For now we have to do this to avoid NIGORI node lookups when
- // we haven't downloaded that node.
- if (!profile_->GetPrefs()->GetBoolean(prefs::kSyncPasswords)) {
- LOG(WARNING) << "Silently dropping SetPassphrase request.";
- return;
- }
-
- if (!sync_initialized()) {
- cached_passphrase_ = passphrase;
- } else {
- backend_->SetPassphrase(passphrase);
- }
+ return backend_.get() && backend_->IsCryptographerReady();
}
void ProfileSyncService::ConfigureDataTypeManager() {
@@ -877,6 +872,14 @@ void ProfileSyncService::DeactivateDataType(
backend_->DeactivateDataType(data_type_controller, change_processor);
}
+void ProfileSyncService::SetPassphrase(const std::string& passphrase) {
+ if (ShouldPushChanges() || observed_passphrase_required_) {
+ backend_->SetPassphrase(passphrase);
+ } else {
+ cached_passphrase_ = passphrase;
+ }
+}
+
void ProfileSyncService::Observe(NotificationType type,
const NotificationSource& source,
const NotificationDetails& details) {
@@ -913,6 +916,9 @@ void ProfileSyncService::Observe(NotificationType type,
}
case NotificationType::SYNC_PASSPHRASE_REQUIRED: {
DCHECK(backend_.get());
+ DCHECK(backend_->IsNigoriEnabled());
+ observed_passphrase_required_ = true;
+
if (!cached_passphrase_.empty()) {
SetPassphrase(cached_passphrase_);
cached_passphrase_.clear();
@@ -955,8 +961,8 @@ void ProfileSyncService::Observe(NotificationType type,
break;
}
case NotificationType::GOOGLE_SIGNIN_SUCCESSFUL: {
- if (!profile_->GetPrefs()->GetBoolean(
- prefs::kSyncUsingSecondaryPassphrase)) {
+ PrefService* prefs = profile_->GetPrefs();
+ if (!prefs->GetBoolean(prefs::kSyncUsingSecondaryPassphrase)) {
const GoogleServiceSigninSuccessDetails* successful =
(Details<const GoogleServiceSigninSuccessDetails>(details).ptr());
SetPassphrase(successful->password);
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index dc3e67d..f0a3db2 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -328,9 +328,10 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
// for sensitive data types.
virtual bool IsCryptographerReady() const;
- // Sets the Cryptographer's passphrase. This will check asynchronously whether
- // the passphrase is valid and notify ProfileSyncServiceObservers via the
- // NotificationService when the outcome is known.
+ // Sets the Cryptographer's passphrase, or caches it until that is possible.
+ // This will check asynchronously whether the passphrase is valid and notify
+ // ProfileSyncServiceObservers via the NotificationService when the outcome
+ // is known.
virtual void SetPassphrase(const std::string& passphrase);
// Returns whether processing changes is allowed. Check this before doing
@@ -484,6 +485,10 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
// credentials were provided.
std::string cached_passphrase_;
+ // Whether we have seen a SYNC_PASSPHRASE_REQUIRED since initializing the
+ // backend, telling us that it is safe to send a passphrase down ASAP.
+ bool observed_passphrase_required_;
+
// Keep track of where we are in a server clear operation
ClearServerDataState clear_server_data_state_;
diff --git a/chrome/browser/sync/profile_sync_service_password_unittest.cc b/chrome/browser/sync/profile_sync_service_password_unittest.cc
index d0598c5..a355c60 100644
--- a/chrome/browser/sync/profile_sync_service_password_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_password_unittest.cc
@@ -144,7 +144,7 @@ class ProfileSyncServicePasswordTest : public AbstractProfileSyncServiceTest {
service_.reset(new TestProfileSyncService(&factory_, &profile_,
"test_user", false, root_task));
service_->RegisterPreferences();
- profile_.GetPrefs()->SetBoolean(prefs::kSyncPasswords, true);
+ profile_.GetPrefs()->SetBoolean(prefs::kSyncPasswords, true);
service_->set_num_expected_resumes(num_resume_expectations);
service_->set_num_expected_pauses(num_pause_expectations);
PasswordDataTypeController* data_type_controller =