Drive: Let DriveUploader use batch request API.

chrome/browser/drive/drive_uploader.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/drive/drive_uploader.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 26 matching lines @@
 // bytes (except the request for uploading the last chunk of data).
 // The value must be a multiple of 512KB according to the spec of GData WAPI and
 // Drive API v2. It is set to a smaller value than 2^31 for working around
 // server side error (crbug.com/264089).
 const int64 kUploadChunkSize = (1LL << 30);  // 1GB
 // Maximum file size to be uploaded by multipart requests. The file that is
 // larger than the size is processed by resumable upload.
 const int64 kMaxMultipartUploadSize = (1LL << 20);  // 1MB
 }  // namespace
 
+// Refcounted helper class to manage batch request. When the instance is
+// destroyed, it commits owned batch request.

[kinaba, 2015/05/08 06:58:12]

Please add some comment on why refcounted control is necessary.

[hirono, 2015/05/11 05:22:05]

Done.

+class DriveUploader::RefCountedBatchRequest
+    : public base::RefCounted<RefCountedBatchRequest> {
+ public:
+  RefCountedBatchRequest(
+      scoped_ptr<BatchRequestConfiguratorInterface> configurator)
+      : configurator_(configurator.Pass()) {}
+
+  // Gets pointer of BatchRequestConfiguratorInterface owned by the instance.
+  BatchRequestConfiguratorInterface* get() const { return configurator_.get(); }
+
+ private:
+  friend class base::RefCounted<RefCountedBatchRequest>;
+  ~RefCountedBatchRequest() { configurator_->Commit(); }
+  scoped_ptr<BatchRequestConfiguratorInterface> configurator_;
+};
+
 // Structure containing current upload information of file, passed between
 // DriveServiceInterface methods and callbacks.
 struct DriveUploader::UploadFileInfo {
   UploadFileInfo(const base::FilePath& local_path,
                  const std::string& content_type,
                  const UploadCompletionCallback& callback,
                  const ProgressCallback& progress_callback)
       : file_path(local_path),
         content_type(content_type),
         completion_callback(callback),
@@ skipping 91 matching lines @@
   DCHECK(!local_file_path.empty());
   DCHECK(!title.empty());
   DCHECK(!content_type.empty());
   DCHECK(!callback.is_null());
 
   return StartUploadFile(
       scoped_ptr<UploadFileInfo>(new UploadFileInfo(
           local_file_path, content_type, callback, progress_callback)),
       base::Bind(&DriveUploader::CallUploadServiceAPINewFile,
                  weak_ptr_factory_.GetWeakPtr(), parent_resource_id, title,
-                 options));
+                 options, current_batch_request_));
+}
+
+void DriveUploader::StartBatchProcessing() {

[kinaba, 2015/05/08 06:58:12]

DCHECK(current_batch_request_ == null)?

[hirono, 2015/05/11 05:22:06]

Done.

+  current_batch_request_ =
+      new RefCountedBatchRequest(drive_service_->StartBatchRequest().Pass());
+}
+
+void DriveUploader::StopBatchProcessing() {
+  current_batch_request_ = nullptr;
 }
 
 CancelCallback DriveUploader::UploadExistingFile(
     const std::string& resource_id,
     const base::FilePath& local_file_path,
     const std::string& content_type,
     const UploadExistingFileOptions& options,
     const UploadCompletionCallback& callback,
     const ProgressCallback& progress_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!resource_id.empty());
   DCHECK(!local_file_path.empty());
   DCHECK(!content_type.empty());
   DCHECK(!callback.is_null());
 
   return StartUploadFile(
       scoped_ptr<UploadFileInfo>(new UploadFileInfo(
           local_file_path, content_type, callback, progress_callback)),
       base::Bind(&DriveUploader::CallUploadServiceAPIExistingFile,
-                 weak_ptr_factory_.GetWeakPtr(), resource_id, options));
+                 weak_ptr_factory_.GetWeakPtr(), resource_id, options,
+                 current_batch_request_));
 }
 
 CancelCallback DriveUploader::ResumeUploadFile(
     const GURL& upload_location,
     const base::FilePath& local_file_path,
     const std::string& content_type,
     const UploadCompletionCallback& callback,
     const ProgressCallback& progress_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!local_file_path.empty());
   DCHECK(!content_type.empty());
   DCHECK(!callback.is_null());
 
   scoped_ptr<UploadFileInfo> upload_file_info(new UploadFileInfo(
-      local_file_path, content_type,
-      callback, progress_callback));
+      local_file_path, content_type, callback, progress_callback));
   upload_file_info->upload_location = upload_location;
 
   return StartUploadFile(
       upload_file_info.Pass(),
       base::Bind(&DriveUploader::StartGetUploadStatus,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 CancelCallback DriveUploader::StartUploadFile(
     scoped_ptr<UploadFileInfo> upload_file_info,
@@ skipping 31 matching lines @@
     UploadFailed(upload_file_info.Pass(), DRIVE_CANCELLED);
     return;
   }
   start_initiate_upload_callback.Run(upload_file_info.Pass());
 }
 
 void DriveUploader::CallUploadServiceAPINewFile(
     const std::string& parent_resource_id,
     const std::string& title,
     const UploadNewFileOptions& options,
+    const scoped_refptr<RefCountedBatchRequest>& batch_request,
     scoped_ptr<UploadFileInfo> upload_file_info) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   UploadFileInfo* const info_ptr = upload_file_info.get();
   if (info_ptr->content_length <= kMaxMultipartUploadSize) {
-    info_ptr->cancel_callback = drive_service_->MultipartUploadNewFile(
+    DriveServiceBatchOperationsInterface* const service =
+        batch_request.get()
+            ? static_cast<DriveServiceBatchOperationsInterface*>(
+                  batch_request.get()->get())
+            : static_cast<DriveServiceBatchOperationsInterface*>(
+                  drive_service_);

[kinaba, 2015/05/08 06:58:12]

I can understand the preference to constness, but the current code
looks too much verbose to me. How about giving it up and writing in a
simple if-branch with comment, like:

DriveServiceBatchOperationsInterface* service = batch_request.get();
// If this is not a batched request, use the API service directly.
if (!service)
  service = drive_service_;

?

[hirono, 2015/05/11 05:22:06]

Done.

+    info_ptr->cancel_callback = service->MultipartUploadNewFile(
         info_ptr->content_type, info_ptr->content_length, parent_resource_id,
         title, info_ptr->file_path, options,
         base::Bind(&DriveUploader::OnMultipartUploadComplete,
                    weak_ptr_factory_.GetWeakPtr(),
                    base::Passed(&upload_file_info)),
         info_ptr->progress_callback);
   } else {
     info_ptr->cancel_callback = drive_service_->InitiateUploadNewFile(
         info_ptr->content_type, info_ptr->content_length, parent_resource_id,
         title, options, base::Bind(&DriveUploader::OnUploadLocationReceived,
                                    weak_ptr_factory_.GetWeakPtr(),
                                    base::Passed(&upload_file_info)));
   }
 }
 
 void DriveUploader::CallUploadServiceAPIExistingFile(
     const std::string& resource_id,
     const UploadExistingFileOptions& options,
+    const scoped_refptr<RefCountedBatchRequest>& batch_request,
     scoped_ptr<UploadFileInfo> upload_file_info) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   UploadFileInfo* const info_ptr = upload_file_info.get();
   if (info_ptr->content_length <= kMaxMultipartUploadSize) {
-    info_ptr->cancel_callback = drive_service_->MultipartUploadExistingFile(
+    DriveServiceBatchOperationsInterface* const service =
+        batch_request.get()
+            ? static_cast<DriveServiceBatchOperationsInterface*>(
+                  batch_request.get()->get())
+            : static_cast<DriveServiceBatchOperationsInterface*>(
+                  drive_service_);

[kinaba, 2015/05/08 06:58:12]

ditto.

[hirono, 2015/05/11 05:22:06]

Done.

+    info_ptr->cancel_callback = service->MultipartUploadExistingFile(
         info_ptr->content_type, info_ptr->content_length, resource_id,
         info_ptr->file_path, options,
         base::Bind(&DriveUploader::OnMultipartUploadComplete,
                    weak_ptr_factory_.GetWeakPtr(),
                    base::Passed(&upload_file_info)),
         info_ptr->progress_callback);
   } else {
     info_ptr->cancel_callback = drive_service_->InitiateUploadExistingFile(
         info_ptr->content_type, info_ptr->content_length, resource_id, options,
         base::Bind(&DriveUploader::OnUploadLocationReceived,
@@ skipping 167 matching lines @@
   } else {
     DVLOG(1) << "Upload failed " << upload_file_info->DebugString();
     if (error == HTTP_PRECONDITION)
       error = HTTP_CONFLICT;  // ETag mismatch.
     upload_file_info->completion_callback.Run(
         error, upload_file_info->upload_location, scoped_ptr<FileResource>());
   }
 }
 
 }  // namespace drive