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

chrome/browser/download/base_file.h

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_DOWNLOAD_BASE_FILE_H_
 #define CHROME_BROWSER_DOWNLOAD_BASE_FILE_H_
 #pragma once
 
 #include <string>
 
 #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"
 
 namespace net {
 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 {
  public:
   BaseFile(const FilePath& full_path,
            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();
 
   // Write a new chunk of data to the file. Returns true on success (all bytes
   // written to the file).
   bool AppendDataToFile(const char* data, size_t data_len);
 
   // Rename the download file. Returns true on success.
   // |path_renamed_| is set to true only if |is_final_rename| is true.
   // Marked virtual for testing.
   virtual bool Rename(const FilePath& full_path, bool is_final_rename);
 
   // Abort the download and automatically close the file.
   void Cancel();
 
   // Indicate that the download has finished. No new data will be received.
   void Finish();
 
   // Informs the OS that this file came from the internet.
   void AnnotateWithSourceInformation();
 
   FilePath full_path() const { return full_path_; }
   bool path_renamed() const { return path_renamed_; }
   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:
   bool Open();
   void Close();
 
   // Full path to the file including the file name.
   FilePath full_path_;
 
   // Whether the download is still using its initial temporary path.
   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_;
 
   // The URL where the download was initiated.
   GURL referrer_url_;
 
   // OS file stream for writing
   linked_ptr<net::FileStream> file_stream_;
 
   // Amount of data received up so far, in bytes.
   int64 bytes_so_far_;
 
   // 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);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_BASE_FILE_H_