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 17f60297aed0ae98ead58b5435323a1d9be97dfe..aa1772217bcdd468eb7371d3df4fb1b0862388c5 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -146,29 +146,177 @@ bool FingerprintsEqual(const HashValueVector& a, const HashValueVector& b) {
   return true;
 }
 
+// A common base for BlockingNetworkDelegate* classes. Takes care of
+// when-to-block and what-to return book-keeping.
+class BlockingNetworkDelegateBase : public TestNetworkDelegate {
+ public:
+  enum State {  // When to block.
+    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
+  };
+
+  void set_retval(int retval) {
+    retval_ = retval;
+  }
+
+  State state() {
+    return state_;
+  }
+
+  // Activates blocking on |state|.
+  void BlockOn(State state) {
+    block_on_ |= state;
+  }
+
+  // Runs the message loop until |state| is reached.
+  void WaitForState(State state) {
+    while (state_ != state)
+      MessageLoop::current()->RunAllPending();
+  }
+
+ protected:
+  BlockingNetworkDelegateBase(int block_on,
+                              int retval,
+                              bool call_base_on_auth_required);
+
+  void ResetState() {
+    state_ = NOT_BLOCKED;
+  }
+
+  // TestNetworkDelegate implementation.
+  virtual int OnBeforeURLRequest(URLRequest* request,
+                                 const CompletionCallback& callback,
+                                 GURL* new_url) OVERRIDE;
+
+  virtual int OnBeforeSendHeaders(URLRequest* request,
+                                  const CompletionCallback& callback,
+                                  HttpRequestHeaders* headers) OVERRIDE;
+
+  virtual int OnHeadersReceived(
+      URLRequest* request,
+      const CompletionCallback& callback,
+      HttpResponseHeaders* original_response_headers,
+      scoped_refptr<HttpResponseHeaders>* override_response_headers) OVERRIDE;
+
+  virtual NetworkDelegate::AuthRequiredResponse OnAuthRequired(
+      URLRequest* request,
+      const AuthChallengeInfo& auth_info,
+      const AuthCallback& callback,
+      AuthCredentials* credentials) OVERRIDE;
+
+ private:
+  int block_on_;  // Bit mask on which states to block.
+  State state_;
+  int retval_;
+  // Should we call TestNetworkDelegate::OnAuthRequired in OnAuthRequired?
+  bool call_base_on_auth_required_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlockingNetworkDelegateBase);
+};
+
+BlockingNetworkDelegateBase::BlockingNetworkDelegateBase(
+    int block_on,
+    int retval,
+    bool call_base_on_auth_required)
+    : block_on_(block_on),
+      state_(NOT_BLOCKED),
+      retval_(retval),
+      call_base_on_auth_required_(call_base_on_auth_required) {
+}
+
+int BlockingNetworkDelegateBase::OnBeforeURLRequest(
+    URLRequest* request,
+    const CompletionCallback& callback,
+    GURL* new_url) {

[mmenke, 2012/09/13 15:28:02]

Suggest you inline these, for consistency with the rest of this file.  Also
think the functions are short enough, that the choice increases readability in
this case.

[vabr (Chromium), 2012/09/13 17:23:35]

I see the lack of consistency, but these are the reasons I have against
in-lining:

* Cascading in-lining in a class hierarchy accumulates long chunks of code which
are copied on each call place, leading to a code bloat. Scott Meyers makes this
point especially about constructors in Item 30 of Effective C++ (3rd edition),
but it applies to member functions as well.

* The style guide says in particular that "virtual functions should never be
declared this way" -- note that the On* methods are virtual. I'm actually
tempted to change this in the existing two classes. In general, the style guide
is against in-lining except for cases where it is proven as necessary. I guess
such a proof should involve a profiler, not just readability or consistency.

My last reason is subjective, but for readability, I actually think that moving
function definition out of class definition is a plus. The interface of the
class fits in a much less space, and is thus easier to read. People interested
in the definition only have to seek at the end of the class.

[mmenke, 2012/09/13 18:48:07]

Rightly or wrongly, in practice this rule tends to be largely ignored in classes
declared in C++ files.
I don't agree with this.  BlockingNetworkDelegateBase does not actually
implement the TestNetworkDelegate interface, and in fact interacts with its
child classes in a rather non-transparent manner.  Without reading both the base
class and the subclasses in their entirety, it's not clear what they actually
do.  As such, I found reading the BlockingNetworkDelegateBase declaration
leaving me with more questions than answers.  If you want to add documentation
to make this no longer the case, I'm fine with that.

+  TestNetworkDelegate::OnBeforeURLRequest(request, callback, new_url);
+  if ((block_on_ & ON_BEFORE_URL_REQUEST) != 0) {

[mmenke, 2012/09/13 15:28:02]

nit:  Suggest you put the "== 0" case first, for consistency.

[vabr (Chromium), 2012/09/13 17:23:35]

Done, also on the places below. Thanks!

+    state_ = ON_BEFORE_URL_REQUEST;
+    return retval_;
+  }
+  return OK;
+}
+
+int BlockingNetworkDelegateBase::OnBeforeSendHeaders(
+    URLRequest* request,
+    const CompletionCallback& callback,
+    HttpRequestHeaders* headers) {
+  TestNetworkDelegate::OnBeforeSendHeaders(request, callback, headers);
+  if ((block_on_ & ON_BEFORE_SEND_HEADERS) != 0) {
+    state_ = ON_BEFORE_SEND_HEADERS;
+    return retval_;
+  }
+  return OK;
+}
+
+int BlockingNetworkDelegateBase::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);
+  if ((block_on_ & ON_HEADERS_RECEIVED) != 0) {
+    state_ = ON_HEADERS_RECEIVED;
+    return retval_;
+  }
+  return OK;
+}
+
+NetworkDelegate::AuthRequiredResponse
+BlockingNetworkDelegateBase::OnAuthRequired(URLRequest* request,
+                                            const AuthChallengeInfo& auth_info,
+                                            const AuthCallback& callback,
+                                            AuthCredentials* credentials) {
+  if (call_base_on_auth_required_) {
+    TestNetworkDelegate::OnAuthRequired(request, auth_info, callback,
+                                        credentials);
+  }
+  if ((block_on_ & ON_AUTH_REQUIRED) != 0) {
+    state_ = ON_AUTH_REQUIRED;
+    // Here we just need to report something different than NO_ACTION.
+    return NetworkDelegate::AUTH_REQUIRED_RESPONSE_CANCEL_AUTH;
+  }
+  return NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
+}
+
 // A network delegate that blocks requests, optionally cancelling or redirecting
 // them.
-class BlockingNetworkDelegate : public TestNetworkDelegate {
+class BlockingNetworkDelegate : public BlockingNetworkDelegateBase {
  public:
   BlockingNetworkDelegate()
-      : retval_(ERR_IO_PENDING),
+      : BlockingNetworkDelegateBase(0, ERR_IO_PENDING, true),
         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_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; }
+    auth_retval_ = retval;
+  }
   void set_auth_callback_retval(NetworkDelegate::AuthRequiredResponse retval) {
-    auth_callback_retval_ = retval; }
+    auth_callback_retval_ = retval;
+  }
   void set_auth_credentials(const AuthCredentials& auth_credentials) {
     auth_credentials_ = auth_credentials;
   }
 
+  // TODO(vabr): remove when corresponding On* overrides are implemented.

[mmenke, 2012/09/13 15:28:02]

Aren't these already implemented by BlockingNetworkDelegateBase?

[vabr (Chromium), 2012/09/13 17:23:35]

Yes, they are, but they will not post the callback task, so the behaviour would
not be as expected.

[mmenke, 2012/09/13 18:48:07]

This sort of gets at one of my main complaints - the functions of the base class
implement the functionality needed by the interface functions of the same name. 
Callbacks are handled and recorded by the child classes based on the parent
class's return value of the function with the same name.

[mmenke, 2012/09/13 20:44:20]

Sorry, that was worded really poorly.  And I ended up including the issue in my
other comments, anyawys.  If you need me to clarify anything, just ask.

+  void BlockOn(State state) {
+    DCHECK(state != ON_BEFORE_SEND_HEADERS && state != ON_HEADERS_RECEIVED);
+    BlockingNetworkDelegateBase::BlockOn(state);
+  }
+
  private:
   // TestNetworkDelegate implementation.
   virtual int OnBeforeURLRequest(URLRequest* request,
@@ -179,13 +327,14 @@ class BlockingNetworkDelegate : public TestNetworkDelegate {
       return OK;
     }
 
-    TestNetworkDelegate::OnBeforeURLRequest(request, callback, new_url);
+    int retval = BlockingNetworkDelegateBase::OnBeforeURLRequest(
+        request, callback, new_url);
 
     if (!redirect_url_.is_empty())
       *new_url = redirect_url_;
 
-    if (retval_ != ERR_IO_PENDING)
-      return retval_;
+    if (retval != ERR_IO_PENDING)
+      return retval;
 
     MessageLoop::current()->PostTask(
         FROM_HERE,
@@ -199,8 +348,8 @@ class BlockingNetworkDelegate : public TestNetworkDelegate {
       const AuthChallengeInfo& auth_info,
       const AuthCallback& callback,
       AuthCredentials* credentials) OVERRIDE {
-    TestNetworkDelegate::OnAuthRequired(request, auth_info, callback,
-                                        credentials);
+    BlockingNetworkDelegateBase::OnAuthRequired(request, auth_info, callback,
+                                                credentials);
     switch (auth_retval_) {
       case NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION:
         break;
@@ -232,83 +381,59 @@ class BlockingNetworkDelegate : public TestNetworkDelegate {
   }
 
 
-  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_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlockingNetworkDelegate);
 };
 
 // A network delegate that allows blocking requests until a callback function is
 // called.
-class BlockingNetworkDelegateWithManualCallback : public TestNetworkDelegate {
+class BlockingNetworkDelegateWithManualCallback
+    : public BlockingNetworkDelegateBase {
  public:
-  enum State {
-    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) {
-  }
-
-  // Activates blocking on |state|.
-  void BlockOn(State state) {
-    block_on_ |= state;
+      : BlockingNetworkDelegateBase(0, ERR_IO_PENDING, false) {
   }
 
   void DoCallback(int rv) {
-    ASSERT_NE(NOT_BLOCKED, state_);
+    ASSERT_NE(BlockingNetworkDelegateBase::NOT_BLOCKED, state());
     CompletionCallback callback = callback_;
     Reset();
     callback.Run(rv);
   }
 
   void DoAuthCallback(NetworkDelegate::AuthRequiredResponse response) {
-    ASSERT_EQ(ON_AUTH_REQUIRED, state_);
+    ASSERT_EQ(BlockingNetworkDelegateBase::ON_AUTH_REQUIRED, state());
     AuthCallback auth_callback = auth_callback_;
     Reset();
     auth_callback.Run(response);
   }
 
-  // Runs the message loop until |state| is reached.
-  void WaitForState(State state) {
-    while (state_ != state)
-      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;
+    int retval = BlockingNetworkDelegateBase::OnBeforeURLRequest(
+        request, callback, new_url);
+    if (retval == ERR_IO_PENDING)
       callback_ = callback;
-      return ERR_IO_PENDING;
-    }
+    return retval;
   }
 
   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;
+    int retval = BlockingNetworkDelegateBase::OnBeforeSendHeaders(
+        request, callback, headers);
+    if (retval == ERR_IO_PENDING)
       callback_ = callback;
-      return ERR_IO_PENDING;
-    }
+    return retval;
   }
 
   virtual int OnHeadersReceived(
@@ -317,16 +442,12 @@ class BlockingNetworkDelegateWithManualCallback : public TestNetworkDelegate {
       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;
+    int retval = BlockingNetworkDelegateBase::OnHeadersReceived(
+        request, callback,
+        original_response_headers, override_response_headers);
+    if (retval == ERR_IO_PENDING)
       callback_ = callback;
-      return ERR_IO_PENDING;
-    }
+    return retval;
   }
 
   virtual NetworkDelegate::AuthRequiredResponse OnAuthRequired(
@@ -334,25 +455,26 @@ class BlockingNetworkDelegateWithManualCallback : public TestNetworkDelegate {
       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;
+    if (BlockingNetworkDelegateBase::OnAuthRequired(
+            request, auth_info, callback, credentials) !=
+        NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION) {
+      // We should block on authorisation requirement.
       auth_callback_ = callback;
       return NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING;
     }
+    return NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
   }
 
   void Reset() {
-    state_ = NOT_BLOCKED;
+    ResetState();
     callback_.Reset();
     auth_callback_.Reset();
   }
 
-  int block_on_;  // Bit mask on which states to block.
-  State state_;
   CompletionCallback callback_;
   AuthCallback auth_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlockingNetworkDelegateWithManualCallback);
 };
 
 
@@ -1950,6 +2072,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequest) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   network_delegate.set_callback_retval(ERR_EMPTY_RESPONSE);
 
   TestURLRequestContextWithProxy context(
@@ -1976,6 +2099,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelRequestSynchronously) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   network_delegate.set_retval(ERR_EMPTY_RESPONSE);
 
   TestURLRequestContextWithProxy context(
@@ -2003,6 +2127,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequest) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   GURL redirect_url(test_server_.GetURL("simple.html"));
   network_delegate.set_redirect_url(redirect_url);
 
@@ -2035,6 +2160,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequestSynchronously) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   GURL redirect_url(test_server_.GetURL("simple.html"));
   network_delegate.set_redirect_url(redirect_url);
   network_delegate.set_retval(OK);
@@ -2069,6 +2195,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateRedirectRequestPost) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   GURL redirect_url(test_server_.GetURL("echo"));
   network_delegate.set_redirect_url(redirect_url);
 
@@ -2110,6 +2237,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredSyncNoAction) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   network_delegate.set_auth_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION);
 
@@ -2142,6 +2270,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredSyncSetAuth) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   network_delegate.set_auth_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_SET_AUTH);
 
@@ -2174,6 +2303,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredSyncCancel) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   network_delegate.set_auth_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_CANCEL_AUTH);
 
@@ -2206,6 +2336,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredAsyncNoAction) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   network_delegate.set_auth_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING);
   network_delegate.set_auth_callback_retval(
@@ -2240,6 +2371,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredAsyncSetAuth) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   network_delegate.set_auth_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING);
   network_delegate.set_auth_callback_retval(
@@ -2276,6 +2408,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateOnAuthRequiredAsyncCancel) {
 
   TestDelegate d;
   BlockingNetworkDelegate network_delegate;
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
   network_delegate.set_auth_retval(
       NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING);
   network_delegate.set_auth_callback_retval(
@@ -2309,8 +2442,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting1) {
 
   TestDelegate d;
   BlockingNetworkDelegateWithManualCallback network_delegate;
-  network_delegate.BlockOn(
-      BlockingNetworkDelegateWithManualCallback::ON_BEFORE_URL_REQUEST);
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
@@ -2321,7 +2453,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting1) {
 
     r.Start();
     network_delegate.WaitForState(
-        BlockingNetworkDelegateWithManualCallback::ON_BEFORE_URL_REQUEST);
+        BlockingNetworkDelegateBase::ON_BEFORE_URL_REQUEST);
     EXPECT_EQ(0, network_delegate.completed_requests());
     // Cancel before callback.
     r.Cancel();
@@ -2343,8 +2475,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting2) {
 
   TestDelegate d;
   BlockingNetworkDelegateWithManualCallback network_delegate;
-  network_delegate.BlockOn(
-      BlockingNetworkDelegateWithManualCallback::ON_BEFORE_SEND_HEADERS);
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_BEFORE_SEND_HEADERS);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
@@ -2355,7 +2486,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting2) {
 
     r.Start();
     network_delegate.WaitForState(
-        BlockingNetworkDelegateWithManualCallback::ON_BEFORE_SEND_HEADERS);
+        BlockingNetworkDelegateBase::ON_BEFORE_SEND_HEADERS);
     EXPECT_EQ(0, network_delegate.completed_requests());
     // Cancel before callback.
     r.Cancel();
@@ -2377,8 +2508,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting3) {
 
   TestDelegate d;
   BlockingNetworkDelegateWithManualCallback network_delegate;
-  network_delegate.BlockOn(
-      BlockingNetworkDelegateWithManualCallback::ON_HEADERS_RECEIVED);
+  network_delegate.BlockOn(BlockingNetworkDelegateBase::ON_HEADERS_RECEIVED);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
@@ -2389,7 +2519,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting3) {
 
     r.Start();
     network_delegate.WaitForState(
-        BlockingNetworkDelegateWithManualCallback::ON_HEADERS_RECEIVED);
+        BlockingNetworkDelegateBase::ON_HEADERS_RECEIVED);
     EXPECT_EQ(0, network_delegate.completed_requests());
     // Cancel before callback.
     r.Cancel();
@@ -2412,7 +2542,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting4) {
   TestDelegate d;
   BlockingNetworkDelegateWithManualCallback network_delegate;
   network_delegate.BlockOn(
-      BlockingNetworkDelegateWithManualCallback::ON_AUTH_REQUIRED);
+      BlockingNetworkDelegateBase::ON_AUTH_REQUIRED);
 
   TestURLRequestContext context(true);
   context.set_network_delegate(&network_delegate);
@@ -2423,7 +2553,7 @@ TEST_F(URLRequestTestHTTP, NetworkDelegateCancelWhileWaiting4) {
 
     r.Start();
     network_delegate.WaitForState(
-        BlockingNetworkDelegateWithManualCallback::ON_AUTH_REQUIRED);
+        BlockingNetworkDelegateBase::ON_AUTH_REQUIRED);
     EXPECT_EQ(0, network_delegate.completed_requests());
     // Cancel before callback.
     r.Cancel();