Files.app: Implement md5 hashing over file streams.

chrome/browser/drive/drive_api_util.h

 // Copyright (c) 2012 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_DRIVE_DRIVE_API_UTIL_H_
 #define CHROME_BROWSER_DRIVE_DRIVE_API_UTIL_H_
 
 #include <string>
 
+#include "base/md5.h"
 #include "base/memory/scoped_ptr.h"
 #include "google_apis/drive/drive_api_error_codes.h"
 #include "google_apis/drive/drive_common_callbacks.h"
 
 class GURL;
 
 namespace base {
 class FilePath;
 class Value;
 }  // namespace base
 
 namespace google_apis {
 class ChangeList;
 class ChangeResource;
 class FileList;
 class FileResource;
 class ResourceEntry;
 }  // namespace google_apis
 
+namespace net {
+class IOBuffer;
+}  // namespace net
+
+namespace storage {
+class FileStreamReader;
+}  // namespace storage
+
 namespace drive {
 namespace util {
 
 // Google Apps MIME types:
 const char kGoogleDocumentMimeType[] = "application/vnd.google-apps.document";
 const char kGoogleDrawingMimeType[] = "application/vnd.google-apps.drawing";
 const char kGooglePresentationMimeType[] =
     "application/vnd.google-apps.presentation";
 const char kGoogleSpreadsheetMimeType[] =
     "application/vnd.google-apps.spreadsheet";
@@ skipping 18 matching lines @@
 std::string TranslateQuery(const std::string& original_query);
 
 // If |resource_id| is in the old resource ID format used by WAPI, converts it
 // into the new format.
 std::string CanonicalizeResourceId(const std::string& resource_id);
 
 // Returns the (base-16 encoded) MD5 digest of the file content at |file_path|,
 // or an empty string if an error is found.
 std::string GetMd5Digest(const base::FilePath& file_path);
 
+// Computes the (base-16 encoded) MD5 digest of data extracted from a file
+// stream.
+class FileStreamMd5Digester {
+ public:
+  FileStreamMd5Digester();
+  ~FileStreamMd5Digester();
+  // Computes an MD5 digest of data read from the given |streamReader|.  The

[mtomasz, 2015/01/31 04:53:12]

nit: \n\n after destructor?

[Ben Kwa, 2015/02/02 22:22:11]

Done.

+  // work occurs asynchronously, and the resulting hash is returned via the
+  // |callback|.  If an error occurs, |callback| is called with an empty string.
+  // Only one stream can be processed at a time by each digester.  Do not call
+  // GetMd5Digest before the results of a previous call have been returned.
+  void GetMd5Digest(scoped_ptr<storage::FileStreamReader> streamReader,

[mtomasz, 2015/01/31 04:53:12]

nit: stream_reader

[Ben Kwa, 2015/02/02 22:22:11]

Done.

+                    const base::Callback<void(const std::string&)>& callback);

[mtomasz, 2015/01/31 04:53:12]

nit: How about typedefing the callback within FileStreamMd5Digester?

[Ben Kwa, 2015/02/02 22:22:11]

Done.

+
+ private:
+  // Kicks off a read of the next chunk from the stream.
+  void readNextChunk();

[mtomasz, 2015/01/31 04:53:12]

nit: read -> Read

[Ben Kwa, 2015/02/02 22:22:12]

Done.

+  // Handles the incoming chunk of data from a stream read.
+  void OnChunkRead(int result);
+
+ private:
+  // Maximum chunk size for read operations.
+  static const int kBufferSize_ = 512 * 1024;  // 512 kB.

[mtomasz, 2015/01/31 04:53:12]

nit: Also, do we need it to be in the header file? How about moving to foobar.cc
anonymous namespace?

[mtomasz, 2015/01/31 04:53:12]

nit: Isn't the chunk too large? We're doing it on UI thread, which we don't want
to block.

[mtomasz, 2015/01/31 04:53:12]

nit: This may be new if initializing static members in a header is allowed.
Please double check.

[Ben Kwa, 2015/02/02 22:22:11]

So... this happens on the IO thread now.  I asked around about it, the
(incremental) cost of the hashing is small compared to the cost of the file read
and so it doesn't really matter which thread the hashing occurs on.  Better to
keep it on the IO thread to reduce the amount of bouncing back and forth.

[Ben Kwa, 2015/02/02 22:22:12]

I figured that keeping this constant scoped within the class would be a good
thing.  I could move it to the .cc file, but then it'd just be a random constant
in drive_api_util.cc, which I thought was more untidy.

[Ben Kwa, 2015/02/02 22:22:12]

The style guides don't explicitly say anything about this, but based on code
search, it's pretty common.  

https://code.google.com/p/chromium/codesearch#search/&q=file:.*%5C.h%20%22sta...

+  scoped_ptr<storage::FileStreamReader> reader_;
+  base::Callback<void(const std::string&)> callback_;
+  scoped_refptr<net::IOBuffer> buffer_;
+  base::MD5Context md5Context_;

[mtomasz, 2015/01/31 04:53:12]

nit: camelCase -> is_not_allowed

[Ben Kwa, 2015/02/02 22:22:12]

Done.

+
+  base::WeakPtrFactory<FileStreamMd5Digester> weak_ptr_factory_;
+  DISALLOW_COPY_AND_ASSIGN(FileStreamMd5Digester);
+};
+
 // Returns preferred file extension for hosted documents which have given mime
 // type.
 std::string GetHostedDocumentExtension(const std::string& mime_type);
 
 // Returns true if the given mime type is corresponding to one of known hosted
 // document types.
 bool IsKnownHostedDocumentMimeType(const std::string& mime_type);
 
 // Returns true if the given file path has an extension corresponding to one of
 // hosted document types.
 bool HasHostedDocumentExtension(const base::FilePath& path);
 
 }  // namespace util
 }  // namespace drive
 
 #endif  // CHROME_BROWSER_DRIVE_DRIVE_API_UTIL_H_