Remove NetworkDelegate::OnAuthRequired().

With the non-NetworkService path now removed, only the
URLRequest::Delegate auth path is used.

This CL leaves a supporting enum and callback in NetworkDelegate, which
aren't used by NetworkDelegate, but are still used. I'll figure out
where to move them in another CL.

Bug: 934009
Change-Id: Icf5f8344ca0326c7ac0bbd72440ad602f65c3363
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1798886Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695824}

net/base/layered_network_delegate.cc

@@ -166,21 +166,6 @@ void LayeredNetworkDelegate::OnPACScriptErrorInternal(
     const base::string16& error) {
 }
 
-NetworkDelegate::AuthRequiredResponse LayeredNetworkDelegate::OnAuthRequired(
-    URLRequest* request,
-    const AuthChallengeInfo& auth_info,
-    AuthCallback callback,
-    AuthCredentials* credentials) {
-  OnAuthRequiredInternal(request, auth_info, credentials);
-  return nested_network_delegate_->NotifyAuthRequired(
-      request, auth_info, std::move(callback), credentials);
-}
-
-void LayeredNetworkDelegate::OnAuthRequiredInternal(
-    URLRequest* request,
-    const AuthChallengeInfo& auth_info,
-    AuthCredentials* credentials) {}
-
 bool LayeredNetworkDelegate::OnCanGetCookies(const URLRequest& request,
                                              const CookieList& cookie_list,
                                              bool allowed_from_caller) {

net/base/layered_network_delegate.h

@@ -71,10 +71,6 @@ class NET_EXPORT LayeredNetworkDelegate : public NetworkDelegate {
   void OnCompleted(URLRequest* request, bool started, int net_error) final;
   void OnURLRequestDestroyed(URLRequest* request) final;
   void OnPACScriptError(int line_number, const base::string16& error) final;
-  AuthRequiredResponse OnAuthRequired(URLRequest* request,
-                                      const AuthChallengeInfo& auth_info,
-                                      AuthCallback callback,
-                                      AuthCredentials* credentials) final;
   bool OnCanGetCookies(const URLRequest& request,
                        const CookieList& cookie_list,
                        bool allowed_from_caller) final;
@@ -154,10 +150,6 @@ class NET_EXPORT LayeredNetworkDelegate : public NetworkDelegate {
                                       CookieOptions* options,
                                       bool allowed_from_caller);
 
-  virtual void OnAuthRequiredInternal(URLRequest* request,
-                                      const AuthChallengeInfo& auth_info,
-                                      AuthCredentials* credentials);
-
   // If this returns false, it short circuits the corresponding call in any
   // nested NetworkDelegates.
   virtual bool OnForcePrivacyModeInternal(

net/base/layered_network_delegate_unittest.cc

@@ -13,7 +13,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/optional.h"
-#include "net/base/auth.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_delegate_impl.h"
 #include "net/base/request_priority.h"
@@ -108,14 +107,6 @@ class TestNetworkDelegateImpl : public NetworkDelegateImpl {
     IncrementAndCompareCounter("on_pac_script_error_count");
   }
 
-  AuthRequiredResponse OnAuthRequired(URLRequest* request,
-                                      const AuthChallengeInfo& auth_info,
-                                      AuthCallback callback,
-                                      AuthCredentials* credentials) override {
-    IncrementAndCompareCounter("on_auth_required_count");
-    return NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
-  }
-
   bool OnCanGetCookies(const URLRequest& request,
                        const CookieList& cookie_list,
                        bool allowed_from_caller) override {
@@ -174,7 +165,6 @@ class TestLayeredNetworkDelegate : public LayeredNetworkDelegate {
   ~TestLayeredNetworkDelegate() override = default;
 
   void CallAndVerify() {
-    AuthChallengeInfo auth_challenge;
     std::unique_ptr<URLRequest> request = context_.CreateRequest(
         GURL(), IDLE, &delegate_, TRAFFIC_ANNOTATION_FOR_TESTS);
     std::unique_ptr<HttpRequestHeaders> request_headers(
@@ -200,9 +190,6 @@ class TestLayeredNetworkDelegate : public LayeredNetworkDelegate {
     OnCompleted(request.get(), false, net::OK);
     OnURLRequestDestroyed(request.get());
     OnPACScriptError(0, base::string16());
-    EXPECT_EQ(
-        NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION,
-        OnAuthRequired(request.get(), auth_challenge, AuthCallback(), nullptr));
     EXPECT_FALSE(OnCanGetCookies(*request, CookieList(), true));
     EXPECT_FALSE(
         OnCanSetCookie(*request, net::CanonicalCookie(), nullptr, true));
@@ -287,13 +274,6 @@ class TestLayeredNetworkDelegate : public LayeredNetworkDelegate {
     EXPECT_EQ(1, (*counters_)["on_pac_script_error_count"]);
   }
 
-  void OnAuthRequiredInternal(URLRequest* request,
-                              const AuthChallengeInfo& auth_info,
-                              AuthCredentials* credentials) override {
-    ++(*counters_)["on_auth_required_count"];
-    EXPECT_EQ(1, (*counters_)["on_auth_required_count"]);
-  }
-
   bool OnCanGetCookiesInternal(const URLRequest& request,
                                const CookieList& cookie_list,
                                bool allowed_from_caller) override {

net/base/network_delegate.cc

@@ -132,15 +132,6 @@ void NetworkDelegate::NotifyPACScriptError(int line_number,
   OnPACScriptError(line_number, error);
 }
 
-NetworkDelegate::AuthRequiredResponse NetworkDelegate::NotifyAuthRequired(
-    URLRequest* request,
-    const AuthChallengeInfo& auth_info,
-    AuthCallback callback,
-    AuthCredentials* credentials) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  return OnAuthRequired(request, auth_info, std::move(callback), credentials);
-}
-
 bool NetworkDelegate::CanGetCookies(const URLRequest& request,
                                     const CookieList& cookie_list,
                                     bool allowed_from_caller) {

net/base/network_delegate.h

@@ -48,6 +48,24 @@ class NET_EXPORT NetworkDelegate {
   // AuthRequiredResponse indicates how a NetworkDelegate handles an
   // OnAuthRequired call. It's placed in this file to prevent url_request.h
   // from having to include network_delegate.h.
+  //
+  //  - AUTH_REQUIRED_RESPONSE_NO_ACTION: |auth_info| is observed, but
+  //    no action is being taken on it.
+  //  - AUTH_REQUIRED_RESPONSE_SET_AUTH: |credentials| is filled in with
+  //    a username and password, which should be used in a response to
+  //    |auth_info|.
+  //  - AUTH_REQUIRED_RESPONSE_CANCEL_AUTH: The authentication challenge
+  //    should not be attempted.
+  //  - AUTH_REQUIRED_RESPONSE_IO_PENDING: The action will be decided
+  //    asynchronously. |callback| will be invoked when the decision is made,
+  //    and one of the other AuthRequiredResponse values will be passed in with
+  //    the same semantics as described above. Note, however, that a pending
+  //    operation may be cancelled by OnURLRequestDestroyed. Once cancelled,
+  //    |request|, |auth_info|, and |credentials| become invalid and |callback|
+  //    may not be called.
+  //
+  // TODO(mmenke): These are no longer used by NetworkDelegate. Move
+  // AuthRequiredResponse and remove AuthCallback.
   enum AuthRequiredResponse {
     AUTH_REQUIRED_RESPONSE_NO_ACTION,
     AUTH_REQUIRED_RESPONSE_SET_AUTH,
@@ -88,10 +106,6 @@ class NET_EXPORT NetworkDelegate {
   void NotifyCompleted(URLRequest* request, bool started, int net_error);
   void NotifyURLRequestDestroyed(URLRequest* request);
   void NotifyPACScriptError(int line_number, const base::string16& error);
-  AuthRequiredResponse NotifyAuthRequired(URLRequest* request,
-                                          const AuthChallengeInfo& auth_info,
-                                          AuthCallback callback,
-                                          AuthCredentials* credentials);
   bool CanGetCookies(const URLRequest& request,
                      const CookieList& cookie_list,
                      bool allowed_from_caller);
@@ -254,31 +268,6 @@ class NET_EXPORT NetworkDelegate {
   virtual void OnPACScriptError(int line_number,
                                 const base::string16& error) = 0;
 
-  // Called when a request receives an authentication challenge
-  // specified by |auth_info|, and is unable to respond using cached
-  // credentials. |callback| and |credentials| must be non-NULL.
-  //
-  // The following return values are allowed:
-  //  - AUTH_REQUIRED_RESPONSE_NO_ACTION: |auth_info| is observed, but
-  //    no action is being taken on it.
-  //  - AUTH_REQUIRED_RESPONSE_SET_AUTH: |credentials| is filled in with
-  //    a username and password, which should be used in a response to
-  //    |auth_info|.
-  //  - AUTH_REQUIRED_RESPONSE_CANCEL_AUTH: The authentication challenge
-  //    should not be attempted.
-  //  - AUTH_REQUIRED_RESPONSE_IO_PENDING: The action will be decided
-  //    asynchronously. |callback| will be invoked when the decision is made,
-  //    and one of the other AuthRequiredResponse values will be passed in with
-  //    the same semantics as described above. Note, however, that a pending
-  //    operation may be cancelled by OnURLRequestDestroyed. Once cancelled,
-  //    |request|, |auth_info|, and |credentials| become invalid and |callback|
-  //    may not be called.
-  virtual AuthRequiredResponse OnAuthRequired(
-      URLRequest* request,
-      const AuthChallengeInfo& auth_info,
-      AuthCallback callback,
-      AuthCredentials* credentials) = 0;
-
   // Called when reading cookies to allow the network delegate to block access
   // to the cookie. This method will never be invoked when
   // LOAD_DO_NOT_SEND_COOKIES is specified.

net/base/network_delegate_impl.cc

@@ -63,14 +63,6 @@ void NetworkDelegateImpl::OnPACScriptError(int line_number,
                                            const base::string16& error) {
 }
 
-NetworkDelegate::AuthRequiredResponse NetworkDelegateImpl::OnAuthRequired(
-    URLRequest* request,
-    const AuthChallengeInfo& auth_info,
-    AuthCallback callback,
-    AuthCredentials* credentials) {
-  return AUTH_REQUIRED_RESPONSE_NO_ACTION;
-}
-
 bool NetworkDelegateImpl::OnCanGetCookies(const URLRequest& request,
                                           const CookieList& cookie_list,
                                           bool allowed_from_caller) {

net/base/network_delegate_impl.h

@@ -73,11 +73,6 @@ class NET_EXPORT NetworkDelegateImpl : public NetworkDelegate {
 
   void OnPACScriptError(int line_number, const base::string16& error) override;
 
-  AuthRequiredResponse OnAuthRequired(URLRequest* request,
-                                      const AuthChallengeInfo& auth_info,
-                                      AuthCallback callback,
-                                      AuthCredentials* credentials) override;
-
   bool OnCanGetCookies(const URLRequest& request,
                        const CookieList& cookie_list,
                        bool allowed_from_caller) override;

net/log/net_log_event_type_list.h

@@ -841,7 +841,6 @@ EVENT_TYPE(URL_REQUEST_REDIRECTED)
 // Measures the time between when a net::URLRequest calls a delegate that can
 // block it, and when the delegate allows the request to resume. Each delegate
 // type has a corresponding event type.
-EVENT_TYPE(NETWORK_DELEGATE_AUTH_REQUIRED)
 EVENT_TYPE(NETWORK_DELEGATE_BEFORE_START_TRANSACTION)
 EVENT_TYPE(NETWORK_DELEGATE_BEFORE_URL_REQUEST)
 EVENT_TYPE(NETWORK_DELEGATE_HEADERS_RECEIVED)

net/proxy_resolution/network_delegate_error_observer_unittest.cc

@@ -58,12 +58,6 @@ class TestNetworkDelegate : public NetworkDelegateImpl {
   void OnPACScriptError(int line_number, const base::string16& error) override {
     got_pac_error_ = true;
   }
-  AuthRequiredResponse OnAuthRequired(URLRequest* request,
-                                      const AuthChallengeInfo& auth_info,
-                                      AuthCallback callback,
-                                      AuthCredentials* credentials) override {
-    return AUTH_REQUIRED_RESPONSE_NO_ACTION;
-  }
   bool OnCanGetCookies(const URLRequest& request,
                        const CookieList& cookie_list,
                        bool allowed_from_caller) override {

net/proxy_resolution/pac_file_fetcher_impl_unittest.cc

@@ -174,14 +174,6 @@ class BasicNetworkDelegate : public NetworkDelegateImpl {
   void OnPACScriptError(int line_number, const base::string16& error) override {
   }
 
-  NetworkDelegate::AuthRequiredResponse OnAuthRequired(
-      URLRequest* request,
-      const AuthChallengeInfo& auth_info,
-      AuthCallback callback,
-      AuthCredentials* credentials) override {
-    return NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
-  }
-
   bool OnCanGetCookies(const URLRequest& request,
                        const CookieList& cookie_list,
                        bool allowed_from_caller) override {

net/url_request/url_request.cc

@@ -1015,58 +1015,11 @@ void URLRequest::SetPriority(RequestPriority priority) {
 
 void URLRequest::NotifyAuthRequired(
     std::unique_ptr<AuthChallengeInfo> auth_info) {
-  NetworkDelegate::AuthRequiredResponse rv =
-      NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
-  auth_info_ = std::move(auth_info);
-  DCHECK(auth_info_);
-  if (network_delegate_) {
-    OnCallToDelegate(NetLogEventType::NETWORK_DELEGATE_AUTH_REQUIRED);
-    rv = network_delegate_->NotifyAuthRequired(
-        this, *auth_info_.get(),
-        base::BindOnce(&URLRequest::NotifyAuthRequiredComplete,
-                       base::Unretained(this)),
-        &auth_credentials_);
-    if (rv == NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING)
-      return;
-  }
-
-  NotifyAuthRequiredComplete(rv);
-}
-
-void URLRequest::NotifyAuthRequiredComplete(
-    NetworkDelegate::AuthRequiredResponse result) {
-  OnCallToDelegateComplete();
-
+  DCHECK(auth_info);
   // Check that there are no callbacks to already canceled requests.
   DCHECK_NE(URLRequestStatus::CANCELED, status_.status());
 
-  // NotifyAuthRequired may be called multiple times, such as
-  // when an authentication attempt fails. Clear out the data
-  // so it can be reset on another round.
-  AuthCredentials credentials = auth_credentials_;
-  auth_credentials_ = AuthCredentials();
-  std::unique_ptr<AuthChallengeInfo> auth_info;
-  auth_info.swap(auth_info_);
-
-  switch (result) {
-    case NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION:
-      // Defer to the URLRequest::Delegate, since the NetworkDelegate
-      // didn't take an action.
-      delegate_->OnAuthRequired(this, *auth_info.get());
-      break;
-
-    case NetworkDelegate::AUTH_REQUIRED_RESPONSE_SET_AUTH:
-      SetAuth(credentials);
-      break;
-
-    case NetworkDelegate::AUTH_REQUIRED_RESPONSE_CANCEL_AUTH:
-      CancelAuth();
-      break;
-
-    case NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING:
-      NOTREACHED();
-      break;
-  }
+  delegate_->OnAuthRequired(this, *auth_info.get());
 }
 
 void URLRequest::NotifyCertificateRequested(

net/url_request/url_request.h

@@ -843,7 +843,6 @@ class NET_EXPORT URLRequest : public base::SupportsUserData {
   // These functions delegate to |delegate_|.  See URLRequest::Delegate for the
   // meaning of these functions.
   void NotifyAuthRequired(std::unique_ptr<AuthChallengeInfo> auth_info);
-  void NotifyAuthRequiredComplete(NetworkDelegate::AuthRequiredResponse result);
   void NotifyCertificateRequested(SSLCertRequestInfo* cert_request_info);
   void NotifySSLCertificateError(int net_error,
                                  const SSLInfo& ssl_info,
@@ -970,13 +969,6 @@ class NET_EXPORT URLRequest : public base::SupportsUserData {
   // TODO(battre): Remove this. http://crbug.com/89049
   bool has_notified_completion_;
 
-  // Authentication data used by the NetworkDelegate for this request,
-  // if one is present. |auth_credentials_| may be filled in when calling
-  // |NotifyAuthRequired| on the NetworkDelegate. |auth_info_| holds
-  // the authentication challenge being handled by |NotifyAuthRequired|.
-  AuthCredentials auth_credentials_;
-  std::unique_ptr<AuthChallengeInfo> auth_info_;
-
   int64_t received_response_content_length_;
 
   base::TimeTicks creation_time_;

net/url_request/url_request_context_builder.cc

@@ -107,14 +107,6 @@ class BasicNetworkDelegate : public NetworkDelegateImpl {
   void OnPACScriptError(int line_number, const base::string16& error) override {
   }
 
-  NetworkDelegate::AuthRequiredResponse OnAuthRequired(
-      URLRequest* request,
-      const AuthChallengeInfo& auth_info,
-      AuthCallback callback,
-      AuthCredentials* credentials) override {
-    return NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
-  }
-
   bool OnCanGetCookies(const URLRequest& request,
                        const CookieList& cookie_list,
                        bool allowed_from_caller) override {

net/url_request/url_request_test_util.cc

@@ -40,13 +40,12 @@ const int kStageBeforeURLRequest = 1 << 0;
 const int kStageBeforeStartTransaction = 1 << 1;
 const int kStageStartTransaction = 1 << 2;
 const int kStageHeadersReceived = 1 << 3;
-const int kStageAuthRequired = 1 << 4;
-const int kStageBeforeRedirect = 1 << 5;
-const int kStageResponseStarted = 1 << 6;
-const int kStageCompletedSuccess = 1 << 7;
-const int kStageCompletedError = 1 << 8;
-const int kStageURLRequestDestroyed = 1 << 9;
-const int kStageDestruction = 1 << 10;
+const int kStageBeforeRedirect = 1 << 4;
+const int kStageResponseStarted = 1 << 5;
+const int kStageCompletedSuccess = 1 << 6;
+const int kStageCompletedError = 1 << 7;
+const int kStageURLRequestDestroyed = 1 << 8;
+const int kStageDestruction = 1 << 9;
 
 }  // namespace
 
@@ -364,7 +363,6 @@ TestNetworkDelegate::TestNetworkDelegate()
       total_network_bytes_received_(0),
       total_network_bytes_sent_(0),
       has_load_timing_info_before_redirect_(false),
-      has_load_timing_info_before_auth_(false),
       experimental_cookie_features_enabled_(false),
       cancel_request_with_policy_violating_referrer_(false),
       will_be_intercepted_on_next_error_(false),
@@ -384,12 +382,6 @@ bool TestNetworkDelegate::GetLoadTimingInfoBeforeRedirect(
   return has_load_timing_info_before_redirect_;
 }
 
-bool TestNetworkDelegate::GetLoadTimingInfoBeforeAuth(
-    LoadTimingInfo* load_timing_info_before_auth) const {
-  *load_timing_info_before_auth = load_timing_info_before_auth_;
-  return has_load_timing_info_before_auth_;
-}
-
 void TestNetworkDelegate::InitRequestStatesIfNew(int request_id) {
   if (next_states_.find(request_id) == next_states_.end()) {
     // TODO(davidben): Although the URLRequest documentation does not allow
@@ -413,8 +405,7 @@ int TestNetworkDelegate::OnBeforeURLRequest(URLRequest* request,
       kStageBeforeStartTransaction |
       kStageResponseStarted |  // data: URLs do not trigger sending headers
       kStageBeforeRedirect |   // a delegate can trigger a redirection
-      kStageCompletedError |   // request canceled by delegate
-      kStageAuthRequired;      // Auth can come next for FTP requests
+      kStageCompletedError;    // request canceled by delegate
   created_requests_++;
   return OK;
 }
@@ -483,7 +474,6 @@ int TestNetworkDelegate::OnHeadersReceived(
   next_states_[req_id] =
       kStageBeforeRedirect |
       kStageResponseStarted |
-      kStageAuthRequired |
       kStageCompletedError;  // e.g. proxy resolution problem
 
   // Basic authentication sends a second request from the URLRequestHttpJob
@@ -616,33 +606,6 @@ void TestNetworkDelegate::OnPACScriptError(int line_number,
                                            const base::string16& error) {
 }
 
-NetworkDelegate::AuthRequiredResponse TestNetworkDelegate::OnAuthRequired(
-    URLRequest* request,
-    const AuthChallengeInfo& auth_info,
-    AuthCallback callback,
-    AuthCredentials* credentials) {
-  load_timing_info_before_auth_ = LoadTimingInfo();
-  request->GetLoadTimingInfo(&load_timing_info_before_auth_);
-  has_load_timing_info_before_auth_ = true;
-  EXPECT_FALSE(load_timing_info_before_auth_.request_start_time.is_null());
-  EXPECT_FALSE(load_timing_info_before_auth_.request_start.is_null());
-
-  int req_id = request->identifier();
-  InitRequestStatesIfNew(req_id);
-  event_order_[req_id] += "OnAuthRequired\n";
-  EXPECT_TRUE(next_states_[req_id] & kStageAuthRequired) <<
-      event_order_[req_id];
-  next_states_[req_id] =
-      kStageBeforeStartTransaction |
-      kStageAuthRequired |  // For example, proxy auth followed by server auth.
-      kStageHeadersReceived |  // Request canceled by delegate simulates empty
-                               // response.
-      kStageResponseStarted |  // data: URLs do not trigger sending headers
-      kStageBeforeRedirect |   // a delegate can trigger a redirection
-      kStageCompletedError;    // request cancelled before callback
-  return NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
-}
-
 bool TestNetworkDelegate::OnCanGetCookies(const URLRequest& request,
                                           const CookieList& cookie_list,
                                           bool allowed_from_caller) {

net/url_request/url_request_test_util.h

@@ -269,11 +269,6 @@ class TestNetworkDelegate : public NetworkDelegateImpl {
   bool GetLoadTimingInfoBeforeRedirect(
       LoadTimingInfo* load_timing_info_before_redirect) const;
 
-  // Same as GetLoadTimingInfoBeforeRedirect, except for calls to
-  // AuthRequiredResponse.
-  bool GetLoadTimingInfoBeforeAuth(
-      LoadTimingInfo* load_timing_info_before_auth) const;
-
   // Will redirect once to the given URL when the next set of headers are
   // received.
   void set_redirect_on_headers_received_url(
@@ -365,11 +360,6 @@ class TestNetworkDelegate : public NetworkDelegateImpl {
   void OnCompleted(URLRequest* request, bool started, int net_error) override;
   void OnURLRequestDestroyed(URLRequest* request) override;
   void OnPACScriptError(int line_number, const base::string16& error) override;
-  NetworkDelegate::AuthRequiredResponse OnAuthRequired(
-      URLRequest* request,
-      const AuthChallengeInfo& auth_info,
-      AuthCallback callback,
-      AuthCredentials* credentials) override;
   bool OnCanGetCookies(const URLRequest& request,
                        const CookieList& cookie_list,
                        bool allowed_from_caller) override;
@@ -419,9 +409,6 @@ class TestNetworkDelegate : public NetworkDelegateImpl {
   LoadTimingInfo load_timing_info_before_redirect_;
   bool has_load_timing_info_before_redirect_;
 
-  LoadTimingInfo load_timing_info_before_auth_;
-  bool has_load_timing_info_before_auth_;
-
   bool experimental_cookie_features_enabled_;           // false by default
   bool cancel_request_with_policy_violating_referrer_;  // false by default
   bool will_be_intercepted_on_next_error_;