Turned job_factory into a pure virtual class

net/url_request/url_request_job_factory.h

Index: net/url_request/url_request_job_factory.h
diff --git a/net/url_request/url_request_job_factory.h b/net/url_request/url_request_job_factory.h
index 746e9486894546b36f8d9fb20a5ffdaec8109bb7..49c8ef3579f34149fdc63721e12692bd596a0a41 100644
--- a/net/url_request/url_request_job_factory.h
+++ b/net/url_request/url_request_job_factory.h
@@ -5,10 +5,9 @@
 #ifndef NET_URL_REQUEST_URL_REQUEST_JOB_FACTORY_H_
 #define NET_URL_REQUEST_URL_REQUEST_JOB_FACTORY_H_
 
-#include <map>
 #include <string>
-#include <vector>
 #include "base/basictypes.h"
+#include "base/compiler_specific.h"
 #include "base/threading/non_thread_safe.h"
 #include "net/base/net_export.h"
 
@@ -66,39 +65,36 @@ class NET_EXPORT URLRequestJobFactory
   };
 
   URLRequestJobFactory();
-  ~URLRequestJobFactory();
+  virtual ~URLRequestJobFactory();
 
+  // TODO(shalev): Remove this from the interface.
   // Sets the ProtocolHandler for a scheme. Returns true on success, false on
   // failure (a ProtocolHandler already exists for |scheme|). On success,
   // URLRequestJobFactory takes ownership of |protocol_handler|.
-  bool SetProtocolHandler(const std::string& scheme,
-                          ProtocolHandler* protocol_handler);
+  virtual bool SetProtocolHandler(const std::string& scheme,
+                                  ProtocolHandler* protocol_handler) = 0;
 
+  // TODO(shalev): Remove this from the interface.
   // Takes ownership of |interceptor|. Adds it to the end of the Interceptor
   // list.
-  void AddInterceptor(Interceptor* interceptor);
+  virtual void AddInterceptor(Interceptor* interceptor) = 0;
 
-  URLRequestJob* MaybeCreateJobWithInterceptor(URLRequest* request) const;
+  virtual URLRequestJob* MaybeCreateJobWithInterceptor(

[willchan no longer on Chromium, 2012/08/16 20:35:37]

Shouldn't there be a TODO here to kill this too?

[erikwright (departed), 2012/08/21 17:43:32]

The two TODOs added are in reference to the mutators. It doesn't make sense for
the factory API to dictate how a specific implementation of the API is
configured. The only universal function of a factory is to create Jobs. Shalev:
please update the TODOs to specifically say "Remove mutators from the
interface.".

Will: If I understand your comments correctly, you are suggesting TODOs for:

1) killing the interceptor related behaviour. This makes sense because an
interceptor is just a special form of a protocol handler and there is no need
for a distinction as to which is implementing a given Job. Shalev: please add a
TODO to this effect.

2) killing the ProtocolHandler related behaviour. I don't know if you are
implying just changing the name, or whether you are implying changing this API
to be the CreateRequest API (returning a URLRequest instead of a Job) that we
have discussed.

For #1, I agree.

For #2, if it's changing the name, I agree. If it's about changing to create
URLRequest instead of Job I disagree, because I'd rather wait until we're a bit
closer to that final step before littering the code with detailed intentions.

[willchan no longer on Chromium, 2012/08/21 17:58:01]

Not sure if I understand your comments. What I was thinking was the
URLRequestJobFactory interface should not know anything about Interceptors or
ProtocolHandlers. It just maybe creates jobs at specific points.

URLRequestJobFactoryImpl on the other hand can segregate stuff into
ProtocolHandlers and Interceptors. But that's specific to
URLRequestJobFactoryImpl, and not something that needs to be part of the base
interface.

On 2012/08/21 17:43:32, erikwright wrote:

[erikwright (departed), 2012/08/21 18:02:13]

Great. We are in agreement.

A single TODO should suffice:

TODO(shalev): Consolidate MaybeCreateJobWithInterceptor and
MaybeCreateJobWithProtocolHandler into a single method.

[shalev, 2012/08/22 19:10:11]

Done.

+      URLRequest* request) const = 0;
 
-  URLRequestJob* MaybeCreateJobWithProtocolHandler(const std::string& scheme,
-                                                   URLRequest* request) const;
+  virtual URLRequestJob* MaybeCreateJobWithProtocolHandler(

[willchan no longer on Chromium, 2012/08/16 20:35:37]

Ditto

+      const std::string& scheme, URLRequest* request) const = 0;
 
-  URLRequestJob* MaybeInterceptRedirect(const GURL& location,
-                                        URLRequest* request) const;
+  virtual URLRequestJob* MaybeInterceptRedirect(
+      const GURL& location, URLRequest* request) const = 0;
 
-  URLRequestJob* MaybeInterceptResponse(URLRequest* request) const;
+  virtual URLRequestJob* MaybeInterceptResponse(URLRequest* request) const = 0;
 
-  bool IsHandledProtocol(const std::string& scheme) const;
+  virtual bool IsHandledProtocol(const std::string& scheme) const = 0;
 
-  bool IsHandledURL(const GURL& url) const;
+  virtual bool IsHandledURL(const GURL& url) const = 0;
 
  private:
-  typedef std::map<std::string, ProtocolHandler*> ProtocolHandlerMap;
-  typedef std::vector<Interceptor*> InterceptorList;
-
-  ProtocolHandlerMap protocol_handler_map_;
-  InterceptorList interceptors_;
-
   DISALLOW_COPY_AND_ASSIGN(URLRequestJobFactory);
 };