Make password generation switched by a preference in chrome settings rather than a command line fla…

chrome/browser/autofill/autofill_manager.cc

Index: chrome/browser/autofill/autofill_manager.cc
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 1ec53e74eceda2afed3dfe4ac9bb59493d443dd2..7a815b83f7ab2fc9f0d22a6a2a0845c212f4363d 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -189,12 +189,15 @@ AutofillManager::AutofillManager(TabContentsWrapper* tab_contents)
       user_did_type_(false),
       user_did_autofill_(false),
       user_did_edit_autofilled_field_(false),
+      password_sync_enabled_(false),
       password_generation_enabled_(false),
       external_delegate_(NULL) {
   // |personal_data_| is NULL when using test-enabled WebContents.
   personal_data_ = PersonalDataManagerFactory::GetForProfile(
       tab_contents->profile()->GetOriginalProfile());
   RegisterWithSyncService();
+  registrar_.Init(tab_contents->profile()->GetPrefs());
+  registrar_.Add(prefs::kPasswordGenerationEnabled, this);
 }
 
 AutofillManager::~AutofillManager() {
@@ -207,6 +210,9 @@ void AutofillManager::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterBooleanPref(prefs::kAutofillEnabled,
                              true,
                              PrefService::SYNCABLE_PREF);
+  prefs->RegisterBooleanPref(prefs::kPasswordGenerationEnabled,
+                             true,

[Garrett Casto, 2012/04/26 23:23:59]

We are going to have this be an opt-in feature at first, so this should default
to false.

[zysxqn, 2012/04/27 18:38:34]

Done.

+                             PrefService::SYNCABLE_PREF);
 #if defined(OS_MACOSX)
   prefs->RegisterBooleanPref(prefs::kAutofillAuxiliaryProfilesEnabled,
                              true,
@@ -234,25 +240,44 @@ void AutofillManager::RegisterWithSyncService() {
   }
 }
 
-void AutofillManager::SendPasswordGenerationStateToRenderer(
+void AutofillManager::SendPasswordSyncStateToRenderer(

[Garrett Casto, 2012/04/26 23:23:59]

Remove this.

     content::RenderViewHost* host, bool enabled) {
-  host->Send(new AutofillMsg_PasswordGenerationEnabled(host->GetRoutingID(),
+  host->Send(new AutofillMsg_PasswordSyncEnabled(host->GetRoutingID(),
                                                        enabled));
 }
 
-void AutofillManager::UpdatePasswordGenerationState(
+void AutofillManager::UpdatePasswordSyncState(
     content::RenderViewHost* host,
     bool new_renderer) {
   if (!sync_service_)
     return;
 
-  // Password generation requires sync for passwords and the password manager
-  // to both be enabled.
+  // Also need to check whether password manager is enabled.
   syncable::ModelTypeSet sync_set = sync_service_->GetPreferredDataTypes();
-  bool password_sync_enabled = (sync_service_->HasSyncSetupCompleted() &&
-                                sync_set.Has(syncable::PASSWORDS));
+  bool new_password_sync_enabled =
+      sync_service_->HasSyncSetupCompleted() &&
+      sync_set.Has(syncable::PASSWORDS) &&
+      tab_contents_wrapper_->password_manager()->IsEnabled();
+  if (new_password_sync_enabled != password_sync_enabled_ ||
+      new_renderer) {
+    password_sync_enabled_ = new_password_sync_enabled;
+    SendPasswordSyncStateToRenderer(host, password_sync_enabled_);
+  }
+}
+
+void AutofillManager::SendPasswordGenerationStateToRenderer(
+    content::RenderViewHost* host, bool enabled) {
+  host->Send(new AutofillMsg_PasswordGenerationEnabled(host->GetRoutingID(),
+                                                       enabled));
+}
+
+void AutofillManager::UpdatePasswordGenerationState(
+    content::RenderViewHost* host, bool new_renderer) {
+  Profile* profile = Profile::FromBrowserContext(
+      web_contents()->GetBrowserContext());
+  // Also need to check whether password manager is enabled.
   bool new_password_generation_enabled =
-      password_sync_enabled &&
+      profile->GetPrefs()->GetBoolean(prefs::kPasswordGenerationEnabled) &&

[Garrett Casto, 2012/04/26 23:23:59]

This should check if password sync is enabled as well.

       tab_contents_wrapper_->password_manager()->IsEnabled();
   if (new_password_generation_enabled != password_generation_enabled_ ||
       new_renderer) {
@@ -262,14 +287,32 @@ void AutofillManager::UpdatePasswordGenerationState(
 }
 
 void AutofillManager::RenderViewCreated(content::RenderViewHost* host) {
-      UpdatePasswordGenerationState(host, true);
+  UpdatePasswordSyncState(host, true);

[Garrett Casto, 2012/04/26 23:23:59]

This should just call UpdatePasswordGenerationState, not UpdatePasswordSyncState

+  UpdatePasswordGenerationState(host, true);
+}
+
+void AutofillManager::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  switch (type) {

[Garrett Casto, 2012/04/26 23:23:59]

I'm actually not totally sure that we need this observer. The password manager
for instance only checks if it's enabled/disabled when it's created.  Sync seems
different to me since if it's disabled, the feature will fail in a very bad way.
I guess since it's already here you can leave it.

[zysxqn, 2012/04/27 18:38:34]

I feel that it is useful since users sometimes expect to see the effect
immediately when they change some preference, so I added it like this. 

+    case chrome::NOTIFICATION_PREF_CHANGED: {
+      DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+      std::string* pref = content::Details<std::string>(details).ptr();
+      DCHECK(*pref == prefs::kPasswordGenerationEnabled);
+      UpdatePasswordGenerationState(web_contents()->GetRenderViewHost(), false);
+      break;
+    }
+    default:
+      NOTREACHED();
+  }
 }
 
 void AutofillManager::OnStateChanged() {
   // It is possible for sync state to change during tab contents destruction.
   // In this case, we don't need to update the renderer since it's going away.
   if (web_contents() && web_contents()->GetRenderViewHost()) {
-    UpdatePasswordGenerationState(web_contents()->GetRenderViewHost(),
+    UpdatePasswordSyncState(web_contents()->GetRenderViewHost(),

[Garrett Casto, 2012/04/26 23:23:59]

Don't change this.

                                   false);
   }
 }
@@ -839,10 +882,12 @@ AutofillManager::AutofillManager(TabContentsWrapper* tab_contents,
       user_did_type_(false),
       user_did_autofill_(false),
       user_did_edit_autofilled_field_(false),
+      password_sync_enabled_(false),
       password_generation_enabled_(false),
       external_delegate_(NULL) {
   DCHECK(tab_contents);
   RegisterWithSyncService();
+  // Test code doens't need registrar_.
 }
 
 void AutofillManager::set_metric_logger(const AutofillMetrics* metric_logger) {