Implemented curl support for web requests

include/AdblockPlus/WebRequest.h

 #ifndef ADBLOCKPLUS_WEB_REQUEST_H
 #define ADBLOCKPLUS_WEB_REQUEST_H
 
 #include <string>
 #include <vector>
-
-enum

[Felix Dahlke, 2013/04/12 03:53:50]

I think this should be in the AdblockPlus namespace, maybe even
AdblockPlus::ServerResponse. We shouldn't put much in the global namespace.

-{
-  NS_OK = 0,
-  NS_ERROR_FAILURE = 0x80004005,
-  NS_ERROR_OUT_OF_MEMORY = 0x8007000e,
-  NS_ERROR_MALFORMED_URI = 0x804b000a,
-  NS_ERROR_CONNECTION_REFUSED = 0x804b000d,
-  NS_ERROR_NET_TIMEOUT = 0x804b000e,
-  NS_ERROR_NO_CONTENT = 0x804b0011,
-  NS_ERROR_UNKNOWN_PROTOCOL = 0x804b0012,
-  NS_ERROR_NET_RESET = 0x804b0014,
-  NS_ERROR_UNKNOWN_HOST = 0x804b001e,
-  NS_ERROR_REDIRECT_LOOP = 0x804b001f,
-  NS_ERROR_UNKNOWN_PROXY_HOST = 0x804b002a,
-  NS_ERROR_NOT_INITIALIZED = 0xc1f30001,
-  NS_CUSTOM_ERROR_BASE = 0x80850000
-};
 
 namespace AdblockPlus
 {
   typedef std::vector<std::pair<std::string, std::string> > HeaderList;
 
   struct ServerResponse
   {
     unsigned int status;
     HeaderList responseHeaders;
     int responseStatus;
     std::string responseText;
   };
 
   class WebRequest
   {
   public:
-    virtual ~WebRequest();
+    enum
+    {
+      NS_OK = 0,
+      NS_ERROR_FAILURE = 0x80004005,
+      NS_ERROR_OUT_OF_MEMORY = 0x8007000e,
+      NS_ERROR_MALFORMED_URI = 0x804b000a,
+      NS_ERROR_CONNECTION_REFUSED = 0x804b000d,
+      NS_ERROR_NET_TIMEOUT = 0x804b000e,
+      NS_ERROR_NO_CONTENT = 0x804b0011,
+      NS_ERROR_UNKNOWN_PROTOCOL = 0x804b0012,
+      NS_ERROR_NET_RESET = 0x804b0014,
+      NS_ERROR_UNKNOWN_HOST = 0x804b001e,
+      NS_ERROR_REDIRECT_LOOP = 0x804b001f,
+      NS_ERROR_UNKNOWN_PROXY_HOST = 0x804b002a,
+      NS_ERROR_NOT_INITIALIZED = 0xc1f30001,
+      NS_CUSTOM_ERROR_BASE = 0x80850000
+    };
+
+    virtual inline ~WebRequest() {};

[Felix Dahlke, 2013/04/16 12:21:09]

Oh, just noticed, inline is actually not necessary for functions declared inside
a class. I think I said it was at some point, so sorry about that :P

[Wladimir Palant, 2013/04/16 13:30:32]

Let's remove it everywhere then later, not only in this patch.

[Felix Dahlke, 2013/04/16 14:51:37]

Okay.

     virtual ServerResponse GET(const std::string& url, const HeaderList& requestHeaders) const = 0;
   };
 
   class DefaultWebRequest : public WebRequest
   {

[Oleksandr, 2013/04/16 12:02:01]

I don't quite understand the logic behind this naming here. Wouldn't it make
more sense to have each implementation to have a different name and have clients
just using the interface (ie WebRequest class) to operate. Is there going to be
any other implementation becides "default"? 

[Felix Dahlke, 2013/04/16 12:21:09]

Yeah, most likely. We cannot really use curl on Android, nor do I think we can
do it in IE. So it's only used by abpshell (and the tests) at this point, and
only when present.

[Oleksandr, 2013/04/16 12:26:18]

Let me rephrase my question: why do all implementations have to have a name
"DefaultWebRequest"? 

Why not WebRequestCurl, WebRequestStub, WebRequestWinInet etc. Each of which
implements WebRequest.

On 2013/04/16 12:21:09, Felix H. Dahlke wrote:

[Wladimir Palant, 2013/04/16 13:30:32]

The thinking behind this:

1) We have clear preferences for the WebRequest implementation: WinHttp on
Win32, curl on Linux and OS X, Java URL on Android.

2) We don't want to compile more than one WebRequest implementation per platform
even if we could. Meaning for example: on Windows we are always going to use
WinHttp and ignore curl completely.

3) If we use different class names for all implementations then we move the
selection logic into the consumers: these will need to determine which
implementation they want to use. We will have to duplicate the selection rules
in each consumer. Alternatively we can use a factory method but that also
duplicates the logic we will need in the build config anyway.

My thought was to make it easy for WebRequest consumers while keeping things
flexible: they can just use DefaultWebRequest and forget about it or they can
use a custom implementation (e.g. for tests). The rules determining which
WebRequest implementation gets compiled and consequently goes by the name
DefaultWebRequest are centralized in the build configuration.

     ServerResponse GET(const std::string& url, const HeaderList& requestHeaders) const;
   };

[Oleksandr, 2013/04/11 16:47:18]

Wouldn't it be better to have this class in a separate file? Especially seeing
there will be another class derived from WebRequest for Windows.

[Felix Dahlke, 2013/04/12 03:53:50]

Agreed.

 }
 
 #endif