Change Interceptors into URLRequestJobFactory::ProtocolHandlers.

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 38eee92062f1c158f08569f755481fabd6d34486..f15282d4ccca59f038e0a1b0adcb5c54c55b7198 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -54,6 +54,7 @@
 #include "net/socket/ssl_client_socket.h"
 #include "net/test/test_server.h"
 #include "net/url_request/ftp_protocol_handler.h"
+#include "net/url_request/protocol_intercept_job_factory.h"
 #include "net/url_request/static_http_user_agent_settings.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_file_dir_job.h"
@@ -464,18 +465,22 @@ class URLRequestTest : public PlatformTest {
     default_context_.set_network_delegate(&default_network_delegate_);
     default_context_.Init();
   }
+  virtual ~URLRequestTest() {}
 
   // Adds the TestJobInterceptor to the default context.
   TestJobInterceptor* AddTestInterceptor() {
-    TestJobInterceptor* interceptor = new TestJobInterceptor();
-    default_context_.set_job_factory(&job_factory_);
-    job_factory_.AddInterceptor(interceptor);
-    return interceptor;
+    TestJobInterceptor* protocol_handler_ = new TestJobInterceptor();
+    job_factory_.reset(new URLRequestJobFactoryImpl);

[erikwright (departed), 2012/12/06 15:26:28]

Is there a reason why this should be a ProtocolInterceptJobFactory wrapping an
empty URLRequestJobFactoryImpl if no-one ever accesses the mutators?

Couldn't it just be an URLRequestJobFactoryImpl with a single ProtocolHandler
then?

Or couldn't we do one of:

1) make the mutators in ProtocolInterceptJobFactory not forward to the wrapped
factory, or
2) create a new URLRequestJobFactory implementation that only holds a single
ProtocolHandler and NOTREACHED()'s all mutators?

[pauljensen, 2012/12/07 18:47:42]

TestJobInterceptor only intercepts the first HTTP request, any subsequent HTTP
requests or non-HTTP requests will be handled by the default static
URLRequest::ProtocolFactory's in URLRequestJobManager.  I was trying to maintain
this exact behavior.  After looking at users of TestJobInterceptor, it looks
like the tests don't actually need to rely on the default static
URLRequest::ProtocolFactory's, so I guess the answer is I could just call
URLRequestJobFactoryImpl::SetProtocolHandler().
Why the concern about mutators?  I thought these were going away imminently.

[erikwright (departed), 2012/12/07 20:12:05]

OK, I see.
It's just that I noticed that you had converted the mutators in the
ProtocolInterceptJobFactory from NOTREACHED to forwarding, which leads me to
wonder who's relying on them (as they will presumably present a problem when we
do attempt to remove them).

If it doesn't break anyone, I'd leave the NOTREACHED, but ultimately I'm also OK
to worry about that separately.

[pauljensen, 2012/12/09 23:14:12]

For now they must pass things like SetProtocolHandler() onto |job_factory_|
until crrev.com/11308362 lands.

+    job_factory_.reset(new ProtocolInterceptJobFactory(job_factory_.Pass(),
+                                                       "http",
+                                                       protocol_handler_));
+    default_context_.set_job_factory(job_factory_.get());
+    return protocol_handler_;
   }
 
  protected:
   TestNetworkDelegate default_network_delegate_;  // Must outlive URLRequest.
-  URLRequestJobFactoryImpl job_factory_;
+  scoped_ptr<URLRequestJobFactory> job_factory_;
   TestURLRequestContext default_context_;
 };