Store attachment crc in AttachmentStore

sync/internal_api/attachments/attachment_downloader_impl.cc

Index: sync/internal_api/attachments/attachment_downloader_impl.cc
diff --git a/sync/internal_api/attachments/attachment_downloader_impl.cc b/sync/internal_api/attachments/attachment_downloader_impl.cc
index 7b732186da665e3c520925f05ae9cee4d496804f..5e1750cc10146dd710ce64fc6edaded00e04052a 100644
--- a/sync/internal_api/attachments/attachment_downloader_impl.cc
+++ b/sync/internal_api/attachments/attachment_downloader_impl.cc
@@ -4,14 +4,17 @@
 
 #include "sync/internal_api/public/attachments/attachment_downloader_impl.h"
 
+#include "base/base64.h"
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
+#include "base/sys_byteorder.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/http/http_util.h"
 #include "net/url_request/url_fetcher.h"
 #include "sync/internal_api/public/attachments/attachment_uploader_impl.h"
+#include "sync/internal_api/public/attachments/attachment_util.h"
 #include "sync/protocol/sync.pb.h"
 #include "url/gurl.h"
 
@@ -116,8 +119,8 @@ void AttachmentDownloaderImpl::OnGetTokenFailure(
        ++iter) {
     DownloadState* download_state = *iter;
     scoped_refptr<base::RefCountedString> null_attachment_data;
-    ReportResult(
-        *download_state, DOWNLOAD_TRANSIENT_ERROR, null_attachment_data);
+    ReportResult(*download_state, DOWNLOAD_TRANSIENT_ERROR,
+                 null_attachment_data, 0);
     DCHECK(state_map_.find(download_state->attachment_url) != state_map_.end());
     state_map_.erase(download_state->attachment_url);
   }
@@ -137,17 +140,24 @@ void AttachmentDownloaderImpl::OnURLFetchComplete(
 
   DownloadResult result = DOWNLOAD_TRANSIENT_ERROR;
   scoped_refptr<base::RefCountedString> attachment_data;
+  uint32_t attachment_crc = 0;
 
   const int response_code = source->GetResponseCode();
   if (response_code == net::HTTP_OK) {
     std::string data_as_string;
     source->GetResponseAsString(&data_as_string);
-    if (VerifyHashIfPresent(*source, data_as_string)) {
-      result = DOWNLOAD_SUCCESS;
-      attachment_data = base::RefCountedString::TakeString(&data_as_string);
-    } else {
-      // TODO(maniscalco): Test me!
+    attachment_data = base::RefCountedString::TakeString(&data_as_string);
+
+    attachment_crc = ComputeCrc32c(attachment_data);
+    uint32_t crc32c_from_headers = 0;
+    if (ExtractCrc32c(source->GetResponseHeaders(), &crc32c_from_headers) &&
+        attachment_crc != crc32c_from_headers) {
+      // Fail download only if there is useful crc in header and it doesn't

[maniscalco, 2014/11/11 00:44:54]

crc -> crc32c (and elsewhere in this comment)

[pavely, 2014/11/11 22:27:14]

Done.

+      // match data. All other cases are fine. When crc is not in headers
+      // locally calculated one will be  stored and used for further checks.

[maniscalco, 2014/11/11 00:44:54]

extra space between be and stored

[pavely, 2014/11/11 22:27:14]

Done.

       result = DOWNLOAD_TRANSIENT_ERROR;
+    } else {
+      result = DOWNLOAD_SUCCESS;
     }
   } else if (response_code == net::HTTP_UNAUTHORIZED) {
     // Server tells us we've got a bad token so invalidate it.
@@ -163,7 +173,7 @@ void AttachmentDownloaderImpl::OnURLFetchComplete(
   } else if (response_code == net::URLFetcher::RESPONSE_CODE_INVALID) {
     result = DOWNLOAD_TRANSIENT_ERROR;
   }
-  ReportResult(download_state, result, attachment_data);
+  ReportResult(download_state, result, attachment_data, attachment_crc);
   state_map_.erase(iter);
 }
 
@@ -197,15 +207,16 @@ void AttachmentDownloaderImpl::RequestAccessToken(
 void AttachmentDownloaderImpl::ReportResult(
     const DownloadState& download_state,
     const DownloadResult& result,
-    const scoped_refptr<base::RefCountedString>& attachment_data) {
+    const scoped_refptr<base::RefCountedString>& attachment_data,
+    uint32_t attachment_crc) {

[maniscalco, 2014/11/11 00:44:54]

crc -> crc32c (and elsewhere in this file)

[pavely, 2014/11/11 22:27:15]

Done.

   std::vector<DownloadCallback>::const_iterator iter;
   for (iter = download_state.user_callbacks.begin();
        iter != download_state.user_callbacks.end();
        ++iter) {
     scoped_ptr<Attachment> attachment;
     if (result == DOWNLOAD_SUCCESS) {
-      attachment.reset(new Attachment(Attachment::CreateWithId(
-          download_state.attachment_id, attachment_data)));
+      attachment.reset(new Attachment(Attachment::RestoreExisting(
+          download_state.attachment_id, attachment_data, attachment_crc)));
     }
 
     base::MessageLoop::current()->PostTask(
@@ -213,51 +224,44 @@ void AttachmentDownloaderImpl::ReportResult(
   }
 }
 
-bool AttachmentDownloaderImpl::VerifyHashIfPresent(
-    const net::URLFetcher& fetcher,
-    const std::string& data) {
-  const net::HttpResponseHeaders* headers = fetcher.GetResponseHeaders();
+bool AttachmentDownloaderImpl::ExtractCrc32c(
+    const net::HttpResponseHeaders* headers,
+    uint32_t* crc32c) {
+  DCHECK(crc32c);
   if (!headers) {
-    // No headers?  It passes.
-    return true;
-  }
-
-  std::string value;
-  if (!ExtractCrc32c(*headers, &value)) {
-    // No crc32c?  It passes.
-    return true;
-  }
-
-  if (value ==
-      AttachmentUploaderImpl::ComputeCrc32cHash(data.data(), data.size())) {
-    return true;
-  } else {
     return false;
   }
-}
 
-bool AttachmentDownloaderImpl::ExtractCrc32c(
-    const net::HttpResponseHeaders& headers,
-    std::string* crc32c) {
-  DCHECK(crc32c);
+  std::string crc32c_encoded;
   std::string header_value;
   void* iter = NULL;
   // Iterate over all matching headers.
-  while (headers.EnumerateHeader(&iter, "x-goog-hash", &header_value)) {
+  while (headers->EnumerateHeader(&iter, "x-goog-hash", &header_value)) {
     // Because EnumerateHeader is smart about list values, header_value will
     // either be empty or a single name=value pair.
     net::HttpUtil::NameValuePairsIterator pair_iter(
         header_value.begin(), header_value.end(), ',');
     if (pair_iter.GetNext()) {
       if (pair_iter.name() == "crc32c") {
-        *crc32c = pair_iter.value();
+        crc32c_encoded = pair_iter.value();
         DCHECK(!pair_iter.GetNext());
-        return true;
+        break;
       }
     }
   }
+  // Check if header was found
+  if (crc32c_encoded.empty())
+    return false;
+  std::string crc32c_raw;
+  if (!base::Base64Decode(crc32c_encoded, &crc32c_raw))
+    return false;
+
+  if (crc32c_raw.size() != sizeof(*crc32c))
+    return false;
 
-  return false;
+  *crc32c =
+      base::NetToHost32(*reinterpret_cast<const uint32_t*>(crc32c_raw.c_str()));
+  return true;
 }
 
 }  // namespace syncer