drive: Discard upload location when GetUploadStatus fails

GetUploadStatus for an unusable URL always results in an error.
Upload location should be discarded to initiate new upload in such cases.

BUG=267452
R=kinaba@chromium.org

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

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

3 files changed, 63 insertions, 12 deletions

diff --git a/chrome/browser/drive/drive_uploader.cc b/chrome/browser/drive/drive_uploader.cc
index 493a763..30808a5 100644
--- a/chrome/browser/drive/drive_uploader.cc
+++ b/chrome/browser/drive/drive_uploader.cc
@@ -55,6 +55,7 @@ struct DriveUploader::UploadFileInfo {
         completion_callback(callback),
         progress_callback(progress_callback),
         content_length(0),
+        next_start_position(-1),
         power_save_blocker(content::PowerSaveBlocker::Create(
             content::PowerSaveBlocker::kPowerSaveBlockPreventAppSuspension,
             "Upload in progress")),
@@ -97,6 +98,8 @@ struct DriveUploader::UploadFileInfo {
   // Header content-Length.
   int64 content_length;
 
+  int64 next_start_position;
+
   // Blocks system suspend while upload is in progress.
   scoped_ptr<content::PowerSaveBlocker> power_save_blocker;
 
@@ -295,7 +298,8 @@ void DriveUploader::OnUploadLocationReceived(
   }
 
   upload_file_info->upload_location = upload_location;
-  UploadNextChunk(upload_file_info.Pass(), 0);  // start_position
+  upload_file_info->next_start_position = 0;
+  UploadNextChunk(upload_file_info.Pass());
 }
 
 void DriveUploader::StartGetUploadStatus(
@@ -313,12 +317,12 @@ void DriveUploader::StartGetUploadStatus(
 }
 
 void DriveUploader::UploadNextChunk(
-    scoped_ptr<UploadFileInfo> upload_file_info,
-    int64 start_position) {
+    scoped_ptr<UploadFileInfo> upload_file_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(upload_file_info);
-  DCHECK_GE(start_position, 0);
-  DCHECK_LE(start_position, upload_file_info->content_length);
+  DCHECK_GE(upload_file_info->next_start_position, 0);
+  DCHECK_LE(upload_file_info->next_start_position,
+            upload_file_info->content_length);
 
   if (upload_file_info->cancelled) {
     UploadFailed(upload_file_info.Pass(), GDATA_CANCELLED);
@@ -326,13 +330,14 @@ void DriveUploader::UploadNextChunk(
   }
 
   // Limit the size of data uploaded per each request by kUploadChunkSize.
-  const int64 end_position = std::min(upload_file_info->content_length,
-                                      start_position + kUploadChunkSize);
+  const int64 end_position = std::min(
+      upload_file_info->content_length,
+      upload_file_info->next_start_position + kUploadChunkSize);
 
   UploadFileInfo* info_ptr = upload_file_info.get();
   info_ptr->cancel_callback = drive_service_->ResumeUpload(
       info_ptr->upload_location,
-      start_position,
+      info_ptr->next_start_position,
       end_position,
       info_ptr->content_length,
       info_ptr->content_type,
@@ -343,7 +348,7 @@ void DriveUploader::UploadNextChunk(
       base::Bind(&DriveUploader::OnUploadProgress,
                  weak_ptr_factory_.GetWeakPtr(),
                  info_ptr->progress_callback,
-                 start_position,
+                 info_ptr->next_start_position,
                  info_ptr->content_length));
 }
 
@@ -397,7 +402,8 @@ void DriveUploader::OnUploadRangeResponseReceived(
            << "-" << response.end_position_received
            << " for [" << upload_file_info->file_path.value() << "]";
 
-  UploadNextChunk(upload_file_info.Pass(), response.end_position_received);
+  upload_file_info->next_start_position = response.end_position_received;
+  UploadNextChunk(upload_file_info.Pass());
 }
 
 void DriveUploader::OnUploadProgress(const ProgressCallback& callback,
@@ -415,6 +421,12 @@ void DriveUploader::UploadFailed(scoped_ptr<UploadFileInfo> upload_file_info,
 
   LOG(ERROR) << "Upload failed " << upload_file_info->DebugString();
 
+  if (upload_file_info->next_start_position < 0) {
+    // Discard the upload location because no request could succeed with it.
+    // Maybe it's obsolete.
+    upload_file_info->upload_location = GURL();
+  }
+
   upload_file_info->completion_callback.Run(
       error, upload_file_info->upload_location, scoped_ptr<ResourceEntry>());
 }
diff --git a/chrome/browser/drive/drive_uploader.h b/chrome/browser/drive/drive_uploader.h
index cb8bc80..cdbb26b 100644
--- a/chrome/browser/drive/drive_uploader.h
+++ b/chrome/browser/drive/drive_uploader.h
@@ -172,8 +172,7 @@ class DriveUploader : public DriveUploaderInterface {
   void StartGetUploadStatus(scoped_ptr<UploadFileInfo> upload_file_info);
 
   // Uploads the next chunk of data from the file.
-  void UploadNextChunk(scoped_ptr<UploadFileInfo> upload_file_info,
-                       int64 start_position);
+  void UploadNextChunk(scoped_ptr<UploadFileInfo> upload_file_info);
 
   // DriveService callback for ResumeUpload.
   void OnUploadRangeResponseReceived(
diff --git a/chrome/browser/drive/drive_uploader_unittest.cc b/chrome/browser/drive/drive_uploader_unittest.cc
index 83afc28..0d8d636 100644
--- a/chrome/browser/drive/drive_uploader_unittest.cc
+++ b/chrome/browser/drive/drive_uploader_unittest.cc
@@ -286,6 +286,21 @@ class MockDriveServiceNoConnectionAtResume : public DummyDriveService {
   }
 };
 
+// Mock DriveService that returns a failure at GetUploadStatus().
+class MockDriveServiceNoConnectionAtGetUploadStatus : public DummyDriveService {
+  // Returns error.
+  virtual CancelCallback GetUploadStatus(
+      const GURL& upload_url,
+      int64 content_length,
+      const UploadRangeCallback& callback) OVERRIDE {
+    base::MessageLoop::current()->PostTask(FROM_HERE,
+        base::Bind(callback,
+                   UploadRangeResponse(GDATA_NO_CONNECTION, -1, -1),
+                   base::Passed(scoped_ptr<ResourceEntry>())));
+    return CancelCallback();
+  }
+};
+
 class DriveUploaderTest : public testing::Test {
  public:
   virtual void SetUp() OVERRIDE {
@@ -477,6 +492,31 @@ TEST_F(DriveUploaderTest, ResumeUploadFail) {
   EXPECT_EQ(GURL(kTestUploadExistingFileURL), upload_location);
 }
 
+TEST_F(DriveUploaderTest, GetUploadStatusFail) {
+  base::FilePath local_path;
+  std::string data;
+  ASSERT_TRUE(test_util::CreateFileOfSpecifiedSize(
+      temp_dir_.path(), 512 * 1024, &local_path, &data));
+
+  GDataErrorCode error = HTTP_SUCCESS;
+  GURL upload_location;
+  scoped_ptr<ResourceEntry> resource_entry;
+
+  MockDriveServiceNoConnectionAtGetUploadStatus mock_service;
+  DriveUploader uploader(&mock_service,
+                         base::MessageLoopProxy::current().get());
+  uploader.ResumeUploadFile(GURL(kTestUploadExistingFileURL),
+                            local_path,
+                            kTestMimeType,
+                            test_util::CreateCopyResultCallback(
+                                &error, &upload_location, &resource_entry),
+                            google_apis::ProgressCallback());
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(GDATA_NO_CONNECTION, error);
+  EXPECT_TRUE(upload_location.is_empty());
+}
+
 TEST_F(DriveUploaderTest, NonExistingSourceFile) {
   GDataErrorCode error = GDATA_OTHER_ERROR;
   GURL upload_location;