[password autofill] Remove references to PasswordForm from RenderViewImpl

chrome/browser/password_manager/password_manager.h

Index: chrome/browser/password_manager/password_manager.h
diff --git a/chrome/browser/password_manager/password_manager.h b/chrome/browser/password_manager/password_manager.h
index a0dbd7c3daeb848ca694a0be58ba96dc7548de7a..86bdaeb1c7d8375c0d8b275771789c41ce71b692 100644
--- a/chrome/browser/password_manager/password_manager.h
+++ b/chrome/browser/password_manager/password_manager.h
@@ -7,6 +7,7 @@
 
 #include <vector>
 
+#include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/observer_list.h"
@@ -42,6 +43,13 @@ class PasswordManager : public LoginModel,
       PasswordManagerDelegate* delegate);
   virtual ~PasswordManager();
 
+  typedef base::Callback<void(const content::PasswordForm&)>
+  PasswordSubmittedCallback;

[Ilya Sherman, 2013/07/27 01:09:48]

nit: Please indent four more spaces.

[Garrett Casto, 2013/08/03 00:38:42]

Done.

+
+  // There is no corresponding remove function as currently all of the
+  // owners of these callbacks have the same lifetime as the PasswordManager.

[Ilya Sherman, 2013/07/27 01:09:48]

That's not entirely accurate -- there is some ordering to the destruction of
objects dangling off a WebContents.  But that's really more to the point of
whether base::Unretained() is safe to use or not.

[Garrett Casto, 2013/08/03 00:38:42]

I tried to clear this comment up a bit. Not sure if this is clearer or not.

+  void AddSubmissionCallback(const PasswordSubmittedCallback& callback);

[Ilya Sherman, 2013/07/27 01:09:48]

Could you add some test coverage for these callbacks?

[Garrett Casto, 2013/08/03 00:38:42]

Done.

+
   // Is saving new data for password autofill enabled for the current profile?
   // For example, saving new data is disabled in Incognito mode, whereas filling
   // data is not.
@@ -69,7 +77,7 @@ class PasswordManager : public LoginModel,
   void ProvisionallySavePassword(const content::PasswordForm& form);
 
   // content::WebContentsObserver overrides.
-  virtual void DidNavigateAnyFrame(
+  virtual void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) OVERRIDE;
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
@@ -88,6 +96,10 @@ class PasswordManager : public LoginModel,
 
  private:
   friend class content::WebContentsUserData<PasswordManager>;
+  friend class PasswordManagerTest;

[Ilya Sherman, 2013/07/27 01:09:48]

Rather than friending the test class -- which allows the class to muck with
*all* of the private state for this object, please just make
OnPasswordFormSubmitted have protected visibility, and create a subclass for
testing that ups the visibility to public.

[Garrett Casto, 2013/08/03 00:38:42]

As you wish. I personally don't have a problem with this for testing code.

+
+  // Handle notification that a password form was submitted.
+  void OnPasswordFormSubmitted(const content::PasswordForm& password_form);
 
   // Possibly set up FieldTrial for testing other possible usernames. This only
   // happens if there are other_possible_usernames to be shown and the
@@ -134,9 +146,6 @@ class PasswordManager : public LoginModel,
   // containing WebContents.
   PasswordManagerDelegate* const delegate_;
 
-  // The LoginModelObserver (i.e LoginView) requiring autofill.
-  LoginModelObserver* observer_;
-
   // Set to false to disable the password manager (will no longer ask if you
   // want to save passwords but will continue to fill passwords).
   BooleanPrefMember password_manager_enabled_;
@@ -145,6 +154,9 @@ class PasswordManager : public LoginModel,
   // notification in const member functions.
   mutable ObserverList<LoginModelObserver> observers_;
 
+  // Callbacks to be notified when a password form has been submitted.
+  std::vector<PasswordSubmittedCallback> submission_callbacks_;
+
   DISALLOW_COPY_AND_ASSIGN(PasswordManager);
 };