Refactor blocking network delegates

net/url_request/url_request_unittest.cc

 // 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.
 
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shlobj.h>
 #endif
@@ skipping 128 matching lines @@
     return false;
 
   for (size_t i = 0; i < size; ++i) {
     if (!a[i].Equals(b[i]))
       return false;
   }
 
   return true;
 }
 
-// A network delegate that blocks requests, optionally cancelling or redirecting
-// them.
+// A network delegate that allows the user to choose a subset of request stages
+// to block in. When blocking, the delegate can do one of the following:
+//  * synchronously return a pre-specified error code, or
+//  * asynchronously return that value via an automatically called callback,
+//    or
+//  * block and wait for the user to do a callback.
+// Additionally, the user may also specify a redirect URL -- then each request
+// with the current URL different from the redirect target will be redirected
+// to that target, in the on-before-URL-request stage, independent of whether
+// the delegate blocks in ON_BEFORE_URL_REQUEST or not.
 class BlockingNetworkDelegate : public TestNetworkDelegate {
  public:
-  BlockingNetworkDelegate()
-      : retval_(ERR_IO_PENDING),
-        callback_retval_(OK),
-        auth_retval_(NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING),
-        auth_callback_retval_(
-            NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION),
-        ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {}
-
-  void set_retval(int retval) { retval_ = retval; }
-  void set_callback_retval(int retval) { callback_retval_ = retval; }
-  void set_redirect_url(const GURL& url) { redirect_url_ = url; }
-  void set_auth_retval(NetworkDelegate::AuthRequiredResponse retval) {
-    auth_retval_ = retval; }
-  void set_auth_callback_retval(NetworkDelegate::AuthRequiredResponse retval) {
-    auth_callback_retval_ = retval; }
-  void set_auth_credentials(const AuthCredentials& auth_credentials) {
-    auth_credentials_ = auth_credentials;
-  }
-
- private:
-  // TestNetworkDelegate implementation.
-  virtual int OnBeforeURLRequest(URLRequest* request,
-                                 const CompletionCallback& callback,
-                                 GURL* new_url) OVERRIDE {
-    if (redirect_url_ == request->url()) {
-      // We've already seen this request and redirected elsewhere.
-      return OK;
-    }
-
-    TestNetworkDelegate::OnBeforeURLRequest(request, callback, new_url);
-
-    if (!redirect_url_.is_empty())
-      *new_url = redirect_url_;
-
-    if (retval_ != ERR_IO_PENDING)
-      return retval_;
-
-    MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&BlockingNetworkDelegate::DoCallback,
-                   weak_factory_.GetWeakPtr(), callback));
-    return ERR_IO_PENDING;
-  }
-
-  virtual NetworkDelegate::AuthRequiredResponse OnAuthRequired(
-      URLRequest* request,
-      const AuthChallengeInfo& auth_info,
-      const AuthCallback& callback,
-      AuthCredentials* credentials) OVERRIDE {
-    TestNetworkDelegate::OnAuthRequired(request, auth_info, callback,
-                                        credentials);
-    switch (auth_retval_) {
-      case NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION:
-        break;
-      case NetworkDelegate::AUTH_REQUIRED_RESPONSE_SET_AUTH:
-        *credentials = auth_credentials_;
-      case NetworkDelegate::AUTH_REQUIRED_RESPONSE_CANCEL_AUTH:
-        break;
-      case NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING:
-        MessageLoop::current()->PostTask(
-            FROM_HERE,
-            base::Bind(&BlockingNetworkDelegate::DoAuthCallback,
-                       weak_factory_.GetWeakPtr(), callback, credentials));
-        break;
-    }
-    return auth_retval_;
-  }
-
-  void DoCallback(const CompletionCallback& callback) {
-    callback.Run(callback_retval_);
-  }
-
-  void DoAuthCallback(const AuthCallback& callback,
-                      AuthCredentials* credentials) {
-    if (auth_callback_retval_ ==
-        NetworkDelegate::AUTH_REQUIRED_RESPONSE_SET_AUTH) {
-      *credentials = auth_credentials_;
-    }
-    callback.Run(auth_callback_retval_);
-  }
-
-
-  int retval_;
-  int callback_retval_;
-  GURL redirect_url_;
-  NetworkDelegate::AuthRequiredResponse auth_retval_;
-  NetworkDelegate::AuthRequiredResponse auth_callback_retval_;
-  AuthCredentials auth_credentials_;
-  base::WeakPtrFactory<BlockingNetworkDelegate> weak_factory_;
-};
-
-// A network delegate that allows blocking requests until a callback function is
-// called.
-class BlockingNetworkDelegateWithManualCallback : public TestNetworkDelegate {
- public:
-  enum State {
+  // Stages in which the delegate can block.
+  enum Stage {
     NOT_BLOCKED = 0,
     ON_BEFORE_URL_REQUEST = 1 << 0,
     ON_BEFORE_SEND_HEADERS = 1 << 1,
     ON_HEADERS_RECEIVED = 1 << 2,
     ON_AUTH_REQUIRED = 1 << 3
   };
 
-  BlockingNetworkDelegateWithManualCallback()
-      : block_on_(0),
-        state_(NOT_BLOCKED) {
+  // Behavior during blocked stages.  During other stages, just
+  // returns net::OK or NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION.
+  enum BlockMode {
+    SYNCHRONOUS,    // No callback, returns specified return values.
+    AUTO_CALLBACK,  // |this| posts a task to run the callback using the
+                    // specified return codes.
+    USER_CALLBACK,  // User takes care of doing a callback.  |retval_| and
+                    // |auth_retval_| are ignored.
+  };
+
+  // Creates a delegate which does not block at all.
+  explicit BlockingNetworkDelegate(BlockMode block_mode);
+
+  // Setters.
+  void set_retval(int retval) {
+    ASSERT_NE(ERR_IO_PENDING, retval);
+    ASSERT_NE(OK, retval);

[mmenke, 2012/09/20 15:37:52]

OK actually makes sense, if BlockMode is AUTO_CALLBACK.

[vabr (Chromium), 2012/09/21 11:11:36]

But in that case OK is the default value.
Until there is a situation where |retval_| would be set to non-OK on an early
stage, and then reset to OK, I would leave the ASSERT here.

+    retval_ = retval;
   }
 
-  // Activates blocking on |state|.
-  void BlockOn(State state) {
-    block_on_ |= state;
+  // If |auth_retval| == AUTH_REQUIRED_RESPONSE_SET_AUTH, then
+  // |auth_credentials_| will be passed with the response.
+  void set_auth_retval(AuthRequiredResponse auth_retval) {
+    ASSERT_NE(AUTH_REQUIRED_RESPONSE_IO_PENDING, auth_retval);
+    auth_retval_ = auth_retval;
+  }
+  void set_auth_credentials(const AuthCredentials& auth_credentials) {
+    auth_credentials_ = auth_credentials;
   }
 
-  void DoCallback(int rv) {
-    ASSERT_NE(NOT_BLOCKED, state_);
-    CompletionCallback callback = callback_;
-    Reset();
-    callback.Run(rv);
+  void set_redirect_url(const GURL& url) {
+    redirect_url_ = url;
   }
 
-  void DoAuthCallback(NetworkDelegate::AuthRequiredResponse response) {
-    ASSERT_EQ(ON_AUTH_REQUIRED, state_);
-    AuthCallback auth_callback = auth_callback_;
-    Reset();
-    auth_callback.Run(response);
+  // Add a stage to block on. Unless |stage| is NOT_BLOCKED or ON_AUTH_REQUIRED,
+  // it is mandatory to call set_retval().
+  void BlockOn(Stage stage) {
+    block_on_ |= stage;
   }
 
-  // Runs the message loop until |state| is reached.
-  void WaitForState(State state) {
-    while (state_ != state)
+  // For users to trigger a callback returning |response|.
+  // Side-effects: resets |stage_blocked_for_callback_| and stored callbacks.
+  // Only call if |block_mode_| == USER_CALLBACK.
+  void DoCallback(int response);
+  void DoAuthCallback(NetworkDelegate::AuthRequiredResponse response);
+
+  // Runs the message loop until |stage| is reached.
+  void WaitForState(Stage stage) {
+    ASSERT_EQ(USER_CALLBACK, block_mode_);
+    while (stage_blocked_for_callback_ != stage)
       MessageLoop::current()->RunAllPending();
   }
 
  private:
   // TestNetworkDelegate implementation.
   virtual int OnBeforeURLRequest(URLRequest* request,
                                  const CompletionCallback& callback,
-                                 GURL* new_url) OVERRIDE {
-    TestNetworkDelegate::OnBeforeURLRequest(request, callback, new_url);
-    if ((block_on_ & ON_BEFORE_URL_REQUEST) == 0) {
-      return OK;
-    } else {
-      state_ = ON_BEFORE_URL_REQUEST;
-      callback_ = callback;
-      return ERR_IO_PENDING;
-    }
-  }
+                                 GURL* new_url) OVERRIDE;
 
   virtual int OnBeforeSendHeaders(URLRequest* request,
                                   const CompletionCallback& callback,
-                                  HttpRequestHeaders* headers) OVERRIDE {
-    TestNetworkDelegate::OnBeforeSendHeaders(request, callback, headers);
-    if ((block_on_ & ON_BEFORE_SEND_HEADERS) == 0) {
-      return OK;
-    } else {
-      state_ = ON_BEFORE_SEND_HEADERS;
-      callback_ = callback;
-      return ERR_IO_PENDING;
-    }
-  }
+                                  HttpRequestHeaders* headers) OVERRIDE;
 
   virtual int OnHeadersReceived(
       URLRequest* request,
       const CompletionCallback& callback,
       HttpResponseHeaders* original_response_headers,
-      scoped_refptr<HttpResponseHeaders>* override_response_headers)
-      OVERRIDE {
-    TestNetworkDelegate::OnHeadersReceived(
-        request, callback, original_response_headers,
-        override_response_headers);
-    if ((block_on_ & ON_HEADERS_RECEIVED) == 0) {
-      return OK;
-    } else {
-      state_ = ON_HEADERS_RECEIVED;
-      callback_ = callback;
-      return ERR_IO_PENDING;
-    }
-  }
+      scoped_refptr<HttpResponseHeaders>* override_response_headers) OVERRIDE;
 
   virtual NetworkDelegate::AuthRequiredResponse OnAuthRequired(
       URLRequest* request,
       const AuthChallengeInfo& auth_info,
       const AuthCallback& callback,
-      AuthCredentials* credentials) OVERRIDE {
-    if ((block_on_ & ON_AUTH_REQUIRED) == 0) {
-      return NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
-    } else {
-      state_ = ON_AUTH_REQUIRED;
-      auth_callback_ = callback;
-      return NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING;
-    }
-  }
+      AuthCredentials* credentials) OVERRIDE;
 
   void Reset() {
-    state_ = NOT_BLOCKED;
+    stage_blocked_for_callback_ = NOT_BLOCKED;
     callback_.Reset();
     auth_callback_.Reset();
   }
 
-  int block_on_;  // Bit mask on which states to block.
-  State state_;
+  void RunCallback(int response, const CompletionCallback& callback);
+  void RunAuthCallback(AuthRequiredResponse response,
+                       const AuthCallback& callback);
+
+  // Checks whether we should block in |stage|. If yes, returns an error code
+  // and optionally sets up callback based on |block_mode_|. If no, returns OK.
+  int MaybeBlockStage(Stage stage, const CompletionCallback& callback);
+
+  // Configuration parameters:
+  const BlockMode block_mode_;
+
+  // Values returned on blocking stages when mode is SYNCHRONOUS or
+  // AUTO_CALLBACK.
+  int retval_;  // To be returned in non-auth stages.
+  AuthRequiredResponse auth_retval_;
+
+  GURL redirect_url_;  // Used if non-empty.
+  int block_on_;  // Bit mask: in which stages to block.
+
+  // |auth_credentials_| will be copied to |*target_auth_credential_| on
+  // callback.
+  AuthCredentials auth_credentials_;
+  AuthCredentials* target_auth_credentials_;
+
+  // Internal parameters:
+  Stage stage_blocked_for_callback_;  // In which stage did we block?
+
+  // Callback objects stored during blocking stages.
   CompletionCallback callback_;
   AuthCallback auth_callback_;
+
+  base::WeakPtrFactory<BlockingNetworkDelegate> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlockingNetworkDelegate);
 };
 
+BlockingNetworkDelegate::BlockingNetworkDelegate(BlockMode block_mode)
+    : block_mode_(block_mode),
+      retval_(block_mode == USER_CALLBACK ? ERR_IO_PENDING : OK),
+      auth_retval_(block_mode == USER_CALLBACK ?
+                   AUTH_REQUIRED_RESPONSE_IO_PENDING :
+                   AUTH_REQUIRED_RESPONSE_NO_ACTION),
+      block_on_(0),
+      target_auth_credentials_(NULL),
+      stage_blocked_for_callback_(NOT_BLOCKED),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
+}
+
+int BlockingNetworkDelegate::OnBeforeURLRequest(
+    URLRequest* request,
+    const CompletionCallback& callback,
+    GURL* new_url) {
+  if (redirect_url_ == request->url())
+    return OK;  // We've already seen this request and redirected elsewhere.
+
+  TestNetworkDelegate::OnBeforeURLRequest(request, callback, new_url);
+
+  if (!redirect_url_.is_empty())
+    *new_url = redirect_url_;
+
+  return MaybeBlockStage(ON_BEFORE_URL_REQUEST, callback);
+}
+
+int BlockingNetworkDelegate::OnBeforeSendHeaders(
+    URLRequest* request,
+    const CompletionCallback& callback,
+    HttpRequestHeaders* headers) {
+  TestNetworkDelegate::OnBeforeSendHeaders(request, callback, headers);
+
+  return MaybeBlockStage(ON_BEFORE_SEND_HEADERS, callback);
+}
+
+int BlockingNetworkDelegate::OnHeadersReceived(
+    URLRequest* request,
+    const CompletionCallback& callback,
+    HttpResponseHeaders* original_response_headers,
+    scoped_refptr<HttpResponseHeaders>* override_response_headers) {
+  TestNetworkDelegate::OnHeadersReceived(
+      request, callback, original_response_headers,
+      override_response_headers);
+
+  return MaybeBlockStage(ON_HEADERS_RECEIVED, callback);
+}
+
+NetworkDelegate::AuthRequiredResponse BlockingNetworkDelegate::OnAuthRequired(
+    URLRequest* request,
+    const AuthChallengeInfo& auth_info,
+    const AuthCallback& callback,
+    AuthCredentials* credentials) {
+  TestNetworkDelegate::OnAuthRequired(request, auth_info, callback,
+                                      credentials);
+  EXPECT_EQ(NOT_BLOCKED, stage_blocked_for_callback_);
+
+  if ((block_on_ & ON_AUTH_REQUIRED) == 0) {
+    return AUTH_REQUIRED_RESPONSE_NO_ACTION;
+  }
+
+  if (block_mode_ == USER_CALLBACK)
+    stage_blocked_for_callback_ = ON_AUTH_REQUIRED;
+
+  if (auth_retval_ == AUTH_REQUIRED_RESPONSE_SET_AUTH)
+    target_auth_credentials_ = credentials;
+
+  switch (block_mode_) {
+    case SYNCHRONOUS:
+      if (auth_retval_ == AUTH_REQUIRED_RESPONSE_SET_AUTH)
+        *target_auth_credentials_ = auth_credentials_;
+      return auth_retval_;
+
+    case AUTO_CALLBACK:
+      MessageLoop::current()->PostTask(
+          FROM_HERE,
+          base::Bind(&BlockingNetworkDelegate::RunAuthCallback,
+                     weak_factory_.GetWeakPtr(), auth_retval_, callback));
+      return AUTH_REQUIRED_RESPONSE_IO_PENDING;
+
+    case USER_CALLBACK:
+      auth_callback_ = callback;
+      return AUTH_REQUIRED_RESPONSE_IO_PENDING;
+  }
+  NOTREACHED();
+  return AUTH_REQUIRED_RESPONSE_NO_ACTION;  // Dummy value.
+}
+
+void BlockingNetworkDelegate::DoCallback(int response) {
+  ASSERT_EQ(block_mode_, USER_CALLBACK);

[mmenke, 2012/09/20 15:37:52]

nit:  Reverse order

[vabr (Chromium), 2012/09/21 11:11:36]

Done.

+  ASSERT_NE(NOT_BLOCKED, stage_blocked_for_callback_);
+  ASSERT_NE(ON_AUTH_REQUIRED, stage_blocked_for_callback_);
+  CompletionCallback callback = callback_;
+  Reset();
+  RunCallback(response, callback);
+}
+
+void BlockingNetworkDelegate::DoAuthCallback(
+    NetworkDelegate::AuthRequiredResponse response) {
+  ASSERT_EQ(block_mode_, USER_CALLBACK);

[mmenke, 2012/09/20 15:37:52]

nit:  Reverse order

[vabr (Chromium), 2012/09/21 11:11:36]

Done.

+  ASSERT_EQ(ON_AUTH_REQUIRED, stage_blocked_for_callback_);
+  AuthCallback auth_callback = auth_callback_;
+  Reset();
+  RunAuthCallback(response, auth_callback);
+}
+
+void BlockingNetworkDelegate::RunCallback(int response,
+                                          const CompletionCallback& callback) {
+  callback.Run(response);
+}
+
+void BlockingNetworkDelegate::RunAuthCallback(AuthRequiredResponse response,
+                                                 const AuthCallback& callback) {
+  if (auth_retval_ == AUTH_REQUIRED_RESPONSE_SET_AUTH) {
+    ASSERT_TRUE(target_auth_credentials_ != NULL);
+    *target_auth_credentials_ = auth_credentials_;
+  }
+  callback.Run(response);
+}
+
+int BlockingNetworkDelegate::MaybeBlockStage(
+    BlockingNetworkDelegate::Stage stage,
+    const CompletionCallback& callback) {
+  EXPECT_EQ(NOT_BLOCKED, stage_blocked_for_callback_);
+
+  if ((block_on_ & stage) == 0) {
+    return OK;
+  }
+
+  EXPECT_NE(OK, retval_);

[mmenke, 2012/09/20 15:37:52]

OK is actually OK for the AUTO_CALLBACK case.  Doesn't matter in the
USER_CALLBACK case, so suggest you move this to the SYNCHRONOUS case.

[vabr (Chromium), 2012/09/21 11:11:36]

Done.

+  if (block_mode_ != SYNCHRONOUS)
+    stage_blocked_for_callback_ = stage;
+
+  switch (block_mode_) {
+    case SYNCHRONOUS:
+      return retval_;
+
+    case AUTO_CALLBACK:
+      MessageLoop::current()->PostTask(
+          FROM_HERE,
+          base::Bind(&BlockingNetworkDelegate::RunCallback,
+                     weak_factory_.GetWeakPtr(), retval_, callback));
+      return ERR_IO_PENDING;
+
+    case USER_CALLBACK:
+      callback_ = callback;
+      return ERR_IO_PENDING;
+  }
+  NOTREACHED();
+  return 0;
+}
+
 class TestURLRequestContextWithProxy : public TestURLRequestContext {
  public:
   // Does not own |delegate|.
   TestURLRequestContextWithProxy(const std::string& proxy,
                                  NetworkDelegate* delegate)
       : TestURLRequestContext(true) {
     context_storage_.set_proxy_service(ProxyService::CreateFixed(proxy));
     set_network_delegate(delegate);
     Init();
   }
@@ skipping 1539 matching lines @@
     EXPECT_EQ(1, network_delegate.error_count());
     EXPECT_EQ(ERR_TUNNEL_CONNECTION_FAILED, network_delegate.last_error());
   }
 }
 
 // Tests that the network delegate can block and cancel a request.
 TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequest) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
-  network_delegate.set_callback_retval(ERR_EMPTY_RESPONSE);
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
+  network_delegate.set_retval(ERR_EMPTY_RESPONSE);
 
   TestURLRequestContextWithProxy context(
       test_server_.host_port_pair().ToString(),
       &network_delegate);
 
   {
     URLRequest r(test_server_.GetURL(""), &d, &context);
 
     r.Start();
     MessageLoop::current()->Run();
 
     EXPECT_EQ(URLRequestStatus::FAILED, r.status().status());
     EXPECT_EQ(ERR_EMPTY_RESPONSE, r.status().error());
     EXPECT_EQ(1, network_delegate.created_requests());
     EXPECT_EQ(0, network_delegate.destroyed_requests());
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that the network delegate can cancel a request synchronously.
 TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestSynchronously) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);

[mmenke, 2012/09/20 15:37:52]

This changes behavior.  It should be SYNCHRONOUS.

[vabr (Chromium), 2012/09/21 11:11:36]

Done.

+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
   network_delegate.set_retval(ERR_EMPTY_RESPONSE);
 
   TestURLRequestContextWithProxy context(
       test_server_.host_port_pair().ToString(),
       &network_delegate);
 
   {
     URLRequest r(test_server_.GetURL(""), &d, &context);
 
     r.Start();
     MessageLoop::current()->Run();
 
     EXPECT_EQ(URLRequestStatus::FAILED, r.status().status());
     EXPECT_EQ(ERR_EMPTY_RESPONSE, r.status().error());
     EXPECT_EQ(1, network_delegate.created_requests());
     EXPECT_EQ(0, network_delegate.destroyed_requests());
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that the network delegate can block and redirect a request to a new
 // URL.
 TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequest) {

[vabr (Chromium), 2012/09/20 11:45:44]

Given that redirect now is done independently of blocking, I am not sure what
should be the difference between NetworkDelegateRedirectRequest and
NetworkDelegateRedirectRequestSynchronously (below).

Matt, do you have any idea what the difference should be, or should I just
delete one of those two tests?

[mmenke, 2012/09/20 15:37:52]

Ahh...There's a bug here.  This should use
network_delegate.BlockOn(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST); to
have the same behavior as before.

In fact, to exactly preserve behavior, all the AUTO_CALLBACK tests should do
this, except those that used to call "network_delegate.set_retval(OK);", since
the default behavior used to be async in ON_BEFORE_URL_REQUEST.

All the AUTO_CALLBACK tests that used to use set_auth_retval should by
SYNCHRONOUS and block on ON_AUTH_REQUIRED, and those that use
set_auth_callback_retval should by AUTO_CALLBACK.

I've noted a couple cases where the wrong mode is used, though haven't checked
them all.

[mmenke, 2012/09/20 15:40:49]

Sorry - comment below is more accurate than the one above, and also applies to
set_retval / set_callback_retval / ON_BEFORE_URL_REQUEST.

[vabr (Chromium), 2012/09/21 11:11:36]

Done.

   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);
   GURL redirect_url(test_server_.GetURL("simple.html"));
   network_delegate.set_redirect_url(redirect_url);
 
   TestURLRequestContextWithProxy context(
       test_server_.host_port_pair().ToString(),
       &network_delegate);
 
   {
     GURL original_url(test_server_.GetURL("empty.html"));
     URLRequest r(original_url, &d, &context);
@@ skipping 11 matching lines @@
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that the network delegate can block and redirect a request to a new
 // URL by setting a redirect_url and returning in OnBeforeURLRequest directly.
 TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequestSynchronously) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::SYNCHRONOUS);
   GURL redirect_url(test_server_.GetURL("simple.html"));
   network_delegate.set_redirect_url(redirect_url);
-  network_delegate.set_retval(OK);
 
   TestURLRequestContextWithProxy context(
       test_server_.host_port_pair().ToString(),
       &network_delegate);
 
   {
     GURL original_url(test_server_.GetURL("empty.html"));
     URLRequest r(original_url, &d, &context);
 
     r.Start();
@@ skipping 10 matching lines @@
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that redirects caused by the network delegate preserve POST data.
 TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequestPost) {
   ASSERT_TRUE(test_server_.Start());
 
   const char kData[] = "hello world";
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::SYNCHRONOUS);

[mmenke, 2012/09/20 15:37:52]

This changes behavior.  It should be AUTO_CALLBACK, with retval of OK (Which is
default).

[mmenke, 2012/09/20 15:37:52]

If we want to preserve behavior here, we should make this async and block on
headers, but don't think that's needed.

[vabr (Chromium), 2012/09/21 11:11:36]

Why headers? I don't think the original BlockingNetworkDelegate could block on
sending headers. Did you mean onAuthRequired?

[vabr (Chromium), 2012/09/21 11:11:36]

Done.

[mmenke, 2012/09/21 15:15:59]

Err...I meant on before send, actually (Hence the comment about it not
mattering).

   GURL redirect_url(test_server_.GetURL("echo"));
   network_delegate.set_redirect_url(redirect_url);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   {
     GURL original_url(test_server_.GetURL("empty.html"));
     URLRequest r(original_url, &d, &context);
@@ skipping 20 matching lines @@
 }
 
 // Tests that the network delegate can synchronously complete OnAuthRequired
 // by taking no action. This indicates that the NetworkDelegate does not want to
 // handle the challenge, and is passing the buck along to the
 // URLRequest::Delegate.
 TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredSyncNoAction) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
-  network_delegate.set_auth_retval(
-      NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION);
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);

[mmenke, 2012/09/20 15:37:52]

This should be SYNCHRONOUS.

[vabr (Chromium), 2012/09/21 11:11:36]

Done.

+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_AUTH_REQUIRED);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   d.set_credentials(AuthCredentials(kUser, kSecret));
 
   {
     GURL url(test_server_.GetURL("auth-basic"));
     URLRequest r(url, &d, &context);
     r.Start();
     MessageLoop::current()->Run();
 
     EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status());
     EXPECT_EQ(0, r.status().error());
     EXPECT_EQ(200, r.GetResponseCode());
     EXPECT_TRUE(d.auth_required_called());
     EXPECT_EQ(1, network_delegate.created_requests());
     EXPECT_EQ(0, network_delegate.destroyed_requests());
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that the network delegate can synchronously complete OnAuthRequired
 // by setting credentials.
 TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredSyncSetAuth) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);

[mmenke, 2012/09/20 15:37:52]

SYNCHRONOUS

[vabr (Chromium), 2012/09/21 11:11:36]

Done.

+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_AUTH_REQUIRED);
   network_delegate.set_auth_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_SET_AUTH);
 
   network_delegate.set_auth_credentials(AuthCredentials(kUser, kSecret));
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   {
@@ skipping 11 matching lines @@
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that the network delegate can synchronously complete OnAuthRequired
 // by cancelling authentication.
 TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredSyncCancel) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);

[mmenke, 2012/09/20 15:37:52]

SYNCHRONOUS

[vabr (Chromium), 2012/09/21 11:11:36]

Done.

+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_AUTH_REQUIRED);
   network_delegate.set_auth_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_CANCEL_AUTH);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   {
     GURL url(test_server_.GetURL("auth-basic"));
     URLRequest r(url, &d, &context);
@@ skipping 11 matching lines @@
 }
 
 // Tests that the network delegate can asynchronously complete OnAuthRequired
 // by taking no action. This indicates that the NetworkDelegate does not want
 // to handle the challenge, and is passing the buck along to the
 // URLRequest::Delegate.
 TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredAsyncNoAction) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_AUTH_REQUIRED);
   network_delegate.set_auth_retval(
-      NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING);
-  network_delegate.set_auth_callback_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   d.set_credentials(AuthCredentials(kUser, kSecret));
 
   {
     GURL url(test_server_.GetURL("auth-basic"));
@@ skipping 10 matching lines @@
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that the network delegate can asynchronously complete OnAuthRequired
 // by setting credentials.
 TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredAsyncSetAuth) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_AUTH_REQUIRED);
   network_delegate.set_auth_retval(
-      NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING);
-  network_delegate.set_auth_callback_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_SET_AUTH);
 
   AuthCredentials auth_credentials(kUser, kSecret);
   network_delegate.set_auth_credentials(auth_credentials);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   {
@@ skipping 12 matching lines @@
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that the network delegate can asynchronously complete OnAuthRequired
 // by cancelling authentication.
 TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredAsyncCancel) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_AUTH_REQUIRED);
   network_delegate.set_auth_retval(
-      NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING);
-  network_delegate.set_auth_callback_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_CANCEL_AUTH);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   {
     GURL url(test_server_.GetURL("auth-basic"));
     URLRequest r(url, &d, &context);
     r.Start();
     MessageLoop::current()->Run();
 
     EXPECT_EQ(URLRequestStatus::SUCCESS, r.status().status());
     EXPECT_EQ(OK, r.status().error());
     EXPECT_EQ(401, r.GetResponseCode());
     EXPECT_FALSE(d.auth_required_called());
     EXPECT_EQ(1, network_delegate.created_requests());
     EXPECT_EQ(0, network_delegate.destroyed_requests());
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that we can handle when a network request was canceled while we were
 // waiting for the network delegate.
 // Part 1: Request is cancelled while waiting for OnBeforeURLRequest callback.
 TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting1) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegateWithManualCallback network_delegate;
-  network_delegate.BlockOn(
-      BlockingNetworkDelegateWithManualCallback::ON_BEFORE_URL_REQUEST);
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::USER_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   {
     URLRequest r(test_server_.GetURL(""), &d, &context);
 
     r.Start();
     network_delegate.WaitForState(
-        BlockingNetworkDelegateWithManualCallback::ON_BEFORE_URL_REQUEST);
+        BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
     EXPECT_EQ(0, network_delegate.completed_requests());
     // Cancel before callback.
     r.Cancel();
     // Ensure that network delegate is notified.
     EXPECT_EQ(1, network_delegate.completed_requests());
     EXPECT_EQ(URLRequestStatus::CANCELED, r.status().status());
     EXPECT_EQ(ERR_ABORTED, r.status().error());
     EXPECT_EQ(1, network_delegate.created_requests());
     EXPECT_EQ(0, network_delegate.destroyed_requests());
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that we can handle when a network request was canceled while we were
 // waiting for the network delegate.
 // Part 2: Request is cancelled while waiting for OnBeforeSendHeaders callback.
 TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting2) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegateWithManualCallback network_delegate;
-  network_delegate.BlockOn(
-      BlockingNetworkDelegateWithManualCallback::ON_BEFORE_SEND_HEADERS);
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::USER_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_BEFORE_SEND_HEADERS);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   {
     URLRequest r(test_server_.GetURL(""), &d, &context);
 
     r.Start();
     network_delegate.WaitForState(
-        BlockingNetworkDelegateWithManualCallback::ON_BEFORE_SEND_HEADERS);
+        BlockingNetworkDelegate::ON_BEFORE_SEND_HEADERS);
     EXPECT_EQ(0, network_delegate.completed_requests());
     // Cancel before callback.
     r.Cancel();
     // Ensure that network delegate is notified.
     EXPECT_EQ(1, network_delegate.completed_requests());
     EXPECT_EQ(URLRequestStatus::CANCELED, r.status().status());
     EXPECT_EQ(ERR_ABORTED, r.status().error());
     EXPECT_EQ(1, network_delegate.created_requests());
     EXPECT_EQ(0, network_delegate.destroyed_requests());
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that we can handle when a network request was canceled while we were
 // waiting for the network delegate.
 // Part 3: Request is cancelled while waiting for OnHeadersReceived callback.
 TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting3) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegateWithManualCallback network_delegate;
-  network_delegate.BlockOn(
-      BlockingNetworkDelegateWithManualCallback::ON_HEADERS_RECEIVED);
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::USER_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_HEADERS_RECEIVED);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   {
     URLRequest r(test_server_.GetURL(""), &d, &context);
 
     r.Start();
     network_delegate.WaitForState(
-        BlockingNetworkDelegateWithManualCallback::ON_HEADERS_RECEIVED);
+        BlockingNetworkDelegate::ON_HEADERS_RECEIVED);
     EXPECT_EQ(0, network_delegate.completed_requests());
     // Cancel before callback.
     r.Cancel();
     // Ensure that network delegate is notified.
     EXPECT_EQ(1, network_delegate.completed_requests());
     EXPECT_EQ(URLRequestStatus::CANCELED, r.status().status());
     EXPECT_EQ(ERR_ABORTED, r.status().error());
     EXPECT_EQ(1, network_delegate.created_requests());
     EXPECT_EQ(0, network_delegate.destroyed_requests());
   }
   EXPECT_EQ(1, network_delegate.destroyed_requests());
 }
 
 // Tests that we can handle when a network request was canceled while we were
 // waiting for the network delegate.
 // Part 4: Request is cancelled while waiting for OnAuthRequired callback.
 TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting4) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegateWithManualCallback network_delegate;
-  network_delegate.BlockOn(
-      BlockingNetworkDelegateWithManualCallback::ON_AUTH_REQUIRED);
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::USER_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_AUTH_REQUIRED);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
   context.Init();
 
   {
     URLRequest r(test_server_.GetURL("auth-basic"), &d, &context);
 
     r.Start();
     network_delegate.WaitForState(
-        BlockingNetworkDelegateWithManualCallback::ON_AUTH_REQUIRED);
+        BlockingNetworkDelegate::ON_AUTH_REQUIRED);
     EXPECT_EQ(0, network_delegate.completed_requests());
     // Cancel before callback.
     r.Cancel();
     // Ensure that network delegate is notified.
     EXPECT_EQ(1, network_delegate.completed_requests());
     EXPECT_EQ(URLRequestStatus::CANCELED, r.status().status());
     EXPECT_EQ(ERR_ABORTED, r.status().error());
     EXPECT_EQ(1, network_delegate.created_requests());
     EXPECT_EQ(0, network_delegate.destroyed_requests());
   }
@@ skipping 2249 matching lines @@
 
     EXPECT_FALSE(r.is_pending());
     EXPECT_EQ(1, d->response_started_count());
     EXPECT_FALSE(d->received_data_before_response());
     EXPECT_EQ(d->bytes_received(), static_cast<int>(file_size));
   }
 }
 #endif  // !defined(DISABLE_FTP_SUPPORT)
 
 }  // namespace net