Add Move operation to the the scheduler

chrome/browser/chromeos/drive/drive_scheduler.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/chromeos/drive/drive_scheduler.h"
 
 #include <math.h>
 
 #include "base/message_loop.h"
 #include "base/rand_util.h"
@@ skipping 16 matching lines @@
 DriveScheduler::JobInfo::JobInfo(JobType in_job_type, FilePath in_file_path)
     : job_type(in_job_type),
       job_id(-1),
       completed_bytes(0),
       total_bytes(0),
       file_path(in_file_path),
       state(STATE_NONE) {
 }
 
 DriveScheduler::QueueEntry::QueueEntry(JobType in_job_type,
-                                       FilePath in_file_path)
-    : job_info(TYPE_REMOVE, in_file_path) {
+                                       FilePath in_file_path,
+                                       FileOperationCallback in_callback)
+    : job_info(in_job_type, in_file_path),
+      callback(in_callback),
+      is_recursive(false) {
 }
 
 DriveScheduler::QueueEntry::~QueueEntry() {
 }
 
-DriveScheduler::RemoveJobPrivate::RemoveJobPrivate(
-    bool in_is_recursive,
-    FileOperationCallback in_callback)
-    : is_recursive(in_is_recursive),
-      callback(in_callback) {
-}
-
-DriveScheduler::RemoveJobPrivate::~RemoveJobPrivate() {
-}
-
 DriveScheduler::DriveScheduler(Profile* profile,
                                file_system::DriveOperations* drive_operations)
     : job_loop_is_running_(false),
       next_job_id_(0),
       throttle_count_(0),
       disable_throttling_(false),
       drive_operations_(drive_operations),
       profile_(profile),
       weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
 DriveScheduler::~DriveScheduler() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   net::NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
 }
 
 void DriveScheduler::Initialize() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   net::NetworkChangeNotifier::AddConnectionTypeObserver(this);
 }
 
+void DriveScheduler::Move(const FilePath& src_file_path,
+                          const FilePath& dest_file_path,
+                          const FileOperationCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  QueueEntry* new_job = new QueueEntry(TYPE_MOVE, src_file_path, callback);

[satorux1, 2012/10/16 04:34:01]

This made me think how the object is deleted. This will be added to job info map
and be deleted as the value is linked_ptr, right? Please add some comment about
lifetime of the object.

Alternatively, you might want to use scoped_ptr<> here, and pass it to
QueueJob() using Pass(). This will make ownership transfer clearer.

[Zachary Kuznia, 2012/10/16 04:59:10]

Done.

+  new_job->dest_file_path = dest_file_path;
+
+  QueueJob(new_job);
+
+  StartJobLoop();
+}
 
 void DriveScheduler::Remove(const FilePath& file_path,
                             bool is_recursive,
                             const FileOperationCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  QueueEntry* new_job = new QueueEntry(TYPE_REMOVE, file_path);
-  new_job->remove_private.reset(new RemoveJobPrivate(is_recursive, callback));
+  QueueEntry* new_job = new QueueEntry(TYPE_REMOVE, file_path, callback);

[satorux1, 2012/10/16 04:34:01]

ditto.

[Zachary Kuznia, 2012/10/16 04:59:10]

Done.

+  new_job->is_recursive = is_recursive;
 
   QueueJob(new_job);
 
   StartJobLoop();
 }
 
 int DriveScheduler::QueueJob(QueueEntry* job) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   int job_id = next_job_id_;
@@ skipping 29 matching lines @@
   int job_id = queue_.front();
   queue_.pop_front();
 
   JobMap::iterator job_iter = job_info_.find(job_id);
   DCHECK(job_iter != job_info_.end());
 
   JobInfo& job_info = job_iter->second->job_info;
   job_info.state = STATE_RUNNING;
 
   switch (job_info.job_type) {
-    case TYPE_REMOVE: {
-      DCHECK(job_iter->second->remove_private.get());
-
-      drive_operations_->Remove(
+    case TYPE_MOVE: {
+      drive_operations_->Move(
           job_info.file_path,
-          job_iter->second->remove_private->is_recursive,
-          base::Bind(&DriveScheduler::OnRemoveDone,
+          job_iter->second->dest_file_path,
+          base::Bind(&DriveScheduler::OnJobDone,
                      weak_ptr_factory_.GetWeakPtr(),
                      job_id));
     }
+    break;
+
+    case TYPE_REMOVE: {
+      drive_operations_->Remove(
+          job_info.file_path,
+          job_iter->second->is_recursive,
+          base::Bind(&DriveScheduler::OnJobDone,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     job_id));
+    }
     break;

[satorux1, 2012/10/16 04:34:01]

Please add:

default:
  NOTREACHED() << "Unknown job type: " << job_info.job_type

[Zachary Kuznia, 2012/10/16 04:59:10]

By leaving out a default case, the compiler will complain if a type is
unhandled.

[satorux1, 2012/10/16 05:10:37]

I see. Then please add some comment about it.

[Zachary Kuznia, 2012/11/07 05:46:17]

Done.

   }
 
 }
 
 bool DriveScheduler::ShouldStopJobLoop() {
   // Should stop if the gdata feature was disabled while running the fetch
   // loop.
   if (profile_->GetPrefs()->GetBoolean(prefs::kDisableGData))
     return true;
 
@@ skipping 34 matching lines @@
   DCHECK(posted);
 }
 
 void DriveScheduler::ResetThrottleAndContinueJobLoop() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   throttle_count_ = 0;
   DoJobLoop();
 }
 
-void DriveScheduler::OnRemoveDone(int job_id, DriveFileError error) {
+void DriveScheduler::OnJobDone(int job_id, DriveFileError error) {
   JobMap::iterator job_iter = job_info_.find(job_id);
   DCHECK(job_iter != job_info_.end());
 
   // Retry, depending on the error.
   if (error == DRIVE_FILE_ERROR_THROTTLED ||
       error == DRIVE_FILE_ERROR_NO_CONNECTION) {
     job_iter->second->job_info.state = STATE_RETRY;
 
     // Requeue the job.
     queue_.push_back(job_id);
     ThrottleAndContinueJobLoop();
   } else {
-    DCHECK(job_iter->second->remove_private.get());
-
     // Handle the callback.
-    if (!job_iter->second->remove_private->callback.is_null()) {
+    if (!job_iter->second->callback.is_null()) {
       MessageLoop::current()->PostTask(FROM_HERE,
-          base::Bind(job_iter->second->remove_private->callback, error));
+          base::Bind(job_iter->second->callback, error));
     }
 
     // Delete the job.
     job_info_.erase(job_id);
     ResetThrottleAndContinueJobLoop();
   }
 }
 
 void DriveScheduler::OnConnectionTypeChanged(
     net::NetworkChangeNotifier::ConnectionType type) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Resume the job loop if the network is back online. Note that we don't
   // need to check the type of the network as it will be checked in
   // ShouldStopJobLoop() as soon as the loop is resumed.
   if (!net::NetworkChangeNotifier::IsOffline())
     StartJobLoop();
 }
 
 }  // namespace drive