Do not involve PasswordManagerDriver in filling HTTP-auth forms; also check realm

chrome/browser/ui/login/login_prompt.h

Index: chrome/browser/ui/login/login_prompt.h
diff --git a/chrome/browser/ui/login/login_prompt.h b/chrome/browser/ui/login/login_prompt.h
index 9f58dcdba547651299d9df5aea62b68be2ada2b8..ddebac0c876ee586dc8abe1a305cfe0a6feb7b66 100644
--- a/chrome/browser/ui/login/login_prompt.h
+++ b/chrome/browser/ui/login/login_prompt.h
@@ -48,10 +48,14 @@ class LoginHandler : public content::ResourceDispatcherHostLoginDelegate,
   // ResourceDispatcherHostLoginDelegate implementation:
   void OnRequestCancelled() override;
 
-  // Initializes the underlying platform specific view.
+  // Initializes the underlying platform specific view. If |manager| is not
+  // null, then also |observed_form| must not be null; in that case |manager| is
+  // used to retrieve stored credentials matching |observed_form| and fill them
+  // in the login prompt.
   virtual void BuildViewForPasswordManager(
       password_manager::PasswordManager* manager,
-      const base::string16& explanation) = 0;
+      const base::string16& explanation,
+      const autofill::PasswordForm* observed_form) = 0;

[Peter Kasting, 2015/10/06 22:14:49]

I can't say I love the pervasive

  DCHECK_EQ(manager == nullptr, observed_form == nullptr);

that this generates.  I wonder if it would make sense to split this into two
functions, one of which takes an explanation, and one of which takes a non-null
manager and observed_form as well.  Maybe not, since most implementations seem
to want to act similarly on both.

Maybe we should just move |manager| to be the second arg (to keep it next to
|observed_form| since they both seem to be optional)?  The DCHECK could also be
shortened:

  DCHECK_EQ(!manager, !observed_form);

...but I'm not sure that's more readable.

[vabr (Chromium), 2015/10/07 10:29:22]

You raise a good point. I attempted to solve this by bundling the login model
and the form into a struct, to enforce passing both or none.

 
   // Sets information about the authentication type (|form|) and the
   // |password_manager| for this profile.
@@ -61,9 +65,8 @@ class LoginHandler : public content::ResourceDispatcherHostLoginDelegate,
   // Returns the WebContents that needs authentication.
   content::WebContents* GetWebContentsForLogin() const;
 
-  // Returns the PasswordManager for the render frame that needs login.
-  password_manager::ContentPasswordManagerDriver*
-  GetPasswordManagerDriverForLogin();
+  // Returns the PasswordManager for the web contents that needs login.
+  password_manager::PasswordManager* GetPasswordManagerForLogin();
 
   // Resend the request with authentication credentials.
   // This function can be called from either thread.
@@ -90,7 +93,8 @@ class LoginHandler : public content::ResourceDispatcherHostLoginDelegate,
  protected:
   ~LoginHandler() override;
 
-  void SetModel(password_manager::LoginModel* model);
+  void SetModel(password_manager::LoginModel* model,
+                const autofill::PasswordForm* observed_form);
 
   // Notify observers that authentication is needed.
   void NotifyAuthNeeded();