Tidy up LoginHandler callbacks.

LoginInterstitialDelegate should take a OnceClosure, and all these
static methods (probably predating base::Callback) can be normal
methods.

This is some small cleanup to make the upcoming big change to
LoginHandler smaller.

Bug: 908926
Change-Id: Idec46fe691ac82b298ed6e90f8e5bcaac31e4a32
Reviewed-on: https://chromium-review.googlesource.com/c/1388039
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626246}

chrome/browser/ui/login/login_handler.cc

@@ -531,21 +531,19 @@ void LoginHandler::GetDialogStrings(const GURL& request_url,
   }
 }
 
-// static
 void LoginHandler::ShowLoginPrompt(const GURL& request_url,
-                                   net::AuthChallengeInfo* auth_info,
-                                   LoginHandler* handler) {
+                                   net::AuthChallengeInfo* auth_info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  WebContents* parent_contents = handler->GetWebContentsForLogin();
+  WebContents* parent_contents = GetWebContentsForLogin();
   if (!parent_contents) {
-    handler->CancelAuth();
+    CancelAuth();
     return;
   }
   prerender::PrerenderContents* prerender_contents =
       prerender::PrerenderContents::FromWebContents(parent_contents);
   if (prerender_contents) {
     prerender_contents->Destroy(prerender::FINAL_STATUS_AUTH_NEEDED);
-    handler->CancelAuth();
+    CancelAuth();
     return;
   }
 
@@ -554,7 +552,7 @@ void LoginHandler::ShowLoginPrompt(const GURL& request_url,
   GetDialogStrings(request_url, *auth_info, &authority, &explanation);
 
   password_manager::PasswordManager* password_manager =
-      handler->GetPasswordManagerForLogin();
+      GetPasswordManagerForLogin();
 
   if (!password_manager) {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
@@ -565,11 +563,11 @@ void LoginHandler::ShowLoginPrompt(const GURL& request_url,
     if (guest &&
         extensions::GetViewType(guest->owner_web_contents()) !=
             extensions::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
-      handler->BuildViewWithoutPasswordManager(authority, explanation);
+      BuildViewWithoutPasswordManager(authority, explanation);
       return;
     }
 #endif
-    handler->CancelAuth();
+    CancelAuth();
     return;
   }
 
@@ -582,26 +580,24 @@ void LoginHandler::ShowLoginPrompt(const GURL& request_url,
   }
 
   PasswordForm observed_form(
-      LoginHandler::MakeInputForPasswordManager(request_url, *auth_info));
-  handler->BuildViewWithPasswordManager(authority, explanation,
-                                        password_manager, observed_form);
+      MakeInputForPasswordManager(request_url, *auth_info));
+  BuildViewWithPasswordManager(authority, explanation, password_manager,
+                               observed_form);
 }
 
-// static
 void LoginHandler::LoginDialogCallback(
     const GURL& request_url,
     const content::GlobalRequestID& request_id,
     net::AuthChallengeInfo* auth_info,
     scoped_refptr<net::HttpResponseHeaders> response_headers,
-    LoginHandler* handler,
     bool is_main_frame) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  WebContents* parent_contents = handler->GetWebContentsForLogin();
-  if (!parent_contents || handler->WasAuthHandled()) {
+  WebContents* parent_contents = GetWebContentsForLogin();
+  if (!parent_contents || WasAuthHandled()) {
     // The request may have been canceled, or it may be for a renderer not
     // hosted by a tab (e.g. an extension). Cancel just in case (canceling twice
     // is a no-op).
-    handler->CancelAuth();
+    CancelAuth();
     return;
   }
 
@@ -612,9 +608,9 @@ void LoginHandler::LoginDialogCallback(
   auto* api =
       extensions::BrowserContextKeyedAPIFactory<extensions::WebRequestAPI>::Get(
           parent_contents->GetBrowserContext());
-  auto continuation = base::BindOnce(&LoginHandler::MaybeSetUpLoginPrompt,
-                                     request_url, base::RetainedRef(auth_info),
-                                     base::RetainedRef(handler), is_main_frame);
+  auto continuation =
+      base::BindOnce(&LoginHandler::MaybeSetUpLoginPrompt, this, request_url,
+                     base::RetainedRef(auth_info), is_main_frame);
   if (api->MaybeProxyAuthRequest(auth_info, std::move(response_headers),
                                  request_id, is_main_frame,
                                  std::move(continuation))) {
@@ -622,36 +618,34 @@ void LoginHandler::LoginDialogCallback(
   }
 #endif
 
-  MaybeSetUpLoginPrompt(request_url, auth_info, handler, is_main_frame,
-                        base::nullopt, false /* should_cancel */);
+  MaybeSetUpLoginPrompt(request_url, auth_info, is_main_frame, base::nullopt,
+                        false /* should_cancel */);
 }
 
-// static
 void LoginHandler::MaybeSetUpLoginPrompt(
     const GURL& request_url,
     net::AuthChallengeInfo* auth_info,
-    LoginHandler* handler,
     bool is_request_for_main_frame,
     const base::Optional<net::AuthCredentials>& credentials,
     bool should_cancel) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  WebContents* parent_contents = handler->GetWebContentsForLogin();
-  if (!parent_contents || handler->WasAuthHandled()) {
+  WebContents* parent_contents = GetWebContentsForLogin();
+  if (!parent_contents || WasAuthHandled()) {
     // The request may have been canceled, or it may be for a renderer not
     // hosted by a tab (e.g. an extension). Cancel just in case (canceling twice
     // is a no-op).
-    handler->CancelAuth();
+    CancelAuth();
     return;
   }
 
   if (should_cancel) {
-    handler->CancelAuth();
+    CancelAuth();
     return;
   }
 
   if (credentials) {
-    handler->SetAuth(credentials->username(), credentials->password());
+    SetAuth(credentials->username(), credentials->password());
     return;
   }
 
@@ -693,15 +687,15 @@ void LoginHandler::MaybeSetUpLoginPrompt(
 
     // Show a blank interstitial for main-frame, cross origin requests
     // so that the correct URL is shown in the omnibox.
-    base::Closure callback =
-        base::Bind(&LoginHandler::ShowLoginPrompt, request_url,
-                   base::RetainedRef(auth_info), base::RetainedRef(handler));
+    base::OnceClosure callback =
+        base::BindOnce(&LoginHandler::ShowLoginPrompt, this, request_url,
+                       base::RetainedRef(auth_info));
     // The interstitial delegate is owned by the interstitial that it creates.
     // This cancels any existing interstitial.
-    handler->SetInterstitialDelegate(
+    SetInterstitialDelegate(
         (new LoginInterstitialDelegate(
              parent_contents, auth_info->is_proxy ? GURL() : request_url,
-             callback))
+             std::move(callback)))
             ->GetWeakPtr());
 
   } else {
@@ -712,7 +706,7 @@ void LoginHandler::MaybeSetUpLoginPrompt(
                                    ? AUTH_PROMPT_TYPE_SUBRESOURCE_CROSS_ORIGIN
                                    : AUTH_PROMPT_TYPE_SUBRESOURCE_SAME_ORIGIN);
     }
-    ShowLoginPrompt(request_url, auth_info, handler);
+    ShowLoginPrompt(request_url, auth_info);
   }
 }
 
@@ -730,8 +724,8 @@ scoped_refptr<LoginHandler> CreateLoginPrompt(
       auth_info, web_contents_getter, std::move(auth_required_callback));
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&LoginHandler::LoginDialogCallback, url, request_id,
-                     base::RetainedRef(auth_info), std::move(response_headers),
-                     base::RetainedRef(handler), is_request_for_main_frame));
+      base::BindOnce(&LoginHandler::LoginDialogCallback, handler, url,
+                     request_id, base::RetainedRef(auth_info),
+                     std::move(response_headers), is_request_for_main_frame));
   return handler;
 }

chrome/browser/ui/login/login_handler.h

@@ -200,9 +200,8 @@ class LoginHandler : public content::LoginDelegate,
                                base::string16* authority,
                                base::string16* explanation);
 
-  static void ShowLoginPrompt(const GURL& request_url,
-                              net::AuthChallengeInfo* auth_info,
-                              LoginHandler* handler);
+  void ShowLoginPrompt(const GURL& request_url,
+                       net::AuthChallengeInfo* auth_info);
 
   // This callback is run on the UI thread and creates a constrained window with
   // a LoginView to prompt the user. If the prompt is triggered because of a
@@ -211,12 +210,11 @@ class LoginHandler : public content::LoginDelegate,
   // created directly in this callback. In both cases, the response will be sent
   // to LoginHandler, which then routes it to the net::URLRequest on the I/O
   // thread.
-  static void LoginDialogCallback(
+  void LoginDialogCallback(
       const GURL& request_url,
       const content::GlobalRequestID& request_id,
       net::AuthChallengeInfo* auth_info,
       scoped_refptr<net::HttpResponseHeaders> response_headers,
-      LoginHandler* handler,
       bool is_main_frame);
 
   // Continuation from |LoginDialogCallback()| after any potential interception
@@ -224,10 +222,9 @@ class LoginHandler : public content::LoginDelegate,
   // request is cancelled. Otherwise |credentials| are used if supplied. Finally
   // if the request is NOT cancelled AND |credentials| is empty, then we'll
   // actually show a login prompt.
-  static void MaybeSetUpLoginPrompt(
+  void MaybeSetUpLoginPrompt(
       const GURL& request_url,
       net::AuthChallengeInfo* auth_info,
-      LoginHandler* handler,
       bool is_main_frame,
       const base::Optional<net::AuthCredentials>& credentials,
       bool should_cancel);

chrome/browser/ui/login/login_interstitial_delegate.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ui/login/login_interstitial_delegate.h"
 
+#include <utility>
+
 const content::InterstitialPageDelegate::TypeID
     LoginInterstitialDelegate::kTypeForTesting =
         &LoginInterstitialDelegate::kTypeForTesting;
@@ -11,8 +13,8 @@ const content::InterstitialPageDelegate::TypeID
 LoginInterstitialDelegate::LoginInterstitialDelegate(
     content::WebContents* web_contents,
     const GURL& request_url,
-    base::Closure& callback)
-    : callback_(callback),
+    base::OnceClosure callback)
+    : callback_(std::move(callback)),
       interstitial_page_(content::InterstitialPage::Create(web_contents,
                                                            true,
                                                            request_url,
@@ -39,7 +41,7 @@ LoginInterstitialDelegate::GetWeakPtr() {
 }
 
 void LoginInterstitialDelegate::CommandReceived(const std::string& command) {
-  callback_.Run();
+  std::move(callback_).Run();
 }
 
 content::InterstitialPageDelegate::TypeID

chrome/browser/ui/login/login_interstitial_delegate.h

@@ -27,7 +27,7 @@ class LoginInterstitialDelegate : public content::InterstitialPageDelegate {
 
   LoginInterstitialDelegate(content::WebContents* web_contents,
                             const GURL& request_url,
-                            base::Closure& callback);
+                            base::OnceClosure callback);
 
   ~LoginInterstitialDelegate() override;
 
@@ -43,7 +43,7 @@ class LoginInterstitialDelegate : public content::InterstitialPageDelegate {
   std::string GetHTMLContents() override;
 
  private:
-  base::Closure callback_;
+  base::OnceClosure callback_;
   content::InterstitialPage* interstitial_page_;
   base::WeakPtrFactory<LoginInterstitialDelegate> weak_ptr_factory_;