Add support to sha256 hash the downloaded file....

chrome/browser/download/base_file.h

Index: chrome/browser/download/base_file.h
===================================================================
--- chrome/browser/download/base_file.h	(revision 70630)
+++ chrome/browser/download/base_file.h	(working copy)
@@ -10,6 +10,7 @@
 
 #include "base/file_path.h"
 #include "base/linked_ptr.h"
+#include "base/scoped_ptr.h"
 #include "chrome/browser/power_save_blocker.h"
 #include "googleurl/src/gurl.h"
 
@@ -17,6 +18,9 @@
 class FileStream;
 }
 
+struct SHA256ContextStr;
+typedef struct SHA256ContextStr SHA256Context;

[Paweł Hajdan Jr., 2011/01/11 08:28:21]

nit: Why do we need this typedef? It seems to add more confusion.

For example, something might #include base_file.h very indirectly, and rely on
SHA256Context typedef. Then, after some unrelated header reshuffling,
SHA256Context may no longer be visible.

Anyway, it's just weird to have two names for the same thing.

[lzheng, 2011/01/12 02:11:54]

I agree it looks confusing. Since the original SHA256Context is typedef'ed, to
avoid using the blapi.h header, that was the only way. 
I added the blapi.h header instead.

On 2011/01/11 08:28:21, Paweł Hajdan Jr. wrote:

+
 // File being downloaded and saved to disk. This is a base class
 // for DownloadFile and SaveFile, which keep more state information.
 class BaseFile {
@@ -25,7 +29,8 @@
            const GURL& source_url,
            const GURL& referrer_url,
            int64 received_bytes,
-           const linked_ptr<net::FileStream>& file_stream);
+           const linked_ptr<net::FileStream>& file_stream,
+           bool calculate_hash);

[Randy Smith (Not in Mondays), 2011/01/11 18:29:51]

A tacky question, but I at least want to ask: What is the cost of always
calculating the hash?  The interface would be simpler and more constrained if
the file hash was just always available after the download had finished; you
wouldn't need to do all the messing around with constructors and creation
routines.

[lzheng, 2011/01/12 02:11:54]

Answered at the beginning of this email.

On 2011/01/11 18:29:51, rdsmith wrote:

   ~BaseFile();
 
   bool Initialize();
@@ -53,6 +58,10 @@
   bool in_progress() const { return file_stream_ != NULL; }
   int64 bytes_so_far() const { return bytes_so_far_; }
 
+  // Set |hash| with sha256 digest for the file. The |hash| would be a
+  // lower-case hex string. Returns false if the digest is not calcuated.

[Paweł Hajdan Jr., 2011/01/11 08:28:21]

nit: How about "Returns true on success" to avoid double negation?

[Randy Smith (Not in Mondays), 2011/01/11 18:29:51]

At what point will the digest be calculated?  Is it tied to the completion of
the download?

[lzheng, 2011/01/12 02:11:54]

Done.

[lzheng, 2011/01/12 02:11:54]

It should be. I added verification for the in_progress.
On 2011/01/11 18:29:51, rdsmith wrote:

+  virtual bool GetSha256Hash(std::string* hash);
+
   virtual std::string DebugString() const;
 
  protected:
@@ -66,6 +75,8 @@
   bool path_renamed_;
 
  private:
+  static const size_t kSha256HashLen_ = 32;

[Paweł Hajdan Jr., 2011/01/11 08:28:21]

nit: The underscore at the end looks weird, I think we don't use those for
constants.

[lzheng, 2011/01/12 02:11:54]

Done.

+
   // Source URL for the file being downloaded.
   GURL source_url_;
 
@@ -81,6 +92,13 @@
   // RAII handle to keep the system from sleeping while we're downloading.
   PowerSaveBlocker power_save_blocker_;
 
+  // Indicates if sha256 hash should be calculated for the downloaded file.
+  bool calculate_hash_;
+
+  scoped_ptr<SHA256Context> sha_context_;

[Paweł Hajdan Jr., 2011/01/11 08:28:21]

nit: Could you add a short comment?

[lzheng, 2011/01/12 02:11:54]

Done.

+
+  unsigned char sha256_hash_[kSha256HashLen_];
+
   DISALLOW_COPY_AND_ASSIGN(BaseFile);
 };
 
Property changes on: chrome/browser/download/base_file.h
___________________________________________________________________
Added: svn:eol-style
   + LF