Refactor blocking network delegates

net/url_request/url_request_unittest.cc

Index: net/url_request/url_request_unittest.cc
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index c1cefe40e2a65f3d1ca1f3b20778b4ce7b4d678d..329382557e2bb611eeb1f66092978ec8c3244e73 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.

[mmenke, 2012/09/19 19:39:47]

nit:  The new rule is to leave this alone, and last I checked, it's still 2012. 
Also, I may not be the fastest reviewer in the world, but I'm not *that* slow. 
:)

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

:D
I swear I did not touch that! Not sure how that number got there. I'll simply
revert it manually to the current year.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -146,106 +146,20 @@ bool FingerprintsEqual(const HashValueVector& a, const HashValueVector& b) {
   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. On blocking, the delegate allows either:

[mmenke, 2012/09/19 19:39:47]

nit:  For readability, suggest you replace the second sentence with "When
blocking, the delegate can do one of the following:", and removing the "to" from
the following lines.

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

Done.

+//  * to synchronously return a pre-specified error code, or
+//  * to asynchronously return that value via an automatically called callback,
+//    or
+//  * to 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 {

[mmenke, 2012/09/19 19:39:47]

TestNetworkDelegate posts a task to quit the current message loop in
OnResponseCompleted, by default.  All the non-USER_CALLBACK tests expect this.

The USER_CALLBACK tests do not - They use the WaitForWhatever function instead
of running the message loop.  We could change the behavior for the USER_CALLBACK
tests to set TestDelegate::quit_on_complete_.

Another option would be to just run the message loop at the end of the
USER_CALLBACK tests, which would make them more consistent with the other tests.
 I believe cancelling should result in calling OnResponseCompleted.

We could just replace USER_CALLBACK with ASYNC_CANCEL, since all the
USER_CALLBACK tests do the same thing, but we'd lose flexibility that way.


This behavior is sufficiently non-obvious that I suggest adding a comments to
both BlockingNetworkDelegate and TestNetworkDelegate about it.

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

I'm a bit confused: are we speaking about TestDelegate or TestNetworkDelegate?

On 2012/09/19 19:39:47, Matt Menke wrote:
This happens on TestDelegate, TestNetworkDelegate does not have that method.
I'm not clear on how to do it. The URLrequest has a link to the TestDelegate,
but does not expose it. Did you have in mind extending the
BlockingNetworkDelegate to take a link to a TestDelegate on constriction, and
set it through that?
Here at least I have an idea how to proceed. Only I don't fully get the problem
-- why is it so important that the message loop is run? I guess if quitting it
in TestDelegate::OnResponseCompleted would be a problem, we would already have
seen some asserts firing, no?
I'm hesitant to lose the flexibility until I fully understand the problem above
and check that the other solutions are worse.
I fully second that, as I'm confused already. :) I'll add comments once I
understand the thing and implement the solution.

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

Sorry, I got them confused.
We actually create the TestDelegate in the individual tests, so can call it
directly, rather than through the TestNetworkDelegate.  Looks like the
TestDelegate is used in a number of other cases without worrying about that quit
message, so I'm probably just being overly paranoid here.

Just forget about this comment.  :)

  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,
@@ -253,33 +167,56 @@ class BlockingNetworkDelegateWithManualCallback : public TestNetworkDelegate {
     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| takes care of doing a callback using the specified
+                    // return codes.

[mmenke, 2012/09/19 19:39:47]

nit:  "|this| posts a task to run the callback using..."

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

Done.

+    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 mode);

[mmenke, 2012/09/19 19:39:47]

nit:  Just for consistency with the class field name, suggest |block_mode|.

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

Done.

+
+  // Setters.
+  void set_retval(int retval) {
+    ASSERT_NE(ERR_IO_PENDING, retval);
+    ASSERT_NE(OK, retval);
+    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();
   }
 
@@ -287,74 +224,218 @@ class BlockingNetworkDelegateWithManualCallback : public TestNetworkDelegate {
   // 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 mode)
+    : block_mode_(mode),
+      retval_(mode == USER_CALLBACK ? ERR_IO_PENDING : OK),
+      auth_retval_(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);
+  stage_blocked_for_callback_ = NOT_BLOCKED;

[mmenke, 2012/09/19 19:39:47]

Don't think there's any way for this not to be true at this point, since it's
reset in Do[Auth]Callback.  May even want to EXPECT on it.

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

Done.

+
+  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);
+  ASSERT_NE(NOT_BLOCKED, stage_blocked_for_callback_);

[mmenke, 2012/09/19 19:39:47]

nit:  ASSERT_NE(ON_AUTH_REQUIRED, stage_blocked_for_callback_);

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

Done.

+  CompletionCallback callback = callback_;
+  Reset();
+  RunCallback(response, callback);
+}
+
+void BlockingNetworkDelegate::DoAuthCallback(
+    NetworkDelegate::AuthRequiredResponse response) {
+  ASSERT_EQ(block_mode_, USER_CALLBACK);
+  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) {
+  stage_blocked_for_callback_ = NOT_BLOCKED;

[mmenke, 2012/09/19 19:39:47]

Don't think there's any way for this not to be true at this point, since it's
reset in Do[Auth]Callback.  May even want to EXPECT on it.

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

Done.

+
+  if ((block_on_ & stage) == 0) {
+    return OK;
+  }
+
+  if (redirect_url_.is_empty() || stage != ON_BEFORE_URL_REQUEST)

[mmenke, 2012/09/19 19:39:47]

This if statement isn't needed anymore, is it?  If redirect_url_ is set and
retval_ is OK, then we don't need to be blocking on ON_BEFORE_URL_REQUEST.  If
any tests currently do this, can just have them not block on that stage.

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

Done.

+    EXPECT_NE(OK, retval_);
+  if (block_mode_ == USER_CALLBACK)
+    stage_blocked_for_callback_ = stage;

[mmenke, 2012/09/19 19:39:47]

optional:  Do we want to set this on AUTO_CALLBACK as well?  Doesn't really
matter, either way, but might be a little more consistent.

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

Done.

+
+  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|.
@@ -1914,8 +1995,10 @@ 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(),
@@ -1940,7 +2023,9 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestSynchronously) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
   network_delegate.set_retval(ERR_EMPTY_RESPONSE);
 
   TestURLRequestContextWithProxy context(
@@ -1967,7 +2052,9 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequest) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
   GURL redirect_url(test_server_.GetURL("simple.html"));
   network_delegate.set_redirect_url(redirect_url);
 
@@ -1999,10 +2086,11 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequestSynchronously) {
   ASSERT_TRUE(test_server_.Start());
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
   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(),
@@ -2033,7 +2121,9 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequestPost) {
   const char kData[] = "hello world";
 
   TestDelegate d;
-  BlockingNetworkDelegate network_delegate;
+  BlockingNetworkDelegate network_delegate(
+      BlockingNetworkDelegate::AUTO_CALLBACK);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_BEFORE_URL_REQUEST);
   GURL redirect_url(test_server_.GetURL("echo"));
   network_delegate.set_redirect_url(redirect_url);
 
@@ -2074,9 +2164,9 @@ 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);
+  network_delegate.BlockOn(BlockingNetworkDelegate::ON_AUTH_REQUIRED);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
@@ -2106,7 +2196,9 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredSyncSetAuth) {
   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_SET_AUTH);
 
@@ -2138,7 +2230,9 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredSyncCancel) {
   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_CANCEL_AUTH);
 
@@ -2170,10 +2264,10 @@ 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);
@@ -2204,10 +2298,10 @@ 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);
@@ -2240,10 +2334,10 @@ 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);
@@ -2273,9 +2367,9 @@ 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);
@@ -2286,7 +2380,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting1) {
 
     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();
@@ -2307,9 +2401,9 @@ 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);
@@ -2320,7 +2414,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting2) {
 
     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();
@@ -2341,9 +2435,9 @@ 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);
@@ -2354,7 +2448,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting3) {
 
     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();
@@ -2375,9 +2469,9 @@ 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);
@@ -2388,7 +2482,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting4) {
 
     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();