Drive: Let DriveUploader use batch request API.

chrome/browser/drive/drive_uploader.cc

Index: chrome/browser/drive/drive_uploader.cc
diff --git a/chrome/browser/drive/drive_uploader.cc b/chrome/browser/drive/drive_uploader.cc
index 76709fdb6608632181e40f53271f5cbd35f5ce43..8581757f4cd0ccb49c2feb90bec07046a63c9660 100644
--- a/chrome/browser/drive/drive_uploader.cc
+++ b/chrome/browser/drive/drive_uploader.cc
@@ -44,6 +44,24 @@ const int64 kUploadChunkSize = (1LL << 30);  // 1GB
 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 {
@@ -155,7 +173,16 @@ CancelCallback DriveUploader::UploadNewFile(
           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(
@@ -175,7 +202,8 @@ CancelCallback DriveUploader::UploadExistingFile(
       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(
@@ -190,8 +218,7 @@ CancelCallback DriveUploader::ResumeUploadFile(
   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(
@@ -243,12 +270,19 @@ 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,
@@ -267,12 +301,19 @@ void DriveUploader::CallUploadServiceAPINewFile(
 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,