Include model type in attachment uploads and downloads

Pass model type using EntitySpecifics field number in HTTP header on
attachment uploads and downloads.  We use the field number because
that's a sync-standard way of communicating model type to the server.

Update comment for GetSpecificsFieldNumberFromModelType to reflect the
fact that this method is use outside of test code.

BUG=440928

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

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

9 files changed, 63 insertions, 21 deletions

diff --git a/sync/internal_api/attachments/attachment_downloader.cc b/sync/internal_api/attachments/attachment_downloader.cc
index fb60201..5b52eb0 100644
--- a/sync/internal_api/attachments/attachment_downloader.cc
+++ b/sync/internal_api/attachments/attachment_downloader.cc
@@ -22,10 +22,11 @@ scoped_ptr<AttachmentDownloader> AttachmentDownloader::Create(
     const OAuth2TokenService::ScopeSet scopes,
     const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
         token_service_provider,
-    const std::string& store_birthday) {
+    const std::string& store_birthday,
+    ModelType model_type) {
   return scoped_ptr<AttachmentDownloader>(new AttachmentDownloaderImpl(
       sync_service_url, url_request_context_getter, account_id, scopes,
-      token_service_provider, store_birthday));
+      token_service_provider, store_birthday, model_type));
 }
 
 }  // namespace syncer
diff --git a/sync/internal_api/attachments/attachment_downloader_impl.cc b/sync/internal_api/attachments/attachment_downloader_impl.cc
index 9e6c488..33ac885 100644
--- a/sync/internal_api/attachments/attachment_downloader_impl.cc
+++ b/sync/internal_api/attachments/attachment_downloader_impl.cc
@@ -51,14 +51,16 @@ AttachmentDownloaderImpl::AttachmentDownloaderImpl(
     const OAuth2TokenService::ScopeSet& scopes,
     const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
         token_service_provider,
-    const std::string& store_birthday)
+    const std::string& store_birthday,
+    ModelType model_type)
     : 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),
-      raw_store_birthday_(store_birthday) {
+      raw_store_birthday_(store_birthday),
+      model_type_(model_type) {
   DCHECK(url_request_context_getter_.get());
   DCHECK(!account_id.empty());
   DCHECK(!scopes.empty());
@@ -194,7 +196,7 @@ scoped_ptr<net::URLFetcher> AttachmentDownloaderImpl::CreateFetcher(
   scoped_ptr<net::URLFetcher> url_fetcher(
       net::URLFetcher::Create(GURL(url), net::URLFetcher::GET, this));
   AttachmentUploaderImpl::ConfigureURLFetcherCommon(
-      url_fetcher.get(), access_token, raw_store_birthday_,
+      url_fetcher.get(), access_token, raw_store_birthday_, model_type_,
       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 501e01f..96ea5ea 100644
--- a/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
@@ -18,6 +18,7 @@
 #include "sync/api/attachments/attachment.h"
 #include "sync/internal_api/public/attachments/attachment_uploader_impl.h"
 #include "sync/internal_api/public/attachments/attachment_util.h"
+#include "sync/internal_api/public/base/model_type.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/leveldatabase/src/util/crc32c.h"
 
@@ -30,6 +31,7 @@ const char kAccessToken[] = "access.token";
 const char kAttachmentServerUrl[] = "http://attachments.com/";
 const char kAttachmentContent[] = "attachment.content";
 const char kStoreBirthday[] = "z00000000-0000-007b-0000-0000000004d2";
+const syncer::ModelType kModelType = syncer::ModelType::ARTICLES;
 
 // MockOAuth2TokenService remembers last request for access token and verifies
 // that only one request is active at a time.
@@ -206,7 +208,7 @@ void AttachmentDownloaderImplTest::SetUp() {
   scopes.insert(GaiaConstants::kChromeSyncOAuth2Scope);
   attachment_downloader_ = AttachmentDownloader::Create(
       GURL(kAttachmentServerUrl), url_request_context_getter_, kAccountId,
-      scopes, token_service_provider, std::string(kStoreBirthday));
+      scopes, token_service_provider, std::string(kStoreBirthday), kModelType);
 }
 
 void AttachmentDownloaderImplTest::TearDown() {
diff --git a/sync/internal_api/attachments/attachment_uploader_impl.cc b/sync/internal_api/attachments/attachment_uploader_impl.cc
index 3819fec..e66330c 100644
--- a/sync/internal_api/attachments/attachment_uploader_impl.cc
+++ b/sync/internal_api/attachments/attachment_uploader_impl.cc
@@ -29,6 +29,7 @@ namespace {
 const char kContentType[] = "application/octet-stream";
 const char kAttachments[] = "attachments/";
 const char kSyncStoreBirthday[] = "X-Sync-Store-Birthday";
+const char kSyncDataTypeId[] = "X-Sync-Data-Type-Id";
 
 }  // namespace
 
@@ -57,7 +58,8 @@ class AttachmentUploaderImpl::UploadState : public net::URLFetcherDelegate,
       const OAuth2TokenService::ScopeSet& scopes,
       OAuth2TokenServiceRequest::TokenServiceProvider* token_service_provider,
       const std::string& raw_store_birthday,
-      const base::WeakPtr<AttachmentUploaderImpl>& owner);
+      const base::WeakPtr<AttachmentUploaderImpl>& owner,
+      ModelType model_type);
 
   ~UploadState() override;
 
@@ -108,6 +110,7 @@ class AttachmentUploaderImpl::UploadState : public net::URLFetcherDelegate,
   // Pointer to the AttachmentUploaderImpl that owns this object.
   base::WeakPtr<AttachmentUploaderImpl> owner_;
   scoped_ptr<OAuth2TokenServiceRequest> access_token_request_;
+  ModelType model_type_;
 
   DISALLOW_COPY_AND_ASSIGN(UploadState);
 };
@@ -122,7 +125,8 @@ AttachmentUploaderImpl::UploadState::UploadState(
     const OAuth2TokenService::ScopeSet& scopes,
     OAuth2TokenServiceRequest::TokenServiceProvider* token_service_provider,
     const std::string& raw_store_birthday,
-    const base::WeakPtr<AttachmentUploaderImpl>& owner)
+    const base::WeakPtr<AttachmentUploaderImpl>& owner,
+    ModelType model_type)
     : OAuth2TokenService::Consumer("attachment-uploader-impl"),
       is_stopped_(false),
       upload_url_(upload_url),
@@ -133,7 +137,8 @@ AttachmentUploaderImpl::UploadState::UploadState(
       scopes_(scopes),
       raw_store_birthday_(raw_store_birthday),
       token_service_provider_(token_service_provider),
-      owner_(owner) {
+      owner_(owner),
+      model_type_(model_type) {
   DCHECK(upload_url_.is_valid());
   DCHECK(url_request_context_getter_.get());
   DCHECK(!account_id_.empty());
@@ -208,7 +213,7 @@ void AttachmentUploaderImpl::UploadState::OnGetTokenSuccess(
   fetcher_.reset(
       net::URLFetcher::Create(upload_url_, net::URLFetcher::POST, this));
   ConfigureURLFetcherCommon(fetcher_.get(), access_token_, raw_store_birthday_,
-                            url_request_context_getter_.get());
+                            model_type_, url_request_context_getter_.get());
 
   const uint32_t crc32c = attachment_.GetCrc32c();
   fetcher_->AddExtraRequestHeader(base::StringPrintf(
@@ -273,13 +278,15 @@ AttachmentUploaderImpl::AttachmentUploaderImpl(
     const OAuth2TokenService::ScopeSet& scopes,
     const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
         token_service_provider,
-    const std::string& store_birthday)
+    const std::string& store_birthday,
+    ModelType model_type)
     : 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),
+      model_type_(model_type),
       weak_ptr_factory_(this) {
   DCHECK(CalledOnValidThread());
   DCHECK(!account_id.empty());
@@ -314,10 +321,10 @@ 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(),
-                      raw_store_birthday_, weak_ptr_factory_.GetWeakPtr()));
+  scoped_ptr<UploadState> upload_state(new UploadState(
+      url, url_request_context_getter_, attachment, callback, account_id_,
+      scopes_, token_service_provider_.get(), raw_store_birthday_,
+      weak_ptr_factory_.GetWeakPtr(), model_type_));
   state_map_.add(unique_id, upload_state.Pass());
 }
 
@@ -360,6 +367,7 @@ void AttachmentUploaderImpl::ConfigureURLFetcherCommon(
     net::URLFetcher* fetcher,
     const std::string& access_token,
     const std::string& raw_store_birthday,
+    ModelType model_type,
     net::URLRequestContextGetter* request_context_getter) {
   DCHECK(request_context_getter);
   DCHECK(fetcher);
@@ -377,6 +385,12 @@ void AttachmentUploaderImpl::ConfigureURLFetcherCommon(
   Base64URLSafeEncode(raw_store_birthday, &encoded_store_birthday);
   fetcher->AddExtraRequestHeader(base::StringPrintf(
       "%s: %s", kSyncStoreBirthday, encoded_store_birthday.c_str()));
+
+  // Use field number to pass ModelType because it's stable and we have server
+  // code to decode it.
+  const int field_number = GetSpecificsFieldNumberFromModelType(model_type);
+  fetcher->AddExtraRequestHeader(
+      base::StringPrintf("%s: %d", kSyncDataTypeId, field_number));
 }
 
 void AttachmentUploaderImpl::Base64URLSafeEncode(const std::string& input,
diff --git a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
index 0c5c45f..2daff32 100644
--- a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
@@ -11,6 +11,7 @@
 #include "base/memory/ref_counted_memory.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/lock.h"
 #include "base/test/histogram_tester.h"
@@ -27,6 +28,7 @@
 #include "net/url_request/url_request_test_util.h"
 #include "sync/api/attachments/attachment.h"
 #include "sync/internal_api/public/attachments/attachment_util.h"
+#include "sync/internal_api/public/base/model_type.h"
 #include "sync/protocol/sync.pb.h"
 #include "testing/gmock/include/gmock/gmock-matchers.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -46,6 +48,8 @@ const char kStoreBirthday[] =
 const char kBase64URLSafeStoreBirthday[] =
     "Rv_d4HQ6SP2dBpM8YY6lcAlZmQVhRiFhGwPTAmDNQVX-JhXXDA";
 const char kSyncStoreBirthdayHeader[] = "X-Sync-Store-Birthday";
+const char kSyncDataTypeIdHeader[] = "X-Sync-Data-Type-Id";
+const syncer::ModelType kModelType = syncer::ModelType::ARTICLES;
 
 }  // namespace
 
@@ -293,7 +297,7 @@ void AttachmentUploaderImplTest::SetUp() {
   scopes.insert(GaiaConstants::kChromeSyncOAuth2Scope);
   uploader().reset(new AttachmentUploaderImpl(
       url, url_request_context_getter_, kAccountId, scopes,
-      token_service_provider, std::string(kStoreBirthday)));
+      token_service_provider, std::string(kStoreBirthday), kModelType));
 
   upload_callback_ = base::Bind(&AttachmentUploaderImplTest::UploadDone,
                                 base::Unretained(this));
@@ -506,6 +510,11 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_Headers) {
   EXPECT_THAT(http_request.headers,
               testing::Contains(testing::Pair(kSyncStoreBirthdayHeader,
                                               kBase64URLSafeStoreBirthday)));
+  EXPECT_THAT(http_request.headers,
+              testing::Contains(testing::Pair(
+                  kSyncDataTypeIdHeader,
+                  base::IntToString(
+                      GetSpecificsFieldNumberFromModelType(kModelType)))));
 }
 
 // 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 8b7180c..1b2e922 100644
--- a/sync/internal_api/public/attachments/attachment_downloader.h
+++ b/sync/internal_api/public/attachments/attachment_downloader.h
@@ -10,6 +10,7 @@
 #include "google_apis/gaia/oauth2_token_service_request.h"
 #include "sync/api/attachments/attachment.h"
 #include "sync/base/sync_export.h"
+#include "sync/internal_api/public/base/model_type.h"
 
 namespace net {
 class URLRequestContextGetter;
@@ -53,6 +54,8 @@ class SYNC_EXPORT AttachmentDownloader {
   // |token_service_provider| provides an OAuth2 token service.
   //
   // |store_birthday| is the raw, sync store birthday.
+  //
+  // |model_type| is the model type this downloader is used with.
   static scoped_ptr<AttachmentDownloader> Create(
       const GURL& sync_service_url,
       const scoped_refptr<net::URLRequestContextGetter>&
@@ -61,7 +64,8 @@ class SYNC_EXPORT AttachmentDownloader {
       const OAuth2TokenService::ScopeSet scopes,
       const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
           token_service_provider,
-      const std::string& store_birthday);
+      const std::string& store_birthday,
+      ModelType model_type);
 };
 
 }  // 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 d6fa533..4e68f27 100644
--- a/sync/internal_api/public/attachments/attachment_downloader_impl.h
+++ b/sync/internal_api/public/attachments/attachment_downloader_impl.h
@@ -11,6 +11,7 @@
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "sync/internal_api/public/attachments/attachment_downloader.h"
+#include "sync/internal_api/public/base/model_type.h"
 #include "url/gurl.h"
 
 namespace base {
@@ -40,6 +41,8 @@ class AttachmentDownloaderImpl : public AttachmentDownloader,
   // |token_service_provider| provides an OAuth2 token service.
   //
   // |store_birthday| is the raw, sync store birthday.
+  //
+  // |model_type| is the model type this downloader is used with.
   AttachmentDownloaderImpl(
       const GURL& sync_service_url,
       const scoped_refptr<net::URLRequestContextGetter>&
@@ -48,7 +51,8 @@ class AttachmentDownloaderImpl : public AttachmentDownloader,
       const OAuth2TokenService::ScopeSet& scopes,
       const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
           token_service_provider,
-      const std::string& store_birthday);
+      const std::string& store_birthday,
+      ModelType model_type);
   ~AttachmentDownloaderImpl() override;
 
   // AttachmentDownloader implementation.
@@ -112,6 +116,8 @@ class AttachmentDownloaderImpl : public AttachmentDownloader,
   // lifetime.
   StateList requests_waiting_for_access_token_;
 
+  ModelType model_type_;
+
   DISALLOW_COPY_AND_ASSIGN(AttachmentDownloaderImpl);
 };
 
diff --git a/sync/internal_api/public/attachments/attachment_uploader_impl.h b/sync/internal_api/public/attachments/attachment_uploader_impl.h
index fe77379..1f85bc6 100644
--- a/sync/internal_api/public/attachments/attachment_uploader_impl.h
+++ b/sync/internal_api/public/attachments/attachment_uploader_impl.h
@@ -11,6 +11,7 @@
 #include "google_apis/gaia/oauth2_token_service_request.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "sync/internal_api/public/attachments/attachment_uploader.h"
+#include "sync/internal_api/public/base/model_type.h"
 
 class GURL;
 
@@ -35,6 +36,8 @@ class SYNC_EXPORT AttachmentUploaderImpl : public AttachmentUploader,
   // |token_service_provider| provides an OAuth2 token service.
   //
   // |store_birthday| is the raw, sync store birthday.
+  //
+  // |model_type| is the model type this uploader is used with.
   AttachmentUploaderImpl(
       const GURL& sync_service_url,
       const scoped_refptr<net::URLRequestContextGetter>&
@@ -43,7 +46,8 @@ class SYNC_EXPORT AttachmentUploaderImpl : public AttachmentUploader,
       const OAuth2TokenService::ScopeSet& scopes,
       const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
           token_service_provider,
-      const std::string& store_birthday);
+      const std::string& store_birthday,
+      ModelType model_type);
   ~AttachmentUploaderImpl() override;
 
   // AttachmentUploader implementation.
@@ -67,6 +71,7 @@ class SYNC_EXPORT AttachmentUploaderImpl : public AttachmentUploader,
       net::URLFetcher* fetcher,
       const std::string& auth_token,
       const std::string& raw_store_birthday,
+      ModelType model_type,
       net::URLRequestContextGetter* request_context_getter);
 
  private:
@@ -89,6 +94,7 @@ class SYNC_EXPORT AttachmentUploaderImpl : public AttachmentUploader,
       token_service_provider_;
   std::string raw_store_birthday_;
   StateMap state_map_;
+  ModelType model_type_;
 
   // Must be last data member.
   base::WeakPtrFactory<AttachmentUploaderImpl> weak_ptr_factory_;
diff --git a/sync/internal_api/public/base/model_type.h b/sync/internal_api/public/base/model_type.h
index 716bece..13c7373 100644
--- a/sync/internal_api/public/base/model_type.h
+++ b/sync/internal_api/public/base/model_type.h
@@ -251,8 +251,6 @@ SYNC_EXPORT_PRIVATE ModelType GetModelTypeFromSpecificsFieldNumber(
 
 // Return the field number of the EntitySpecifics field associated with
 // a model type.
-//
-// Used by tests outside of sync.
 SYNC_EXPORT int GetSpecificsFieldNumberFromModelType(
     ModelType model_type);