Send birthday on sync attachment uploads and downloads.

Refactor common URLFetcher settings into shared static method
(ConfigureURLFetcherCommon) called by both uploader and downloader.

Include User-Agent on uploads and downloads to aid in troubleshooting.

Use header constants from net::HttpRequestHeaders.

Make ReadTransaction's GetAttachmentIdsToUpload const.

BUG=371521, 433898

Review URL: https://codereview.chromium.org/729363005

Cr-Commit-Position: refs/heads/master@{#304541}

10 files changed, 114 insertions, 65 deletions

diff --git a/sync/internal_api/attachments/attachment_downloader.cc b/sync/internal_api/attachments/attachment_downloader.cc
index a3c780f..fb60201 100644
--- a/sync/internal_api/attachments/attachment_downloader.cc
+++ b/sync/internal_api/attachments/attachment_downloader.cc
@@ -21,13 +21,11 @@ scoped_ptr<AttachmentDownloader> AttachmentDownloader::Create(
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet scopes,
     const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
-        token_service_provider) {
-  return scoped_ptr<AttachmentDownloader>(
-      new AttachmentDownloaderImpl(sync_service_url,
-                                   url_request_context_getter,
-                                   account_id,
-                                   scopes,
-                                   token_service_provider));
+        token_service_provider,
+    const std::string& store_birthday) {
+  return scoped_ptr<AttachmentDownloader>(new AttachmentDownloaderImpl(
+      sync_service_url, url_request_context_getter, account_id, scopes,
+      token_service_provider, store_birthday));
 }
 
 }  // namespace syncer
diff --git a/sync/internal_api/attachments/attachment_downloader_impl.cc b/sync/internal_api/attachments/attachment_downloader_impl.cc
index 232b99e..c4edcec 100644
--- a/sync/internal_api/attachments/attachment_downloader_impl.cc
+++ b/sync/internal_api/attachments/attachment_downloader_impl.cc
@@ -47,17 +47,20 @@ AttachmentDownloaderImpl::AttachmentDownloaderImpl(
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet& scopes,
     const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
-        token_service_provider)
+        token_service_provider,
+    const std::string& store_birthday)
     : OAuth2TokenService::Consumer("attachment-downloader-impl"),
       sync_service_url_(sync_service_url),
       url_request_context_getter_(url_request_context_getter),
       account_id_(account_id),
       oauth2_scopes_(scopes),
-      token_service_provider_(token_service_provider) {
+      token_service_provider_(token_service_provider),
+      raw_store_birthday_(store_birthday) {
+  DCHECK(url_request_context_getter_.get());
   DCHECK(!account_id.empty());
   DCHECK(!scopes.empty());
   DCHECK(token_service_provider_.get());
-  DCHECK(url_request_context_getter_.get());
+  DCHECK(!raw_store_birthday_.empty());
 }
 
 AttachmentDownloaderImpl::~AttachmentDownloaderImpl() {
@@ -182,15 +185,9 @@ scoped_ptr<net::URLFetcher> AttachmentDownloaderImpl::CreateFetcher(
     const std::string& access_token) {
   scoped_ptr<net::URLFetcher> url_fetcher(
       net::URLFetcher::Create(GURL(url), net::URLFetcher::GET, this));
-  url_fetcher->SetAutomaticallyRetryOn5xx(false);
-  const std::string auth_header("Authorization: Bearer " + access_token);
-  url_fetcher->AddExtraRequestHeader(auth_header);
-  url_fetcher->SetRequestContext(url_request_context_getter_.get());
-  url_fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
-                            net::LOAD_DO_NOT_SEND_COOKIES |
-                            net::LOAD_DISABLE_CACHE);
-  // TODO(maniscalco): Set an appropriate headers (User-Agent, what else?) on
-  // the request (bug 371521).
+  AttachmentUploaderImpl::ConfigureURLFetcherCommon(
+      url_fetcher.get(), access_token, raw_store_birthday_,
+      url_request_context_getter_.get());
   return url_fetcher.Pass();
 }
 
diff --git a/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc b/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
index f36fbad..bd45627 100644
--- a/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
@@ -28,6 +28,7 @@ const char kAccountId[] = "attachments@gmail.com";
 const char kAccessToken[] = "access.token";
 const char kAttachmentServerUrl[] = "http://attachments.com/";
 const char kAttachmentContent[] = "attachment.content";
+const char kStoreBirthday[] = "z00000000-0000-007b-0000-0000000004d2";
 
 // MockOAuth2TokenService remembers last request for access token and verifies
 // that only one request is active at a time.
@@ -202,12 +203,9 @@ void AttachmentDownloaderImplTest::SetUp() {
 
   OAuth2TokenService::ScopeSet scopes;
   scopes.insert(GaiaConstants::kChromeSyncOAuth2Scope);
-  attachment_downloader_ =
-      AttachmentDownloader::Create(GURL(kAttachmentServerUrl),
-                                   url_request_context_getter_,
-                                   kAccountId,
-                                   scopes,
-                                   token_service_provider);
+  attachment_downloader_ = AttachmentDownloader::Create(
+      GURL(kAttachmentServerUrl), url_request_context_getter_, kAccountId,
+      scopes, token_service_provider, std::string(kStoreBirthday));
 }
 
 void AttachmentDownloaderImplTest::TearDown() {
diff --git a/sync/internal_api/attachments/attachment_uploader_impl.cc b/sync/internal_api/attachments/attachment_uploader_impl.cc
index 2f14263..7edbf94 100644
--- a/sync/internal_api/attachments/attachment_uploader_impl.cc
+++ b/sync/internal_api/attachments/attachment_uploader_impl.cc
@@ -25,6 +25,7 @@ namespace {
 
 const char kContentType[] = "application/octet-stream";
 const char kAttachments[] = "attachments/";
+const char kSyncStoreBirthday[] = "X-Sync-Store-Birthday";
 
 }  // namespace
 
@@ -52,6 +53,7 @@ class AttachmentUploaderImpl::UploadState : public net::URLFetcherDelegate,
       const std::string& account_id,
       const OAuth2TokenService::ScopeSet& scopes,
       OAuth2TokenServiceRequest::TokenServiceProvider* token_service_provider,
+      const std::string& raw_store_birthday,
       const base::WeakPtr<AttachmentUploaderImpl>& owner);
 
   ~UploadState() override;
@@ -98,6 +100,7 @@ class AttachmentUploaderImpl::UploadState : public net::URLFetcherDelegate,
   std::string account_id_;
   OAuth2TokenService::ScopeSet scopes_;
   std::string access_token_;
+  std::string raw_store_birthday_;
   OAuth2TokenServiceRequest::TokenServiceProvider* token_service_provider_;
   // Pointer to the AttachmentUploaderImpl that owns this object.
   base::WeakPtr<AttachmentUploaderImpl> owner_;
@@ -115,6 +118,7 @@ AttachmentUploaderImpl::UploadState::UploadState(
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet& scopes,
     OAuth2TokenServiceRequest::TokenServiceProvider* token_service_provider,
+    const std::string& raw_store_birthday,
     const base::WeakPtr<AttachmentUploaderImpl>& owner)
     : OAuth2TokenService::Consumer("attachment-uploader-impl"),
       is_stopped_(false),
@@ -124,6 +128,7 @@ AttachmentUploaderImpl::UploadState::UploadState(
       user_callbacks_(1, user_callback),
       account_id_(account_id),
       scopes_(scopes),
+      raw_store_birthday_(raw_store_birthday),
       token_service_provider_(token_service_provider),
       owner_(owner) {
   DCHECK(upload_url_.is_valid());
@@ -131,6 +136,7 @@ AttachmentUploaderImpl::UploadState::UploadState(
   DCHECK(!account_id_.empty());
   DCHECK(!scopes_.empty());
   DCHECK(token_service_provider_);
+  DCHECK(!raw_store_birthday_.empty());
   GetToken();
 }
 
@@ -195,8 +201,13 @@ void AttachmentUploaderImpl::UploadState::OnGetTokenSuccess(
   access_token_ = access_token;
   fetcher_.reset(
       net::URLFetcher::Create(upload_url_, net::URLFetcher::POST, this));
-  fetcher_->SetAutomaticallyRetryOn5xx(false);
-  fetcher_->SetRequestContext(url_request_context_getter_.get());
+  ConfigureURLFetcherCommon(fetcher_.get(), access_token_, raw_store_birthday_,
+                            url_request_context_getter_.get());
+
+  const uint32_t crc32c = attachment_.GetCrc32c();
+  fetcher_->AddExtraRequestHeader(base::StringPrintf(
+      "X-Goog-Hash: crc32c=%s", FormatCrc32cHash(crc32c).c_str()));
+
   // TODO(maniscalco): Is there a better way?  Copying the attachment data into
   // a string feels wrong given how large attachments may be (several MBs).  If
   // we may end up switching from URLFetcher to URLRequest, this copy won't be
@@ -204,16 +215,7 @@ void AttachmentUploaderImpl::UploadState::OnGetTokenSuccess(
   scoped_refptr<base::RefCountedMemory> memory = attachment_.GetData();
   const std::string upload_content(memory->front_as<char>(), memory->size());
   fetcher_->SetUploadData(kContentType, upload_content);
-  const std::string auth_header("Authorization: Bearer " + access_token_);
-  fetcher_->AddExtraRequestHeader(auth_header);
-  const uint32_t crc32c = attachment_.GetCrc32c();
-  fetcher_->AddExtraRequestHeader(base::StringPrintf(
-      "X-Goog-Hash: crc32c=%s", FormatCrc32cHash(crc32c).c_str()));
-  fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
-                         net::LOAD_DO_NOT_SEND_COOKIES |
-                         net::LOAD_DISABLE_CACHE);
-  // TODO(maniscalco): Set appropriate headers (e.g. User-Agent) on the request
-  // and include the "sync birthday" (bug 371521).
+
   fetcher_->Start();
 }
 
@@ -264,17 +266,20 @@ AttachmentUploaderImpl::AttachmentUploaderImpl(
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet& scopes,
     const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
-        token_service_provider)
+        token_service_provider,
+    const std::string& store_birthday)
     : sync_service_url_(sync_service_url),
       url_request_context_getter_(url_request_context_getter),
       account_id_(account_id),
       scopes_(scopes),
       token_service_provider_(token_service_provider),
+      raw_store_birthday_(store_birthday),
       weak_ptr_factory_(this) {
   DCHECK(CalledOnValidThread());
   DCHECK(!account_id.empty());
   DCHECK(!scopes.empty());
   DCHECK(token_service_provider_.get());
+  DCHECK(!raw_store_birthday_.empty());
 }
 
 AttachmentUploaderImpl::~AttachmentUploaderImpl() {
@@ -304,14 +309,9 @@ void AttachmentUploaderImpl::UploadAttachment(const Attachment& attachment,
 
   const GURL url = GetURLForAttachmentId(sync_service_url_, attachment_id);
   scoped_ptr<UploadState> upload_state(
-      new UploadState(url,
-                      url_request_context_getter_,
-                      attachment,
-                      callback,
-                      account_id_,
-                      scopes_,
-                      token_service_provider_.get(),
-                      weak_ptr_factory_.GetWeakPtr()));
+      new UploadState(url, url_request_context_getter_, attachment, callback,
+                      account_id_, scopes_, token_service_provider_.get(),
+                      raw_store_birthday_, weak_ptr_factory_.GetWeakPtr()));
   state_map_.add(unique_id, upload_state.Pass());
 }
 
@@ -350,4 +350,27 @@ std::string AttachmentUploaderImpl::FormatCrc32cHash(uint32_t crc32c) {
   return encoded;
 }
 
+void AttachmentUploaderImpl::ConfigureURLFetcherCommon(
+    net::URLFetcher* fetcher,
+    const std::string& access_token,
+    const std::string& raw_store_birthday,
+    net::URLRequestContextGetter* request_context_getter) {
+  DCHECK(request_context_getter);
+  DCHECK(fetcher);
+  fetcher->SetAutomaticallyRetryOn5xx(false);
+  fetcher->SetRequestContext(request_context_getter);
+  fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
+                        net::LOAD_DO_NOT_SEND_COOKIES |
+                        net::LOAD_DISABLE_CACHE);
+  fetcher->AddExtraRequestHeader(base::StringPrintf(
+      "%s: Bearer %s", net::HttpRequestHeaders::kAuthorization,
+      access_token.c_str()));
+  // Encode the birthday.  Birthday is opaque so we assume it could contain
+  // anything.  Encode it so that it's safe to pass as an HTTP header value.
+  std::string encoded_store_birthday;
+  base::Base64Encode(raw_store_birthday, &encoded_store_birthday);
+  fetcher->AddExtraRequestHeader(base::StringPrintf(
+      "%s: %s", kSyncStoreBirthday, encoded_store_birthday.c_str()));
+}
+
 }  // namespace syncer
diff --git a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
index d31fc18..2df6719 100644
--- a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "sync/internal_api/public/attachments/attachment_uploader_impl.h"
 
+#include "base/base64.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
@@ -18,6 +19,7 @@
 #include "google_apis/gaia/fake_oauth2_token_service.h"
 #include "google_apis/gaia/gaia_constants.h"
 #include "google_apis/gaia/oauth2_token_service_request.h"
+#include "net/http/http_status_code.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
@@ -33,12 +35,11 @@ namespace {
 const char kAttachmentData[] = "some data";
 const char kAccountId[] = "some-account-id";
 const char kAccessToken[] = "some-access-token";
-const char kAuthorization[] = "Authorization";
-const char kContentLength[] = "Content-Length";
-const char kContentType[] = "Content-Type";
 const char kContentTypeValue[] = "application/octet-stream";
 const char kXGoogHash[] = "X-Goog-Hash";
 const char kAttachments[] = "/attachments/";
+const char kStoreBirthday[] = "z00000000-0000-007b-0000-0000000004d2";
+const char kSyncStoreBirthdayHeader[] = "X-Sync-Store-Birthday";
 
 }  // namespace
 
@@ -284,11 +285,9 @@ void AttachmentUploaderImplTest::SetUp() {
 
   OAuth2TokenService::ScopeSet scopes;
   scopes.insert(GaiaConstants::kChromeSyncOAuth2Scope);
-  uploader().reset(new AttachmentUploaderImpl(url,
-                                              url_request_context_getter_,
-                                              kAccountId,
-                                              scopes,
-                                              token_service_provider));
+  uploader().reset(new AttachmentUploaderImpl(
+      url, url_request_context_getter_, kAccountId, scopes,
+      token_service_provider, std::string(kStoreBirthday)));
 
   upload_callback_ = base::Bind(&AttachmentUploaderImplTest::UploadDone,
                                 base::Unretained(this));
@@ -479,19 +478,27 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_Headers) {
   ASSERT_EQ(1U, http_requests_received().size());
   const HttpRequest& http_request = http_requests_received().front();
 
-  const std::string auth_header_name(kAuthorization);
   const std::string auth_header_value(std::string("Bearer ") + kAccessToken);
 
+  EXPECT_THAT(http_request.headers,
+              testing::Contains(testing::Pair(
+                  net::HttpRequestHeaders::kAuthorization, auth_header_value)));
   EXPECT_THAT(
       http_request.headers,
-      testing::Contains(testing::Pair(kAuthorization, auth_header_value)));
+      testing::Contains(testing::Key(net::HttpRequestHeaders::kContentLength)));
+  EXPECT_THAT(http_request.headers,
+              testing::Contains(testing::Pair(
+                  net::HttpRequestHeaders::kContentType, kContentTypeValue)));
   EXPECT_THAT(http_request.headers,
-              testing::Contains(testing::Key(kContentLength)));
+              testing::Contains(testing::Key(kXGoogHash)));
   EXPECT_THAT(
       http_request.headers,
-      testing::Contains(testing::Pair(kContentType, kContentTypeValue)));
+      testing::Contains(testing::Key(net::HttpRequestHeaders::kUserAgent)));
+  std::string encoded_store_birthday;
+  base::Base64Encode(kStoreBirthday, &encoded_store_birthday);
   EXPECT_THAT(http_request.headers,
-              testing::Contains(testing::Key(kXGoogHash)));
+              testing::Contains(testing::Pair(kSyncStoreBirthdayHeader,
+                                              encoded_store_birthday)));
 }
 
 // Verify two overlapping calls to upload the same attachment result in only one
diff --git a/sync/internal_api/public/attachments/attachment_downloader.h b/sync/internal_api/public/attachments/attachment_downloader.h
index 12db422..8b7180c 100644
--- a/sync/internal_api/public/attachments/attachment_downloader.h
+++ b/sync/internal_api/public/attachments/attachment_downloader.h
@@ -51,6 +51,8 @@ class SYNC_EXPORT AttachmentDownloader {
   // |scopes| is the set of scopes to use for downloads.
   //
   // |token_service_provider| provides an OAuth2 token service.
+  //
+  // |store_birthday| is the raw, sync store birthday.
   static scoped_ptr<AttachmentDownloader> Create(
       const GURL& sync_service_url,
       const scoped_refptr<net::URLRequestContextGetter>&
@@ -58,7 +60,8 @@ class SYNC_EXPORT AttachmentDownloader {
       const std::string& account_id,
       const OAuth2TokenService::ScopeSet scopes,
       const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
-          token_service_provider);
+          token_service_provider,
+      const std::string& store_birthday);
 };
 
 }  // namespace syncer
diff --git a/sync/internal_api/public/attachments/attachment_downloader_impl.h b/sync/internal_api/public/attachments/attachment_downloader_impl.h
index 2e75ce2..d6fa533 100644
--- a/sync/internal_api/public/attachments/attachment_downloader_impl.h
+++ b/sync/internal_api/public/attachments/attachment_downloader_impl.h
@@ -38,6 +38,8 @@ class AttachmentDownloaderImpl : public AttachmentDownloader,
   // |scopes| is the set of scopes to use for downloads.
   //
   // |token_service_provider| provides an OAuth2 token service.
+  //
+  // |store_birthday| is the raw, sync store birthday.
   AttachmentDownloaderImpl(
       const GURL& sync_service_url,
       const scoped_refptr<net::URLRequestContextGetter>&
@@ -45,7 +47,8 @@ class AttachmentDownloaderImpl : public AttachmentDownloader,
       const std::string& account_id,
       const OAuth2TokenService::ScopeSet& scopes,
       const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
-          token_service_provider);
+          token_service_provider,
+      const std::string& store_birthday);
   ~AttachmentDownloaderImpl() override;
 
   // AttachmentDownloader implementation.
@@ -101,6 +104,7 @@ class AttachmentDownloaderImpl : public AttachmentDownloader,
   scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>
       token_service_provider_;
   scoped_ptr<OAuth2TokenService::Request> access_token_request_;
+  std::string raw_store_birthday_;
 
   StateMap state_map_;
   // |requests_waiting_for_access_token_| only keeps references to DownloadState
diff --git a/sync/internal_api/public/attachments/attachment_uploader_impl.h b/sync/internal_api/public/attachments/attachment_uploader_impl.h
index d2279ef..9ef7bcd 100644
--- a/sync/internal_api/public/attachments/attachment_uploader_impl.h
+++ b/sync/internal_api/public/attachments/attachment_uploader_impl.h
@@ -33,6 +33,8 @@ class SYNC_EXPORT AttachmentUploaderImpl : public AttachmentUploader,
   // |scopes| is the set of scopes to use for uploads.
   //
   // |token_service_provider| provides an OAuth2 token service.
+  //
+  // |store_birthday| is the raw, sync store birthday.
   AttachmentUploaderImpl(
       const GURL& sync_service_url,
       const scoped_refptr<net::URLRequestContextGetter>&
@@ -40,7 +42,8 @@ class SYNC_EXPORT AttachmentUploaderImpl : public AttachmentUploader,
       const std::string& account_id,
       const OAuth2TokenService::ScopeSet& scopes,
       const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
-          token_service_provider);
+          token_service_provider,
+      const std::string& store_birthday);
   ~AttachmentUploaderImpl() override;
 
   // AttachmentUploader implementation.
@@ -58,6 +61,14 @@ class SYNC_EXPORT AttachmentUploaderImpl : public AttachmentUploader,
   // (https://cloud.google.com/storage/docs/reference-headers#xgooghash).
   static std::string FormatCrc32cHash(uint32_t crc32c);
 
+  // Apply common settings to |fetcher|, suitable for both uploads and
+  // downloads.
+  static void ConfigureURLFetcherCommon(
+      net::URLFetcher* fetcher,
+      const std::string& auth_token,
+      const std::string& raw_store_birthday,
+      net::URLRequestContextGetter* request_context_getter);
+
  private:
   class UploadState;
   typedef std::string UniqueId;
@@ -71,6 +82,7 @@ class SYNC_EXPORT AttachmentUploaderImpl : public AttachmentUploader,
   OAuth2TokenService::ScopeSet scopes_;
   scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>
       token_service_provider_;
+  std::string raw_store_birthday_;
   StateMap state_map_;
 
   // Must be last data member.
diff --git a/sync/internal_api/public/read_transaction.h b/sync/internal_api/public/read_transaction.h
index 2084b9a..c75d16a 100644
--- a/sync/internal_api/public/read_transaction.h
+++ b/sync/internal_api/public/read_transaction.h
@@ -46,9 +46,12 @@ class SYNC_EXPORT ReadTransaction : public BaseTransaction {
   void GetDataTypeContext(ModelType type,
                           sync_pb::DataTypeContext* context) const;
 
-  // Clears |id_set| and fills it with the ids of attachments that need to be
+  // Clear |id_set| and fill it with the ids of attachments that need to be
   // uploaded to the sync server.
-  void GetAttachmentIdsToUpload(ModelType type, AttachmentIdSet* id_set);
+  void GetAttachmentIdsToUpload(ModelType type, AttachmentIdSet* id_set) const;
+
+  // Return the current (opaque) store birthday.
+  std::string GetStoreBirthday() const;
 
  private:
   void* operator new(size_t size);  // Transaction is meant for stack use only.
diff --git a/sync/internal_api/read_transaction.cc b/sync/internal_api/read_transaction.cc
index 7d44fe4..872818f 100644
--- a/sync/internal_api/read_transaction.cc
+++ b/sync/internal_api/read_transaction.cc
@@ -48,10 +48,14 @@ void ReadTransaction::GetDataTypeContext(
 }
 
 void ReadTransaction::GetAttachmentIdsToUpload(ModelType type,
-                                               AttachmentIdSet* id_set) {
+                                               AttachmentIdSet* id_set) const {
   DCHECK(id_set);
   transaction_->directory()->GetAttachmentIdsToUpload(
       transaction_, type, id_set);
 }
 
+std::string ReadTransaction::GetStoreBirthday() const {
+  return transaction_->directory()->store_birthday();
+}
+
 }  // namespace syncer