Revert of Consolidate attachment URL construction. (https://codereview.chromium.org/355093002/)

Reason for revert:
Broke compile on ChromiumOS on Linux:

../../chrome/browser/sync/profile_sync_components_factory_impl.cc: In member function 'virtual scoped_ptr<syncer::AttachmentService> ProfileSyncComponentsFactoryImpl::CreateAttachmentService(syncer::AttachmentService::Delegate*)':
../../chrome/browser/sync/profile_sync_components_factory_impl.cc:648:44: error: 'url_prefix' was not declared in this scope
ninja: build stopped: subcommand failed.

http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Full/builds/6230/steps/compile/logs/stdio

Original issue's description:
> Consolidate attachment URL construction.
> 
> Fix bug where attachment URLs could end up with too many slashes.
> 
> BUG=
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=280193

TBR=pavely@chromium.org,maniscalco@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@280196 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 23 insertions, 65 deletions

diff --git a/sync/internal_api/attachments/attachment_downloader_impl.cc b/sync/internal_api/attachments/attachment_downloader_impl.cc
index 740c4fa..18c49d8 100644
--- a/sync/internal_api/attachments/attachment_downloader_impl.cc
+++ b/sync/internal_api/attachments/attachment_downloader_impl.cc
@@ -9,7 +9,6 @@
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_fetcher.h"
-#include "sync/internal_api/public/attachments/attachment_uploader_impl.h"
 #include "sync/protocol/sync.pb.h"
 #include "url/gurl.h"
 
@@ -36,7 +35,7 @@ AttachmentDownloaderImpl::DownloadState::DownloadState(
 }
 
 AttachmentDownloaderImpl::AttachmentDownloaderImpl(
-    const GURL& sync_service_url,
+    const std::string& url_prefix,
     const scoped_refptr<net::URLRequestContextGetter>&
         url_request_context_getter,
     const std::string& account_id,
@@ -44,13 +43,14 @@ AttachmentDownloaderImpl::AttachmentDownloaderImpl(
     scoped_ptr<OAuth2TokenServiceRequest::TokenServiceProvider>
         token_service_provider)
     : OAuth2TokenService::Consumer("attachment-downloader-impl"),
-      sync_service_url_(sync_service_url),
+      url_prefix_(url_prefix),
       url_request_context_getter_(url_request_context_getter),
       account_id_(account_id),
       oauth2_scopes_(scopes),
       token_service_provider_(token_service_provider.Pass()) {
   DCHECK(token_service_provider_);
   DCHECK(url_request_context_getter_);
+  DCHECK(!url_prefix_.empty());
 }
 
 AttachmentDownloaderImpl::~AttachmentDownloaderImpl() {
@@ -61,8 +61,7 @@ void AttachmentDownloaderImpl::DownloadAttachment(
     const DownloadCallback& callback) {
   DCHECK(CalledOnValidThread());
 
-  AttachmentUrl url = AttachmentUploaderImpl::GetURLForAttachmentId(
-                          sync_service_url_, attachment_id).spec();
+  AttachmentUrl url = GetAttachmentUrl(attachment_id);
 
   StateMap::iterator iter = state_map_.find(url);
   if (iter == state_map_.end()) {
@@ -152,6 +151,11 @@ void AttachmentDownloaderImpl::OnURLFetchComplete(
   state_map_.erase(iter);
 }
 
+AttachmentDownloaderImpl::AttachmentUrl
+AttachmentDownloaderImpl::GetAttachmentUrl(const AttachmentId& attachment_id) {
+  return url_prefix_ + attachment_id.GetProto().unique_id();
+}
+
 scoped_ptr<net::URLFetcher> AttachmentDownloaderImpl::CreateFetcher(
     const AttachmentUrl& url,
     const std::string& access_token) {
diff --git a/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc b/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
index b8938cd..f8d1393 100644
--- a/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc
@@ -187,7 +187,7 @@ void AttachmentDownloaderImplTest::SetUp() {
   OAuth2TokenService::ScopeSet scopes;
   scopes.insert(GaiaConstants::kChromeSyncOAuth2Scope);
   attachment_downloader_ =
-      AttachmentDownloader::Create(GURL(kAttachmentServerUrl),
+      AttachmentDownloader::Create(kAttachmentServerUrl,
                                    url_request_context_getter_,
                                    kAccountId,
                                    scopes,
diff --git a/sync/internal_api/attachments/attachment_uploader_impl.cc b/sync/internal_api/attachments/attachment_uploader_impl.cc
index 9048b20..90c620e5 100644
--- a/sync/internal_api/attachments/attachment_uploader_impl.cc
+++ b/sync/internal_api/attachments/attachment_uploader_impl.cc
@@ -14,11 +14,11 @@
 #include "net/url_request/url_fetcher_delegate.h"
 #include "sync/api/attachments/attachment.h"
 #include "sync/protocol/sync.pb.h"
+#include "url/gurl.h"
 
 namespace {
 
 const char kContentType[] = "application/octet-stream";
-const char kAttachments[] = "attachments/";
 
 }  // namespace
 
@@ -209,14 +209,14 @@ void AttachmentUploaderImpl::UploadState::ReportResult(
 }
 
 AttachmentUploaderImpl::AttachmentUploaderImpl(
-    const GURL& sync_service_url,
+    const std::string& url_prefix,
     const scoped_refptr<net::URLRequestContextGetter>&
         url_request_context_getter,
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet& scopes,
     scoped_ptr<OAuth2TokenServiceRequest::TokenServiceProvider>
         token_service_provider)
-    : sync_service_url_(sync_service_url),
+    : url_prefix_(url_prefix),
       url_request_context_getter_(url_request_context_getter),
       account_id_(account_id),
       scopes_(scopes),
@@ -237,7 +237,7 @@ void AttachmentUploaderImpl::UploadAttachment(const Attachment& attachment,
   DCHECK(!unique_id.empty());
   StateMap::iterator iter = state_map_.find(unique_id);
   if (iter == state_map_.end()) {
-    const GURL url = GetURLForAttachmentId(sync_service_url_, attachment_id);
+    const GURL url = GetUploadURLForAttachmentId(attachment_id);
     scoped_ptr<UploadState> upload_state(
         new UploadState(url,
                         url_request_context_getter_,
@@ -256,19 +256,9 @@ void AttachmentUploaderImpl::UploadAttachment(const Attachment& attachment,
   }
 }
 
-// Static.
-GURL AttachmentUploaderImpl::GetURLForAttachmentId(
-    const GURL& sync_service_url,
-    const AttachmentId& attachment_id) {
-  std::string path = sync_service_url.path();
-  if (path.empty() || *path.rbegin() != '/') {
-    path += '/';
-  }
-  path += kAttachments;
-  path += attachment_id.GetProto().unique_id();
-  GURL::Replacements replacements;
-  replacements.SetPathStr(path);
-  return sync_service_url.ReplaceComponents(replacements);
+GURL AttachmentUploaderImpl::GetUploadURLForAttachmentId(
+    const AttachmentId& attachment_id) const {
+  return GURL(url_prefix_ + attachment_id.GetProto().unique_id());
 }
 
 void AttachmentUploaderImpl::DeleteUploadStateFor(const UniqueId& unique_id) {
diff --git a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
index 90cb07d..3e3c67e 100644
--- a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
@@ -33,7 +33,6 @@ const char kAttachmentData[] = "some data";
 const char kAccountId[] = "some-account-id";
 const char kAccessToken[] = "some-access-token";
 const char kAuthorization[] = "Authorization";
-const char kAttachments[] = "/attachments/";
 
 }  // namespace
 
@@ -265,7 +264,8 @@ void AttachmentUploaderImplTest::SetUp() {
       base::Bind(&RequestHandler::HandleRequest,
                  base::Unretained(request_handler_.get())));
 
-  GURL url(base::StringPrintf("http://localhost:%d/", server_.port()));
+  std::string url_prefix(
+      base::StringPrintf("http://localhost:%d/uploads/", server_.port()));
 
   token_service_.reset(new MockOAuth2TokenService);
   scoped_ptr<OAuth2TokenServiceRequest::TokenServiceProvider>
@@ -273,7 +273,7 @@ void AttachmentUploaderImplTest::SetUp() {
 
   OAuth2TokenService::ScopeSet scopes;
   scopes.insert(GaiaConstants::kChromeSyncOAuth2Scope);
-  uploader().reset(new AttachmentUploaderImpl(url,
+  uploader().reset(new AttachmentUploaderImpl(url_prefix,
                                               url_request_context_getter_,
                                               kAccountId,
                                               scopes,
@@ -377,42 +377,6 @@ net::HttpStatusCode RequestHandler::GetStatusCode() const {
   return status_code_;
 }
 
-TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_NoPath) {
-  AttachmentId id = AttachmentId::Create();
-  std::string unique_id = id.GetProto().unique_id();
-  GURL sync_service_url("https://example.com");
-  EXPECT_EQ("https://example.com/attachments/" + unique_id,
-            AttachmentUploaderImpl::GetURLForAttachmentId(sync_service_url, id)
-                .spec());
-}
-
-TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_JustSlash) {
-  AttachmentId id = AttachmentId::Create();
-  std::string unique_id = id.GetProto().unique_id();
-  GURL sync_service_url("https://example.com/");
-  EXPECT_EQ("https://example.com/attachments/" + unique_id,
-            AttachmentUploaderImpl::GetURLForAttachmentId(sync_service_url, id)
-                .spec());
-}
-
-TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_Path) {
-  AttachmentId id = AttachmentId::Create();
-  std::string unique_id = id.GetProto().unique_id();
-  GURL sync_service_url("https://example.com/service");
-  EXPECT_EQ("https://example.com/service/attachments/" + unique_id,
-            AttachmentUploaderImpl::GetURLForAttachmentId(sync_service_url, id)
-                .spec());
-}
-
-TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_PathAndSlash) {
-  AttachmentId id = AttachmentId::Create();
-  std::string unique_id = id.GetProto().unique_id();
-  GURL sync_service_url("https://example.com/service/");
-  EXPECT_EQ("https://example.com/service/attachments/" + unique_id,
-            AttachmentUploaderImpl::GetURLForAttachmentId(sync_service_url, id)
-                .spec());
-}
-
 // Verify the "happy case" of uploading an attachment.
 //
 // Token is requested, token is returned, HTTP request is made, attachment is
@@ -439,7 +403,7 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_HappyCase) {
   ASSERT_EQ(1U, http_requests_received().size());
   const HttpRequest& http_request = http_requests_received().front();
   EXPECT_EQ(net::test_server::METHOD_POST, http_request.method);
-  std::string expected_relative_url(kAttachments +
+  std::string expected_relative_url("/uploads/" +
                                     attachment.GetId().GetProto().unique_id());
   EXPECT_EQ(expected_relative_url, http_request.relative_url);
   EXPECT_TRUE(http_request.has_content);
@@ -542,7 +506,7 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_ServiceUnavilable) {
   ASSERT_EQ(1U, http_requests_received().size());
   const HttpRequest& http_request = http_requests_received().front();
   EXPECT_EQ(net::test_server::METHOD_POST, http_request.method);
-  std::string expected_relative_url(kAttachments +
+  std::string expected_relative_url("/uploads/" +
                                     attachment.GetId().GetProto().unique_id());
   EXPECT_EQ(expected_relative_url, http_request.relative_url);
   EXPECT_TRUE(http_request.has_content);
@@ -580,7 +544,7 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_BadToken) {
   ASSERT_EQ(1U, http_requests_received().size());
   const HttpRequest& http_request = http_requests_received().front();
   EXPECT_EQ(net::test_server::METHOD_POST, http_request.method);
-  std::string expected_relative_url(kAttachments +
+  std::string expected_relative_url("/uploads/" +
                                     attachment.GetId().GetProto().unique_id());
   EXPECT_EQ(expected_relative_url, http_request.relative_url);
   EXPECT_TRUE(http_request.has_content);