Add resource ID based download requests in {GDataWapi/Drive} requests.

BUG=254025

Review URL: https://chromiumcodereview.appspot.com/18211008

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

11 files changed, 254 insertions, 40 deletions

diff --git a/chrome/browser/drive/drive_api_service.cc b/chrome/browser/drive/drive_api_service.cc
index b061366..b5a4b5a 100644
--- a/chrome/browser/drive/drive_api_service.cc
+++ b/chrome/browser/drive/drive_api_service.cc
@@ -25,7 +25,7 @@ using google_apis::AuthorizeAppCallback;
 using google_apis::CancelCallback;
 using google_apis::ChangeList;
 using google_apis::DownloadActionCallback;
-using google_apis::DownloadFileRequest;
+using google_apis::DownloadFileRequestBase;
 using google_apis::EntryActionCallback;
 using google_apis::FileList;
 using google_apis::FileResource;
@@ -491,13 +491,14 @@ CancelCallback DriveAPIService::DownloadFile(
   DCHECK(!download_action_callback.is_null());
   // get_content_callback may be null.
 
+  // TODO(kinaba): crbug.com/254025: use resource_id based download request.
   return sender_->StartRequestWithRetry(
-      new DownloadFileRequest(sender_.get(),
-                              download_action_callback,
-                              get_content_callback,
-                              progress_callback,
-                              download_url,
-                              local_cache_path));
+      new DownloadFileRequestBase(sender_.get(),
+                                  download_action_callback,
+                                  get_content_callback,
+                                  progress_callback,
+                                  download_url,
+                                  local_cache_path));
 }
 
 CancelCallback DriveAPIService::DeleteResource(
diff --git a/chrome/browser/drive/gdata_wapi_service.cc b/chrome/browser/drive/gdata_wapi_service.cc
index 582a980..57adfa0 100644
--- a/chrome/browser/drive/gdata_wapi_service.cc
+++ b/chrome/browser/drive/gdata_wapi_service.cc
@@ -32,7 +32,7 @@ using google_apis::CopyHostedDocumentRequest;
 using google_apis::CreateDirectoryRequest;
 using google_apis::DeleteResourceRequest;
 using google_apis::DownloadActionCallback;
-using google_apis::DownloadFileRequest;
+using google_apis::DownloadFileRequestBase;
 using google_apis::EntryActionCallback;
 using google_apis::GDataErrorCode;
 using google_apis::GDATA_PARSE_ERROR;
@@ -342,13 +342,14 @@ CancelCallback GDataWapiService::DownloadFile(
   DCHECK(!download_action_callback.is_null());
   // get_content_callback and progress_callback may be null.
 
+  // TODO(kinaba): crbug.com/254025: use resource_id based download request.
   return sender_->StartRequestWithRetry(
-      new DownloadFileRequest(sender_.get(),
-                              download_action_callback,
-                              get_content_callback,
-                              progress_callback,
-                              download_url,
-                              local_cache_path));
+      new DownloadFileRequestBase(sender_.get(),
+                                  download_action_callback,
+                                  get_content_callback,
+                                  progress_callback,
+                                  download_url,
+                                  local_cache_path));
 }
 
 CancelCallback GDataWapiService::DeleteResource(
diff --git a/chrome/browser/google_apis/base_requests.cc b/chrome/browser/google_apis/base_requests.cc
index 8452d07..49a5f07 100644
--- a/chrome/browser/google_apis/base_requests.cc
+++ b/chrome/browser/google_apis/base_requests.cc
@@ -602,9 +602,9 @@ GetUploadStatusRequestBase::GetExtraRequestHeaders() const {
   return headers;
 }
 
-//============================ DownloadFileRequest ===========================
+//============================ DownloadFileRequestBase =========================
 
-DownloadFileRequest::DownloadFileRequest(
+DownloadFileRequestBase::DownloadFileRequestBase(
     RequestSender* sender,
     const DownloadActionCallback& download_action_callback,
     const GetContentCallback& get_content_callback,
@@ -622,38 +622,40 @@ DownloadFileRequest::DownloadFileRequest(
   // get_content_callback may be null.
 }
 
-DownloadFileRequest::~DownloadFileRequest() {}
+DownloadFileRequestBase::~DownloadFileRequestBase() {}
 
 // Overridden from UrlFetchRequestBase.
-GURL DownloadFileRequest::GetURL() const {
+GURL DownloadFileRequestBase::GetURL() const {
   return download_url_;
 }
 
-bool DownloadFileRequest::GetOutputFilePath(base::FilePath* local_file_path) {
+bool DownloadFileRequestBase::GetOutputFilePath(
+    base::FilePath* local_file_path) {
   // Configure so that the downloaded content is saved to |output_file_path_|.
   *local_file_path = output_file_path_;
   return true;
 }
 
-void DownloadFileRequest::OnURLFetchDownloadProgress(const URLFetcher* source,
-                                                     int64 current,
-                                                     int64 total) {
+void DownloadFileRequestBase::OnURLFetchDownloadProgress(
+    const URLFetcher* source,
+    int64 current,
+    int64 total) {
   if (!progress_callback_.is_null())
     progress_callback_.Run(current, total);
 }
 
-bool DownloadFileRequest::ShouldSendDownloadData() {
+bool DownloadFileRequestBase::ShouldSendDownloadData() {
   return !get_content_callback_.is_null();
 }
 
-void DownloadFileRequest::OnURLFetchDownloadData(
+void DownloadFileRequestBase::OnURLFetchDownloadData(
     const URLFetcher* source,
     scoped_ptr<std::string> download_data) {
   if (!get_content_callback_.is_null())
     get_content_callback_.Run(HTTP_SUCCESS, download_data.Pass());
 }
 
-void DownloadFileRequest::ProcessURLFetchResults(const URLFetcher* source) {
+void DownloadFileRequestBase::ProcessURLFetchResults(const URLFetcher* source) {
   GDataErrorCode code = GetErrorCode(source);
 
   // Take over the ownership of the the downloaded temp file.
@@ -668,7 +670,8 @@ void DownloadFileRequest::ProcessURLFetchResults(const URLFetcher* source) {
   OnProcessURLFetchResultsComplete(code == HTTP_SUCCESS);
 }
 
-void DownloadFileRequest::RunCallbackOnPrematureFailure(GDataErrorCode code) {
+void DownloadFileRequestBase::RunCallbackOnPrematureFailure(
+    GDataErrorCode code) {
   download_action_callback_.Run(code, base::FilePath());
 }
 
diff --git a/chrome/browser/google_apis/base_requests.h b/chrome/browser/google_apis/base_requests.h
index 7236b8a..47c98b1 100644
--- a/chrome/browser/google_apis/base_requests.h
+++ b/chrome/browser/google_apis/base_requests.h
@@ -438,8 +438,8 @@ typedef base::Callback<void(GDataErrorCode error,
                             const base::FilePath& temp_file)>
     DownloadActionCallback;
 
-// This class performs the request for downloading of a given document/file.
-class DownloadFileRequest : public UrlFetchRequestBase {
+// This is a base class for performing the request for downloading a file.
+class DownloadFileRequestBase : public UrlFetchRequestBase {
  public:
   // download_action_callback:
   //   This callback is called when the download is complete. Must not be null.
@@ -458,13 +458,14 @@ class DownloadFileRequest : public UrlFetchRequestBase {
   // output_file_path:
   //   Specifies the file path to save the downloaded file.
   //
-  DownloadFileRequest(RequestSender* sender,
-                      const DownloadActionCallback& download_action_callback,
-                      const GetContentCallback& get_content_callback,
-                      const ProgressCallback& progress_callback,
-                      const GURL& download_url,
-                      const base::FilePath& output_file_path);
-  virtual ~DownloadFileRequest();
+  DownloadFileRequestBase(
+      RequestSender* sender,
+      const DownloadActionCallback& download_action_callback,
+      const GetContentCallback& get_content_callback,
+      const ProgressCallback& progress_callback,
+      const GURL& download_url,
+      const base::FilePath& output_file_path);
+  virtual ~DownloadFileRequestBase();
 
  protected:
   // UrlFetchRequestBase overrides.
@@ -488,7 +489,7 @@ class DownloadFileRequest : public UrlFetchRequestBase {
   const GURL download_url_;
   const base::FilePath output_file_path_;
 
-  DISALLOW_COPY_AND_ASSIGN(DownloadFileRequest);
+  DISALLOW_COPY_AND_ASSIGN(DownloadFileRequestBase);
 };
 
 }  // namespace google_apis
diff --git a/chrome/browser/google_apis/base_requests_server_unittest.cc b/chrome/browser/google_apis/base_requests_server_unittest.cc
index 6a298b3..74dad4c 100644
--- a/chrome/browser/google_apis/base_requests_server_unittest.cc
+++ b/chrome/browser/google_apis/base_requests_server_unittest.cc
@@ -86,7 +86,7 @@ TEST_F(BaseRequestsServerTest, DownloadFileRequest_ValidFile) {
   base::FilePath temp_file;
   {
     base::RunLoop run_loop;
-    DownloadFileRequest* request = new DownloadFileRequest(
+    DownloadFileRequestBase* request = new DownloadFileRequestBase(
         request_sender_.get(),
         test_util::CreateQuitCallback(
             &run_loop,
@@ -120,7 +120,7 @@ TEST_F(BaseRequestsServerTest, DownloadFileRequest_NonExistentFile) {
   base::FilePath temp_file;
   {
     base::RunLoop run_loop;
-    DownloadFileRequest* request = new DownloadFileRequest(
+    DownloadFileRequestBase* request = new DownloadFileRequestBase(
         request_sender_.get(),
         test_util::CreateQuitCallback(
             &run_loop,
diff --git a/chrome/browser/google_apis/drive_api_requests.cc b/chrome/browser/google_apis/drive_api_requests.cc
index 2e495bd..04e163e 100644
--- a/chrome/browser/google_apis/drive_api_requests.cc
+++ b/chrome/browser/google_apis/drive_api_requests.cc
@@ -631,5 +631,27 @@ void GetUploadStatusRequest::OnRangeRequestComplete(
   ParseFileResourceWithUploadRangeAndRun(callback_, response, value.Pass());
 }
 
+//========================== DownloadFileRequest ==========================
+
+DownloadFileRequest::DownloadFileRequest(
+    RequestSender* sender,
+    const DriveApiUrlGenerator& url_generator,
+    const std::string& resource_id,
+    const base::FilePath& output_file_path,
+    const DownloadActionCallback& download_action_callback,
+    const GetContentCallback& get_content_callback,
+    const ProgressCallback& progress_callback)
+    : DownloadFileRequestBase(
+          sender,
+          download_action_callback,
+          get_content_callback,
+          progress_callback,
+          url_generator.GenerateDownloadFileUrl(resource_id),
+          output_file_path) {
+}
+
+DownloadFileRequest::~DownloadFileRequest() {
+}
+
 }  // namespace drive
 }  // namespace google_apis
diff --git a/chrome/browser/google_apis/drive_api_requests.h b/chrome/browser/google_apis/drive_api_requests.h
index 1afa8c1..0fa5a79 100644
--- a/chrome/browser/google_apis/drive_api_requests.h
+++ b/chrome/browser/google_apis/drive_api_requests.h
@@ -528,6 +528,23 @@ class GetUploadStatusRequest : public GetUploadStatusRequestBase {
   DISALLOW_COPY_AND_ASSIGN(GetUploadStatusRequest);
 };
 
+//========================== DownloadFileRequest ==========================
+
+// This class performs the request for downloading of a specified file.
+class DownloadFileRequest : public DownloadFileRequestBase {
+ public:
+  // See also DownloadFileRequestBase's comment for parameters meaning.
+  DownloadFileRequest(RequestSender* sender,
+                      const DriveApiUrlGenerator& url_generator,
+                      const std::string& resource_id,
+                      const base::FilePath& output_file_path,
+                      const DownloadActionCallback& download_action_callback,
+                      const GetContentCallback& get_content_callback,
+                      const ProgressCallback& progress_callback);
+  virtual ~DownloadFileRequest();
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadFileRequest);
+};
 
 }  // namespace drive
 }  // namespace google_apis
diff --git a/chrome/browser/google_apis/drive_api_requests_unittest.cc b/chrome/browser/google_apis/drive_api_requests_unittest.cc
index 7fed20d..74f93cb 100644
--- a/chrome/browser/google_apis/drive_api_requests_unittest.cc
+++ b/chrome/browser/google_apis/drive_api_requests_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/bind.h"
+#include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/run_loop.h"
@@ -41,6 +42,7 @@ const char kTestChildrenResponse[] =
 
 const char kTestUploadExistingFilePath[] = "/upload/existingfile/path";
 const char kTestUploadNewFilePath[] = "/upload/newfile/path";
+const char kTestDownloadPathPrefix[] = "/download/";
 
 }  // namespace
 
@@ -84,10 +86,13 @@ class DriveApiRequestsTest : public testing::Test {
     test_server_.RegisterRequestHandler(
         base::Bind(&DriveApiRequestsTest::HandleContentResponse,
                    base::Unretained(this)));
+    test_server_.RegisterRequestHandler(
+        base::Bind(&DriveApiRequestsTest::HandleDownloadRequest,
+                   base::Unretained(this)));
 
     GURL test_base_url = test_util::GetBaseUrlForTesting(test_server_.port());
     url_generator_.reset(new DriveApiUrlGenerator(
-        test_base_url, test_base_url.Resolve("download/")));
+        test_base_url, test_base_url.Resolve(kTestDownloadPathPrefix)));
 
     // Reset the server's expected behavior just in case.
     ResetExpectedResponse();
@@ -305,6 +310,28 @@ class DriveApiRequestsTest : public testing::Test {
     return response.PassAs<net::test_server::HttpResponse>();
   }
 
+  // Handles a request for downloading a file.
+  scoped_ptr<net::test_server::HttpResponse> HandleDownloadRequest(
+      const net::test_server::HttpRequest& request) {
+    http_request_ = request;
+
+    const GURL absolute_url = test_server_.GetURL(request.relative_url);
+    std::string id;
+    if (!test_util::RemovePrefix(absolute_url.path(),
+                                 kTestDownloadPathPrefix,
+                                 &id)) {
+      return scoped_ptr<net::test_server::HttpResponse>();
+    }
+
+    // For testing, returns a text with |id| repeated 3 times.
+    scoped_ptr<net::test_server::BasicHttpResponse> response(
+        new net::test_server::BasicHttpResponse);
+    response->set_code(net::HTTP_OK);
+    response->set_content(id + id + id);
+    response->set_content_type("text/plain");
+    return response.PassAs<net::test_server::HttpResponse>();
+  }
+
   // These are for the current upload file status.
   int64 received_bytes_;
   int64 content_length_;
@@ -1341,4 +1368,40 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETagConflicting) {
   EXPECT_TRUE(http_request_.content.empty());
 }
 
+TEST_F(DriveApiRequestsTest, DownloadFileRequest) {
+  const base::FilePath kDownloadedFilePath =
+      temp_dir_.path().AppendASCII("cache_file");
+  const std::string kTestId("dummyId");
+
+  GDataErrorCode result_code = GDATA_OTHER_ERROR;
+  base::FilePath temp_file;
+  {
+    base::RunLoop run_loop;
+    drive::DownloadFileRequest* request = new drive::DownloadFileRequest(
+        request_sender_.get(),
+        *url_generator_,
+        kTestId,
+        kDownloadedFilePath,
+        test_util::CreateQuitCallback(
+            &run_loop,
+            test_util::CreateCopyResultCallback(&result_code, &temp_file)),
+        GetContentCallback(),
+        ProgressCallback());
+    request_sender_->StartRequestWithRetry(request);
+    run_loop.Run();
+  }
+
+  std::string contents;
+  file_util::ReadFileToString(temp_file, &contents);
+  base::Delete(temp_file, false);
+
+  EXPECT_EQ(HTTP_SUCCESS, result_code);
+  EXPECT_EQ(net::test_server::METHOD_GET, http_request_.method);
+  EXPECT_EQ(kTestDownloadPathPrefix + kTestId, http_request_.relative_url);
+  EXPECT_EQ(kDownloadedFilePath, temp_file);
+
+  const std::string expected_contents = kTestId + kTestId + kTestId;
+  EXPECT_EQ(expected_contents, contents);
+}
+
 }  // namespace google_apis
diff --git a/chrome/browser/google_apis/gdata_wapi_requests.cc b/chrome/browser/google_apis/gdata_wapi_requests.cc
index 4f562cf..eb38afa 100644
--- a/chrome/browser/google_apis/gdata_wapi_requests.cc
+++ b/chrome/browser/google_apis/gdata_wapi_requests.cc
@@ -711,4 +711,27 @@ void GetUploadStatusRequest::OnRangeRequestComplete(
   callback_.Run(response, ParseResourceEntry(value.Pass()));
 }
 
+
+//========================== DownloadFileRequest ==========================
+
+DownloadFileRequest::DownloadFileRequest(
+    RequestSender* sender,
+    const GDataWapiUrlGenerator& url_generator,
+    const DownloadActionCallback& download_action_callback,
+    const GetContentCallback& get_content_callback,
+    const ProgressCallback& progress_callback,
+    const std::string& resource_id,
+    const base::FilePath& output_file_path)
+    : DownloadFileRequestBase(
+          sender,
+          download_action_callback,
+          get_content_callback,
+          progress_callback,
+          url_generator.GenerateDownloadFileUrl(resource_id),
+          output_file_path) {
+}
+
+DownloadFileRequest::~DownloadFileRequest() {
+}
+
 }  // namespace google_apis
diff --git a/chrome/browser/google_apis/gdata_wapi_requests.h b/chrome/browser/google_apis/gdata_wapi_requests.h
index d367761..ba6f774 100644
--- a/chrome/browser/google_apis/gdata_wapi_requests.h
+++ b/chrome/browser/google_apis/gdata_wapi_requests.h
@@ -489,6 +489,25 @@ class GetUploadStatusRequest : public GetUploadStatusRequestBase {
   DISALLOW_COPY_AND_ASSIGN(GetUploadStatusRequest);
 };
 
+
+//========================== DownloadFileRequest ==========================
+
+// This class performs the request for downloading of a specified file.
+class DownloadFileRequest : public DownloadFileRequestBase {
+ public:
+  // See also DownloadFileRequestBase's comment for parameters meaning.
+  DownloadFileRequest(RequestSender* sender,
+                      const GDataWapiUrlGenerator& url_generator,
+                      const DownloadActionCallback& download_action_callback,
+                      const GetContentCallback& get_content_callback,
+                      const ProgressCallback& progress_callback,
+                      const std::string& resource_id,
+                      const base::FilePath& output_file_path);
+  virtual ~DownloadFileRequest();
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadFileRequest);
+};
+
 }  // namespace google_apis
 
 #endif  // CHROME_BROWSER_GOOGLE_APIS_GDATA_WAPI_REQUESTS_H_
diff --git a/chrome/browser/google_apis/gdata_wapi_requests_unittest.cc b/chrome/browser/google_apis/gdata_wapi_requests_unittest.cc
index 9772754..cbdd2fa 100644
--- a/chrome/browser/google_apis/gdata_wapi_requests_unittest.cc
+++ b/chrome/browser/google_apis/gdata_wapi_requests_unittest.cc
@@ -6,6 +6,7 @@
 #include <map>
 
 #include "base/bind.h"
+#include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/json/json_reader.h"
@@ -37,6 +38,7 @@ namespace {
 const char kTestGDataAuthToken[] = "testtoken";
 const char kTestUserAgent[] = "test-user-agent";
 const char kTestETag[] = "test_etag";
+const char kTestDownloadPathPrefix[] = "/download/";
 
 class GDataWapiRequestsTest : public testing::Test {
  public:
@@ -79,10 +81,13 @@ class GDataWapiRequestsTest : public testing::Test {
     test_server_.RegisterRequestHandler(
         base::Bind(&GDataWapiRequestsTest::HandleUploadRequest,
                    base::Unretained(this)));
+    test_server_.RegisterRequestHandler(
+        base::Bind(&GDataWapiRequestsTest::HandleDownloadRequest,
+                   base::Unretained(this)));
 
     GURL test_base_url = test_util::GetBaseUrlForTesting(test_server_.port());
     url_generator_.reset(new GDataWapiUrlGenerator(
-        test_base_url, test_base_url.Resolve("download/")));
+        test_base_url, test_base_url.Resolve(kTestDownloadPathPrefix)));
 
     received_bytes_ = 0;
     content_length_ = 0;
@@ -312,6 +317,28 @@ class GDataWapiRequestsTest : public testing::Test {
     return response.PassAs<net::test_server::HttpResponse>();
   }
 
+  // Handles a request for downloading a file.
+  scoped_ptr<net::test_server::HttpResponse> HandleDownloadRequest(
+      const net::test_server::HttpRequest& request) {
+    http_request_ = request;
+
+    const GURL absolute_url = test_server_.GetURL(request.relative_url);
+    std::string id;
+    if (!test_util::RemovePrefix(absolute_url.path(),
+                                 kTestDownloadPathPrefix,
+                                 &id)) {
+      return scoped_ptr<net::test_server::HttpResponse>();
+    }
+
+    // For testing, returns a text with |id| repeated 3 times.
+    scoped_ptr<net::test_server::BasicHttpResponse> response(
+        new net::test_server::BasicHttpResponse);
+    response->set_code(net::HTTP_OK);
+    response->set_content(id + id + id);
+    response->set_content_type("text/plain");
+    return response.PassAs<net::test_server::HttpResponse>();
+  }
+
   content::TestBrowserThreadBundle thread_bundle_;
   net::test_server::EmbeddedTestServer test_server_;
   scoped_ptr<TestingProfile> profile_;
@@ -1542,4 +1569,41 @@ TEST_F(GDataWapiRequestsTest, UploadExistingFileWithETagConflict) {
   EXPECT_EQ(kWrongETag, http_request_.headers["If-Match"]);
 }
 
+TEST_F(GDataWapiRequestsTest, DownloadFileRequest) {
+  const base::FilePath kDownloadedFilePath =
+      temp_dir_.path().AppendASCII("cache_file");
+  const std::string kTestIdWithTypeLabel("file:dummyId");
+  const std::string kTestId("dummyId");
+
+  GDataErrorCode result_code = GDATA_OTHER_ERROR;
+  base::FilePath temp_file;
+  {
+    base::RunLoop run_loop;
+    DownloadFileRequest* request = new DownloadFileRequest(
+        request_sender_.get(),
+        *url_generator_,
+        test_util::CreateQuitCallback(
+            &run_loop,
+            test_util::CreateCopyResultCallback(&result_code, &temp_file)),
+        GetContentCallback(),
+        ProgressCallback(),
+        kTestIdWithTypeLabel,
+        kDownloadedFilePath);
+    request_sender_->StartRequestWithRetry(request);
+    run_loop.Run();
+  }
+
+  std::string contents;
+  file_util::ReadFileToString(temp_file, &contents);
+  base::Delete(temp_file, false);
+
+  EXPECT_EQ(HTTP_SUCCESS, result_code);
+  EXPECT_EQ(net::test_server::METHOD_GET, http_request_.method);
+  EXPECT_EQ(kTestDownloadPathPrefix + kTestId, http_request_.relative_url);
+  EXPECT_EQ(kDownloadedFilePath, temp_file);
+
+  const std::string expected_contents = kTestId + kTestId + kTestId;
+  EXPECT_EQ(expected_contents, contents);
+}
+
 }  // namespace google_apis