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

chrome/browser/autofill/autofill_manager.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_AUTOFILL_AUTOFILL_MANAGER_H_
 #define CHROME_BROWSER_AUTOFILL_AUTOFILL_MANAGER_H_
 #pragma once
 
 #include <list>
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "base/string16.h"
 #include "base/time.h"
 #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;
 class AutofillField;
 class AutofillProfile;
 class AutofillMetrics;
 class CreditCard;
 class PersonalDataManager;
 class PrefService;
 class ProfileSyncService;
@@ skipping 16 matching lines @@
 namespace webkit {
 namespace forms {
 struct FormData;
 struct FormField;
 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> {
  public:
   explicit AutofillManager(TabContentsWrapper* tab_contents);
 
   // Registers our Enable/Disable Autofill pref.
   static void RegisterUserPrefs(PrefService* prefs);
 
   // Set our external delegate.
@@ skipping 34 matching lines @@
 
   // Returns the value of the AutofillEnabled pref.
   virtual bool IsAutofillEnabled() const;
 
   // Uploads the form data to the Autofill server.
   virtual void UploadFormData(const FormStructure& submitted_form);
 
   // 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(
       const FormStructure* submitted_form,
       const base::TimeTicks& load_time,
       const base::TimeTicks& interaction_time,
       const base::TimeTicks& submission_time);
 
   // Maps GUIDs to and from IDs that are used to identify profiles and credit
   // cards sent to and from the renderer process.
@@ skipping 32 matching lines @@
   // AutofillDownloadManager::Observer:
   virtual void OnLoadedServerPredictions(
       const std::string& response_xml) OVERRIDE;
 
   // ProfileSyncServiceObserver:
   virtual void OnStateChanged() OVERRIDE;
 
   // 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,
                                      bool new_renderer);
 
   void OnFormsSeen(const std::vector<webkit::forms::FormData>& forms,
                    const base::TimeTicks& timestamp);
   void OnTextFieldDidChange(const webkit::forms::FormData& form,
                             const webkit::forms::FormField& field,
                             const base::TimeTicks& timestamp);
@@ skipping 142 matching lines @@
   bool user_did_type_;
   // Has the user autofilled a form on this page?
   bool user_did_autofill_;
   // Has the user edited a field that was previously autofilled?
   bool user_did_edit_autofilled_field_;
   // When the page finished loading.
   base::TimeTicks forms_loaded_timestamp_;
   // 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_;
 
   // GUID to ID mapping.  We keep two maps to convert back and forth.
   mutable std::map<GUIDPair, int> guid_id_map_;
   mutable std::map<int, GUIDPair> id_guid_map_;
 
   // Delegate to perform external processing (display, selection) on
   // our behalf.  Weak.
@@ skipping 18 matching lines @@
                            TestTabContentsWithExternalDelegate);
   FRIEND_TEST_ALL_PREFIXES(AutofillMetricsTest,
                            UserHappinessFormLoadAndSubmission);
   FRIEND_TEST_ALL_PREFIXES(AutofillMetricsTest, UserHappinessFormInteraction);
   FRIEND_TEST_ALL_PREFIXES(AutofillMetricsTest, FormFillDuration);
 
   DISALLOW_COPY_AND_ASSIGN(AutofillManager);
 };
 
 #endif  // CHROME_BROWSER_AUTOFILL_AUTOFILL_MANAGER_H_