Documentation, polishing and fixes

 - Add/fix basic documentation for Downloader and DownloaderContext classes
 - Move DownloaderContext class definition to Downloader.cc file
 - fix cbdata leaks inside DownloaderCotnext destructor, caused by simple typo
   mistake.
 - Do not cbdatalock the ClientHttpRequest object inside DownloaderContext.
   This class is responsible to hold the main pointer and finaly delete the
   ClientHttpRequest object.

diff --git a/src/Downloader.cc b/src/Downloader.cc
index 045d1288bf577b119564f4f51675a9100f30a352..901b2559a874bd2545b92966a0e6fe1db848077f 100644 (file)
--- a/src/Downloader.cc
+++ b/src/Downloader.cc
@@ -7,13 +7,33 @@
 #include "http/one/RequestParser.h"
 #include "http/Stream.h"
 
+/// Used to hold and pass the required info and buffers to the
+/// clientStream callbacks
+class DownloaderContext: public RefCountable
+{
+    CBDATA_CLASS(DownloaderContext);
+
+public:
+    typedef RefCount<DownloaderContext> Pointer;
+
+    DownloaderContext(Downloader *dl, ClientHttpRequest *h):
+        downloader(cbdataReference(dl)),
+        http(h)
+        {}
+    ~DownloaderContext();
+    void finished();
+    Downloader* downloader;
+    ClientHttpRequest *http;
+    char requestBuffer[HTTP_REQBUF_SZ];
+};
+
 CBDATA_CLASS_INIT(DownloaderContext);
 CBDATA_CLASS_INIT(Downloader);
 
 DownloaderContext::~DownloaderContext()
 {
     debugs(33, 5, HERE);
-    cbdataReference(downloader);
+    cbdataReferenceDone(downloader);
     if (http)
         finished();
 }
@@ -21,7 +41,6 @@ DownloaderContext::~DownloaderContext()
 void
 DownloaderContext::finished()
 {
-    cbdataReference(http);
     delete http;
     http = NULL;
 }

diff --git a/src/Downloader.h b/src/Downloader.h
index b11a112a83a8572bfd8f2b82f2d0440185666669..4d6546252dd6ca34a4a736ef9463e8ce6b42f15f 100644 (file)
--- a/src/Downloader.h
+++ b/src/Downloader.h
@@ -12,26 +12,13 @@ class ClientHttpRequest;
 class StoreIOBuffer;
 class clientStreamNode;
 class HttpReply;
-class Downloader;
-
-class DownloaderContext: public RefCountable
-{
-    CBDATA_CLASS(DownloaderContext);
-
-public:
-    typedef RefCount<DownloaderContext> Pointer;
-
-    DownloaderContext(Downloader *dl, ClientHttpRequest *h):
-        downloader(cbdataReference(dl)),
-        http(cbdataReference(h))
-        {}
-    ~DownloaderContext();
-    void finished();
-    Downloader* downloader;
-    ClientHttpRequest *http;
-    char requestBuffer[HTTP_REQBUF_SZ];
-};
+class DownloaderContext;
+typedef RefCount<DownloaderContext> DownloaderContextPointer;
 
+/// The Downloader class fetches SBuf-storable things for other Squid
+/// components/transactions using internal requests. For example, it is used
+/// to fetch missing intermediate certificates when validating origin server
+/// certificate chains.
 class Downloader: virtual public AsyncJob
 {
     CBDATA_CLASS(Downloader);
@@ -58,16 +45,15 @@ public:
     /* AsyncJob API */
     virtual bool doneAll() const;
 
-    DownloaderContext::Pointer const &context() {return context_;};
     void handleReply(clientStreamNode * node, ClientHttpRequest *http, HttpReply *header, StoreIOBuffer receivedData);
 protected:
 
     /* AsyncJob API */
     virtual void start();
-    virtual void prepUserConnection() {};
 
 private:
 
+    /// Initializes and starts the HTTP GET request to the remote server
     bool buildRequest();
 
     /// Schedules for execution the "callback" with parameters the status
@@ -81,9 +67,10 @@ private:
     AsyncCall::Pointer callback; ///< callback to call when download finishes
     Http::StatusCode status; ///< the download status code
     SBuf object; ///< the object body data
-    unsigned int level_; ///< holds the nested downloads level
+    const unsigned int level_; ///< holds the nested downloads level
 
-    DownloaderContext::Pointer context_;
+    /// Pointer to an object that stores the clientStream required info
+    DownloaderContextPointer context_;
 };
 
 #endif