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

chrome/browser/autofill/autofill_manager.h

Index: chrome/browser/autofill/autofill_manager.h
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index d0045f2fac945675aeff3ef132da7b2c08bf3be7..ec20eefceb720cc91b15dee010a3d93c735ddd8b 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -23,7 +23,9 @@
 #include "chrome/browser/autofill/autofill_download.h"
 #include "chrome/browser/autofill/field_types.h"
 #include "chrome/browser/autofill/form_structure.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
 #include "chrome/browser/sync/profile_sync_service_observer.h"
+#include "content/public/browser/notification_observer.h"
 #include "content/public/browser/web_contents_observer.h"
 
 class AutofillExternalDelegate;
@@ -60,7 +62,8 @@ struct PasswordFormFillData;
 
 // Manages saving and restoring the user's personal information entered into web
 // forms.
-class AutofillManager : public content::WebContentsObserver,
+class AutofillManager : public content::NotificationObserver,
+                        public content::WebContentsObserver,
                         public AutofillDownloadManager::Observer,
                         public ProfileSyncServiceObserver,
                         public base::RefCounted<AutofillManager> {
@@ -115,12 +118,13 @@ class AutofillManager : public content::WebContentsObserver,
   // Reset cache.
   void Reset();
 
-  // Informs the renderer of the current password generation state. This is a
-  // separate function to aid with testing.
+  // Informs the renderers of the current password generation state for use
+  // in password generation. This is a separate function to aid with testing.

[Ilya Sherman, 2012/05/01 22:27:48]

nit: Please remove "for use in password generation" -- it is redundant.

[zysxqn, 2012/05/03 00:57:24]

Done.

   virtual void SendPasswordGenerationStateToRenderer(
       content::RenderViewHost* host,
       bool enabled);
 
+

[Ilya Sherman, 2012/05/01 22:27:48]

nit: Please remove this extra blank line.

[zysxqn, 2012/05/03 00:57:24]

Done.

   // Logs quality metrics for the |submitted_form| and uploads the form data
   // to the crowdsourcing server, if appropriate.
   virtual void UploadFormDataAsyncCallback(
@@ -173,8 +177,13 @@ class AutofillManager : public content::WebContentsObserver,
   // Register as an observer with the sync service.
   void RegisterWithSyncService();
 
-  // Determines what the current state of password generation is, and if it has
-  // changed from |password_generation_enabled_|. If it has changed or if
+  // content::NotificationObserver override
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  // Determines that the current state of password generation is, and if it

[Ilya Sherman, 2012/05/01 22:27:48]

nit: "that" -> "what"

[zysxqn, 2012/05/03 00:57:24]

Done.

+  // has changed from password_generation_enabled_. If it has changed or if
   // |new_renderer| is true, it notifies the renderer of this change via
   // SendPasswordGenerationStateToRenderer.
   void UpdatePasswordGenerationState(content::RenderViewHost* host,
@@ -337,13 +346,15 @@ class AutofillManager : public content::WebContentsObserver,
   // When the user first interacted with a potentially fillable form on this
   // page.
   base::TimeTicks initial_interaction_timestamp_;
-  // If password generation is enabled. We cache this value so that we don't
-  // spam the renderer with messages during startup when the sync state
-  // is changing rapidly.
-  bool password_generation_enabled_;
   // The ProfileSyncService associated with this tab. This may be NULL in
   // testing.
   base::WeakPtr<ProfileSyncService> sync_service_;
+  // If password generation should be enabled. We cache this value so that we

[Ilya Sherman, 2012/05/01 22:27:48]

nit: "should be" -> "is"

[zysxqn, 2012/05/03 00:57:24]

Done.

+  // don't spam the renderer with messages during startup when the underlying
+  // states are changing rapidly.
+  bool password_generation_enabled_;

[Ilya Sherman, 2012/05/01 22:27:48]

nit: Should this perhaps be a BooleanPrefMember instead?  You could even pass it
|this| as an observer, and do away with the PrefChangeRegistrar below.

[zysxqn, 2012/05/03 00:57:24]

Since it is actually affected by three things: prefs, sync states, and password
manager enabled states, I think keeping it as is is easier. thoughts?

[Ilya Sherman, 2012/05/04 06:04:50]

Ok, that makes sense -- I hadn't noticed when I wrote this comment that this
boolean reflects the combination of multiple underlying states.

+  // Handle preference change event.

[Ilya Sherman, 2012/05/01 22:27:48]

nit: This comment is currently redundant with the variable's type.  Perhaps
instead: "Listens for changes to the 'enabled' state for password generation."

[zysxqn, 2012/05/03 00:57:24]

Done.

+  PrefChangeRegistrar registrar_;
 
   // Our copy of the form data.
   ScopedVector<FormStructure> form_structures_;