Decrease priority of syncing after copying to BACKGROUND.

As a result copying multiple files doesn't block other operations initiated by the user. TEST=Tested manually by copying 100 files from Downloads to Drive, then opening a not cached file on Drive BUG=420278 Review URL: https://codereview.chromium.org/692693003 Cr-Commit-Position: refs/heads/master@{#303970}
dednal · Nov 13, 2014 · fe48afd · fe48afd
1 parent 2e45711
commit fe48afd
Show file tree

Hide file tree

Showing 15 changed files with 52 additions and 23 deletions.
diff --git a/chrome/browser/chromeos/drive/file_system.cc b/chrome/browser/chromeos/drive/file_system.cc
@@ -182,7 +182,7 @@ FileError ResetOnBlockingPool(internal::ResourceMetadata* resource_metadata,
   FileError error = resource_metadata->Reset();
   if (error != FILE_ERROR_OK)
     return error;
- return cache->ClearAll() ? FILE_ERROR_OK : FILE_ERROR_FAILED;
+  return cache->ClearAll() ? FILE_ERROR_OK : FILE_ERROR_FAILED;
 }
 
 // Part of GetPathFromResourceId().
@@ -791,8 +791,9 @@ void FileSystem::OnFileChangedByOperation(const FileChange& changed_files) {
       FileSystemObserver, observers_, OnFileChanged(changed_files));
 }
 
-void FileSystem::OnEntryUpdatedByOperation(const std::string& local_id) {
-  sync_client_->AddUpdateTask(ClientContext(USER_INITIATED), local_id);
+void FileSystem::OnEntryUpdatedByOperation(const ClientContext& context,
+                                           const std::string& local_id) {
+  sync_client_->AddUpdateTask(context, local_id);
 }
 
 void FileSystem::OnDriveSyncError(file_system::DriveSyncErrorType type,

diff --git a/chrome/browser/chromeos/drive/file_system.h b/chrome/browser/chromeos/drive/file_system.h
@@ -28,7 +28,7 @@ class ResourceEntry;
 }  // namespace google_apis
 
 namespace drive {
-
+struct ClientContext;
 class DriveServiceInterface;
 class EventLogger;
 class FileCacheEntry;
@@ -164,7 +164,8 @@ class FileSystem : public FileSystemInterface,
   // file_system::OperationDelegate overrides.
   virtual void OnFileChangedByOperation(
       const FileChange& changed_files) override;
-  virtual void OnEntryUpdatedByOperation(const std::string& local_id) override;
+  virtual void OnEntryUpdatedByOperation(const ClientContext& context,
+                                         const std::string& local_id) override;
   virtual void OnDriveSyncError(file_system::DriveSyncErrorType type,
                                 const std::string& local_id) override;
   virtual bool WaitForSyncComplete(

diff --git a/chrome/browser/chromeos/drive/file_system/copy_operation.cc b/chrome/browser/chromeos/drive/file_system/copy_operation.cc
@@ -331,8 +331,11 @@ void CopyOperation::CopyAfterTryToCopyLocally(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!params->callback.is_null());
 
-  for (size_t i = 0; i < updated_local_ids->size(); ++i)
-    delegate_->OnEntryUpdatedByOperation((*updated_local_ids)[i]);
+  for (const auto& id : *updated_local_ids) {
+    // Syncing for copy should be done in background, so pass the BACKGROUND
+    // context. See: crbug.com/420278.
+    delegate_->OnEntryUpdatedByOperation(ClientContext(BACKGROUND), id);
+  }
 
   if (*directory_changed) {
     FileChange changed_file;
@@ -503,7 +506,10 @@ void CopyOperation::TransferJsonGdocFileAfterLocalWork(
     // This reparenting is already done in LocalWorkForTransferJsonGdocFile().
     case IS_ORPHAN: {
       DCHECK(!params->changed_path.empty());
-      delegate_->OnEntryUpdatedByOperation(params->local_id);
+      // Syncing for copy should be done in background, so pass the BACKGROUND
+      // context. See: crbug.com/420278.
+      delegate_->OnEntryUpdatedByOperation(ClientContext(BACKGROUND),
+                                           params->local_id);
 
       FileChange changed_file;
       changed_file.Update(
@@ -661,7 +667,9 @@ void CopyOperation::ScheduleTransferRegularFileAfterUpdateLocalState(
     FileChange changed_file;
     changed_file.Update(remote_dest_path, *entry, FileChange::ADD_OR_UPDATE);
     delegate_->OnFileChangedByOperation(changed_file);
-    delegate_->OnEntryUpdatedByOperation(*local_id);
+    // Syncing for copy should be done in background, so pass the BACKGROUND
+    // context. See: crbug.com/420278.
+    delegate_->OnEntryUpdatedByOperation(ClientContext(BACKGROUND), *local_id);
   }
   callback.Run(error);
 }

diff --git a/chrome/browser/chromeos/drive/file_system/create_directory_operation.cc b/chrome/browser/chromeos/drive/file_system/create_directory_operation.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/chromeos/drive/file_change.h"
 #include "chrome/browser/chromeos/drive/file_system/operation_delegate.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
+#include "chrome/browser/chromeos/drive/job_scheduler.h"
 #include "chrome/browser/chromeos/drive/resource_metadata.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -170,9 +171,9 @@ void CreateDirectoryOperation::CreateDirectoryAfterUpdateLocalState(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
-  for (std::set<std::string>::const_iterator it = updated_local_ids->begin();
-       it != updated_local_ids->end(); ++it)
-    delegate_->OnEntryUpdatedByOperation(*it);
+  for (const auto& id : *updated_local_ids) {
+    delegate_->OnEntryUpdatedByOperation(ClientContext(USER_INITIATED), id);
+  }
 
   delegate_->OnFileChangedByOperation(*changed_files);
 

diff --git a/chrome/browser/chromeos/drive/file_system/create_file_operation.cc b/chrome/browser/chromeos/drive/file_system/create_file_operation.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/chromeos/drive/drive.pb.h"
 #include "chrome/browser/chromeos/drive/file_change.h"
 #include "chrome/browser/chromeos/drive/file_system/operation_delegate.h"
+#include "chrome/browser/chromeos/drive/job_scheduler.h"
 #include "chrome/browser/chromeos/drive/resource_metadata.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/mime_util.h"
@@ -131,7 +132,9 @@ void CreateFileOperation::CreateFileAfterUpdateLocalState(
     changed_file.Update(
         file_path, FileChange::FILE_TYPE_FILE, FileChange::ADD_OR_UPDATE);
     delegate_->OnFileChangedByOperation(changed_file);
-    delegate_->OnEntryUpdatedByOperation(entry->local_id());
+    // Synchronize in the background.
+    delegate_->OnEntryUpdatedByOperation(ClientContext(BACKGROUND),
+                                         entry->local_id());
   }
   callback.Run(error);
 }

diff --git a/chrome/browser/chromeos/drive/file_system/get_file_for_saving_operation.cc b/chrome/browser/chromeos/drive/file_system/get_file_for_saving_operation.cc
@@ -193,7 +193,7 @@ void GetFileForSavingOperation::OnWriteEvent(
   logger_->Log(logging::LOG_INFO, "Detected modification to %s.",
                local_id.c_str());
 
-  delegate_->OnEntryUpdatedByOperation(local_id);
+  delegate_->OnEntryUpdatedByOperation(ClientContext(USER_INITIATED), local_id);
 
   // Clients may have enlarged the file. By FreeDiskpSpaceIfNeededFor(0),
   // we try to ensure (0 + the-minimum-safe-margin = 512MB as of now) space.

diff --git a/chrome/browser/chromeos/drive/file_system/get_file_for_saving_operation_unittest.cc b/chrome/browser/chromeos/drive/file_system/get_file_for_saving_operation_unittest.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/chromeos/drive/file_errors.h"
 #include "chrome/browser/chromeos/drive/file_system/operation_test_base.h"
 #include "chrome/browser/chromeos/drive/file_write_watcher.h"
+#include "chrome/browser/chromeos/drive/job_scheduler.h"
 #include "content/public/test/test_utils.h"
 #include "google_apis/drive/test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -36,7 +37,8 @@ class TestDelegate : public OperationDelegate {
   }
 
   // OperationDelegate overrides.
-  virtual void OnEntryUpdatedByOperation(const std::string& local_id) override {
+  virtual void OnEntryUpdatedByOperation(const ClientContext& /* context */,
+                                         const std::string& local_id) override {
     updated_local_id_ = local_id;
     if (!quit_closure_.is_null())
       quit_closure_.Run();

diff --git a/chrome/browser/chromeos/drive/file_system/move_operation.cc b/chrome/browser/chromeos/drive/file_system/move_operation.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/chromeos/drive/drive.pb.h"
 #include "chrome/browser/chromeos/drive/file_change.h"
 #include "chrome/browser/chromeos/drive/file_system/operation_delegate.h"
+#include "chrome/browser/chromeos/drive/job_scheduler.h"
 #include "chrome/browser/chromeos/drive/resource_metadata.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -113,7 +114,8 @@ void MoveOperation::MoveAfterUpdateLocalState(
   if (error == FILE_ERROR_OK) {
     // Notify the change of directory.
     delegate_->OnFileChangedByOperation(*changed_files);
-    delegate_->OnEntryUpdatedByOperation(*local_id);
+    delegate_->OnEntryUpdatedByOperation(ClientContext(USER_INITIATED),
+                                         *local_id);
   }
   callback.Run(error);
 }

diff --git a/chrome/browser/chromeos/drive/file_system/open_file_operation.cc b/chrome/browser/chromeos/drive/file_system/open_file_operation.cc
@@ -170,7 +170,8 @@ void OpenFileOperation::CloseFile(
   if (--open_files_[local_id] == 0) {
     // All clients closes this file, so notify to upload the file.
     open_files_.erase(local_id);
-    delegate_->OnEntryUpdatedByOperation(local_id);
+    delegate_->OnEntryUpdatedByOperation(ClientContext(USER_INITIATED),
+                                         local_id);
 
     // Clients may have enlarged the file. By FreeDiskpSpaceIfNeededFor(0),
     // we try to ensure (0 + the-minimum-safe-margin = 512MB as of now) space.

diff --git a/chrome/browser/chromeos/drive/file_system/operation_delegate.h b/chrome/browser/chromeos/drive/file_system/operation_delegate.h
@@ -12,6 +12,8 @@ class FilePath;
 }
 
 namespace drive {
+
+struct ClientContext;
 class FileChange;
 
 namespace file_system {
@@ -35,8 +37,10 @@ class OperationDelegate {
   // changed directory.
   virtual void OnFileChangedByOperation(const FileChange& changed_files) {}
 
-  // Sent when an entry is updated and sync is needed.
-  virtual void OnEntryUpdatedByOperation(const std::string& local_id) {}
+  // Sent when an entry is updated and sync is needed. The passed |context| is
+  // used for syncing.
+  virtual void OnEntryUpdatedByOperation(const ClientContext& context,
+                                         const std::string& local_id) {}
 
   // Sent when a specific drive sync error occurred.
   // |local_id| is the local ID of the resource entry.

diff --git a/chrome/browser/chromeos/drive/file_system/operation_test_base.cc b/chrome/browser/chromeos/drive/file_system/operation_test_base.cc
@@ -35,6 +35,7 @@ void OperationTestBase::LoggingDelegate::OnFileChangedByOperation(
 }
 
 void OperationTestBase::LoggingDelegate::OnEntryUpdatedByOperation(
+    const ClientContext& /* client_context */,
     const std::string& local_id) {
   updated_local_ids_.insert(local_id);
 }

diff --git a/chrome/browser/chromeos/drive/file_system/operation_test_base.h b/chrome/browser/chromeos/drive/file_system/operation_test_base.h
@@ -23,7 +23,7 @@ class SequencedTaskRunner;
 }  // namespace base
 
 namespace drive {
-
+struct ClientContext;
 class EventLogger;
 class FakeDriveService;
 class FakeFreeDiskSpaceGetter;
@@ -59,6 +59,7 @@ class OperationTestBase : public testing::Test {
     virtual void OnFileChangedByOperation(
         const FileChange& changed_files) override;
     virtual void OnEntryUpdatedByOperation(
+        const ClientContext& context,
         const std::string& local_id) override;
     virtual void OnDriveSyncError(DriveSyncErrorType type,
                                   const std::string& local_id) override;

diff --git a/chrome/browser/chromeos/drive/file_system/remove_operation.cc b/chrome/browser/chromeos/drive/file_system/remove_operation.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/chromeos/drive/file_change.h"
 #include "chrome/browser/chromeos/drive/file_system/operation_delegate.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
+#include "chrome/browser/chromeos/drive/job_scheduler.h"
 #include "chrome/browser/chromeos/drive/resource_metadata.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -118,7 +119,8 @@ void RemoveOperation::RemoveAfterUpdateLocalState(
     changed_file.Update(*changed_path, *entry, FileChange::DELETE);
     if (error == FILE_ERROR_OK) {
       delegate_->OnFileChangedByOperation(changed_file);
-      delegate_->OnEntryUpdatedByOperation(*local_id);
+      delegate_->OnEntryUpdatedByOperation(ClientContext(USER_INITIATED),
+                                           *local_id);
     }
   }
 

diff --git a/chrome/browser/chromeos/drive/file_system/touch_operation.cc b/chrome/browser/chromeos/drive/file_system/touch_operation.cc
@@ -11,6 +11,7 @@
 #include "chrome/browser/chromeos/drive/file_change.h"
 #include "chrome/browser/chromeos/drive/file_errors.h"
 #include "chrome/browser/chromeos/drive/file_system/operation_delegate.h"
+#include "chrome/browser/chromeos/drive/job_scheduler.h"
 #include "chrome/browser/chromeos/drive/resource_metadata.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -102,7 +103,8 @@ void TouchOperation::TouchFileAfterUpdateLocalState(
 
   if (error == FILE_ERROR_OK) {
     delegate_->OnFileChangedByOperation(changed_files);
-    delegate_->OnEntryUpdatedByOperation(*local_id);
+    delegate_->OnEntryUpdatedByOperation(ClientContext(USER_INITIATED),
+                                         *local_id);
   }
   callback.Run(error);
 }

diff --git a/chrome/browser/chromeos/drive/file_system/truncate_operation.cc b/chrome/browser/chromeos/drive/file_system/truncate_operation.cc
@@ -139,7 +139,7 @@ void TruncateOperation::TruncateAfterTruncateOnBlockingPool(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
-  delegate_->OnEntryUpdatedByOperation(local_id);
+  delegate_->OnEntryUpdatedByOperation(ClientContext(USER_INITIATED), local_id);
 
   callback.Run(error);
 }