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

chrome/browser/ui/login/login_prompt.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_UI_LOGIN_LOGIN_PROMPT_H_
 #define CHROME_BROWSER_UI_LOGIN_LOGIN_PROMPT_H_
 
 #include <string>
 
 #include "base/basictypes.h"
@@ skipping 30 matching lines @@
   LoginHandler(net::AuthChallengeInfo* auth_info, net::URLRequest* request);
 
   // Builds the platform specific LoginHandler. Used from within
   // CreateLoginPrompt() which creates tasks.
   static LoginHandler* Create(net::AuthChallengeInfo* auth_info,
                               net::URLRequest* request);
 
   // 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.
   void SetPasswordForm(const autofill::PasswordForm& form);
   void SetPasswordManager(password_manager::PasswordManager* password_manager);
 
   // 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.
   void SetAuth(const base::string16& username, const base::string16& password);
 
   // Display the error page without asking for credentials again.
   // This function can be called from either thread.
   void CancelAuth();
 
   // Implements the content::NotificationObserver interface.
   // Listens for AUTH_SUPPLIED and AUTH_CANCELLED notifications from other
   // LoginHandlers so that this LoginHandler has the chance to dismiss itself
   // if it was waiting for the same authentication.
   void Observe(int type,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
   // Who/where/what asked for the authentication.
   const net::AuthChallengeInfo* auth_info() const { return auth_info_.get(); }
 
   // Returns whether authentication had been handled (SetAuth or CancelAuth).
   bool WasAuthHandled() const;
 
  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();
 
   // Performs necessary cleanup before deletion.
   void ReleaseSoon();
 
   // Closes the native dialog.
   virtual void CloseDialog() = 0;
 
@@ skipping 117 matching lines @@
 // Helper to remove the ref from an net::URLRequest to the LoginHandler.
 // Should only be called from the IO thread, since it accesses an
 // net::URLRequest.
 void ResetLoginHandlerForRequest(net::URLRequest* request);
 
 // Get the signon_realm under which the identity should be saved.
 std::string GetSignonRealm(const GURL& url,
                            const net::AuthChallengeInfo& auth_info);
 
 #endif  // CHROME_BROWSER_UI_LOGIN_LOGIN_PROMPT_H_