Pass calls to GetDocuments through the scheduler

R=kinaba@chromium.org
BUG=160904


Review URL: https://chromiumcodereview.appspot.com/11418127

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

chrome/browser/chromeos/drive/drive_feed_loader.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/chromeos/drive/drive_feed_loader_observer.h"
 #include "chrome/browser/chromeos/drive/drive_feed_processor.h"
 #include "chrome/browser/chromeos/drive/drive_file_system_util.h"
+#include "chrome/browser/chromeos/drive/drive_scheduler.h"
 #include "chrome/browser/chromeos/drive/drive_webapps_registry.h"
 #include "chrome/browser/google_apis/drive_api_parser.h"
 #include "chrome/browser/google_apis/drive_api_util.h"
@@ -254,11 +255,13 @@ struct DriveFeedLoader::GetDocumentsUiState {
 DriveFeedLoader::DriveFeedLoader(
     DriveResourceMetadata* resource_metadata,
     google_apis::DriveServiceInterface* drive_service,
+    DriveScheduler* scheduler,
     DriveWebAppsRegistryInterface* webapps_registry,
     DriveCache* cache,
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner)
     : resource_metadata_(resource_metadata),
       drive_service_(drive_service),
+      scheduler_(scheduler),
       webapps_registry_(webapps_registry),
       cache_(cache),
       blocking_task_runner_(blocking_task_runner),
@@ -405,7 +408,7 @@ void DriveFeedLoader::LoadFromServer(scoped_ptr<LoadFeedParams> params) {
   // base::Passed() may get evaluated first, so get a pointer to params.
   LoadFeedParams* params_ptr = params.get();
   if (google_apis::util::IsDriveV2ApiEnabled()) {
-    drive_service_->GetDocuments(
+    scheduler_->GetDocuments(
         params_ptr->feed_to_load,
         params_ptr->start_changestamp,
         std::string(),  // No search query.
@@ -416,7 +419,7 @@ void DriveFeedLoader::LoadFromServer(scoped_ptr<LoadFeedParams> params) {
                    base::Passed(&params),
                    start_time));
   } else {
-    drive_service_->GetDocuments(
+    scheduler_->GetDocuments(
         params_ptr->feed_to_load,
         params_ptr->start_changestamp,
         params_ptr->search_query,
@@ -559,7 +562,7 @@ void DriveFeedLoader::OnParseFeed(
     // pointer so we can use it bellow.
     LoadFeedParams* params_ptr = params.get();
     // Kick off the remaining part of the feeds.
-    drive_service_->GetDocuments(
+    scheduler_->GetDocuments(
         next_feed_url,
         params_ptr->start_changestamp,
         params_ptr->search_query,
@@ -662,7 +665,7 @@ void DriveFeedLoader::OnGetChangelist(scoped_ptr<LoadFeedParams> params,
     // Kick off the remaining part of the feeds.
     // Extract the pointer so we can use it bellow.
     LoadFeedParams* params_ptr = params.get();
-    drive_service_->GetDocuments(
+    scheduler_->GetDocuments(
         current_feed->next_link(),
         params_ptr->start_changestamp,
         std::string(),  // No search query.

chrome/browser/chromeos/drive/drive_feed_loader.h

@@ -31,6 +31,7 @@ namespace drive {
 class DriveCache;
 class DriveFeedLoaderObserver;
 class DriveFeedProcessor;
+class DriveScheduler;
 class DriveWebAppsRegistryInterface;
 
 // Callback run as a response to SearchFromServer and LoadDirectoryFromServer.
@@ -45,6 +46,7 @@ class DriveFeedLoader {
   DriveFeedLoader(
       DriveResourceMetadata* resource_metadata,
       google_apis::DriveServiceInterface* drive_service,
+      DriveScheduler* scheduler,
       DriveWebAppsRegistryInterface* webapps_registry,
       DriveCache* cache,
       scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
@@ -174,6 +176,7 @@ class DriveFeedLoader {
 
   DriveResourceMetadata* resource_metadata_;  // Not owned.
   google_apis::DriveServiceInterface* drive_service_;  // Not owned.
+  DriveScheduler* scheduler_;  // Not owned.
   DriveWebAppsRegistryInterface* webapps_registry_;  // Not owned.
   DriveCache* cache_;  // Not owned.
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;

chrome/browser/chromeos/drive/drive_file_system.cc

@@ -305,7 +305,9 @@ DriveFileSystem::DriveFileSystem(
       last_update_check_error_(DRIVE_FILE_OK),
       hide_hosted_docs_(false),
       blocking_task_runner_(blocking_task_runner),
-      scheduler_(new DriveScheduler(profile, &drive_operations_)),
+      scheduler_(new DriveScheduler(profile,
+                                    drive_service,
+                                    &drive_operations_)),
       polling_interval_sec_(kFastPollingIntervalInSec),
       push_notification_enabled_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(ui_weak_ptr_factory_(this)),
@@ -343,6 +345,7 @@ void DriveFileSystem::ResetResourceMetadata() {
   resource_metadata_.reset(new DriveResourceMetadata);
   feed_loader_.reset(new DriveFeedLoader(resource_metadata_.get(),
                                          drive_service_,
+                                         scheduler_.get(),
                                          webapps_registry_,
                                          cache_,
                                          blocking_task_runner_));

chrome/browser/chromeos/drive/drive_scheduler.h

@@ -8,6 +8,7 @@
 #include "base/memory/linked_ptr.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/chromeos/drive/drive_file_system_interface.h"
+#include "chrome/browser/google_apis/drive_service_interface.h"
 #include "net/base/network_change_notifier.h"
 
 #include <deque>
@@ -33,6 +34,7 @@ class DriveScheduler
   // Enum representing the type of job.
   enum JobType {
     TYPE_COPY,
+    TYPE_GET_DOCUMENTS,
     TYPE_MOVE,
     TYPE_REMOVE,
     TYPE_TRANSFER_LOCAL_TO_REMOTE,
@@ -76,6 +78,7 @@ class DriveScheduler
   };
 
   DriveScheduler(Profile* profile,
+                 google_apis::DriveServiceInterface* drive_service,
                  file_system::DriveOperations* drive_operations);
   virtual ~DriveScheduler();
 
@@ -88,6 +91,14 @@ class DriveScheduler
             const FilePath& dest_file_path,
             const FileOperationCallback& callback);
 
+  // Adds a GetDocuments operation to the queue.
+  void GetDocuments(const GURL& feed_url,
+                    int64 start_changestamp,
+                    const std::string& search_query,
+                    bool shared_with_me,
+                    const std::string& directory_resource_id,
+                    const google_apis::GetDataCallback& callback);
+
   // Adds a transfer operation to the queue.
   void TransferFileFromRemoteToLocal(const FilePath& remote_src_file_path,
                                      const FilePath& local_dest_file_path,
@@ -119,8 +130,7 @@ class DriveScheduler
   // Represents a single entry in the job queue.
   struct QueueEntry {
     QueueEntry(JobType in_job_type,
-               FilePath in_file_path,
-               FileOperationCallback in_callback);
+               FilePath in_file_path);
     ~QueueEntry();
 
     JobInfo job_info;
@@ -133,7 +143,7 @@ class DriveScheduler
     //   TYPE_TRANSFER_LOCAL_TO_REMOTE,
     //   TYPE_TRANSFER_REGULAR_FILE,
     //   TYPE_TRANSFER_REMOTE_TO_LOCAL
-    FileOperationCallback callback;
+    FileOperationCallback file_operation_callback;
 
     // Destination of the operation.
     // Used by:
@@ -144,8 +154,18 @@ class DriveScheduler
     //   TYPE_TRANSFER_REMOTE_TO_LOCAL
     FilePath dest_file_path;
 
-    // Whether the operation is recursive.  Used by: TYPE_REMOVE
+    // Whether the operation is recursive.  Used by:
+    //   TYPE_REMOVE
     bool is_recursive;
+
+    // Parameters for GetDocuments().  Used by:
+    //   TYPE_GET_DOCUMENTS
+    GURL feed_url;
+    int64 start_changestamp;
+    std::string search_query;
+    bool shared_with_me;
+    std::string directory_resource_id;
+    google_apis::GetDataCallback get_data_callback;
   };
 
   // Adds the specified job to the queue.  Takes ownership of |job|
@@ -170,9 +190,17 @@ class DriveScheduler
   // Resets the throttle delay to the initial value, and continues the job loop.
   void ResetThrottleAndContinueJobLoop();
 
-  // Callback for job finishing.  Retries the job if needed, otherwise cleans up
-  // the job, invokes the callback, and continues the job loop.
-  void OnJobDone(int job_id, DriveFileError error);
+  // Retries the job if needed, otherwise cleans up the job, invokes the
+  // callback, and continues the job loop.
+  scoped_ptr<QueueEntry> OnJobDone(int job_id, DriveFileError error);
+
+  // Callback for job finishing with a FileOperationCallback.
+  void OnFileOperationJobDone(int job_id, DriveFileError error);
+
+  // Callback for job finishing with a GetDataCallback.
+  void OnGetDataJobDone(int job_id,
+                        google_apis::GDataErrorCode error,
+                        scoped_ptr<base::Value> feed_data);
 
   // net::NetworkChangeNotifier::ConnectionTypeObserver override.
   virtual void OnConnectionTypeChanged(
@@ -206,6 +234,8 @@ class DriveScheduler
   // Drive operations.
   file_system::DriveOperations* drive_operations_;
 
+  google_apis::DriveServiceInterface* drive_service_;
+
   Profile* profile_;
 
   // Note: This should remain the last member so it'll be destroyed and

chrome/browser/chromeos/drive/drive_scheduler_unittest.cc

@@ -5,6 +5,8 @@
 #include "chrome/browser/chromeos/drive/drive_scheduler.h"
 
 #include "base/bind.h"
+#include "base/file_util.h"
+#include "base/json/json_reader.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/chromeos/drive/drive_test_util.h"
 #include "chrome/browser/chromeos/drive/file_system/drive_operations.h"
@@ -20,10 +22,147 @@
 
 using ::testing::AnyNumber;
 using ::testing::DoAll;
+using ::testing::Eq;
 using ::testing::Return;
 using ::testing::StrictMock;
 using ::testing::_;
 
+namespace google_apis {
+
+class FakeDriveService : public DriveServiceInterface {
+  virtual void Initialize(Profile* profile) {
+  }
+
+  virtual void AddObserver(DriveServiceObserver* observer) {
+  }
+
+  virtual void RemoveObserver(DriveServiceObserver* observer) {
+  }
+
+  virtual bool CanStartOperation() const {
+    return true;
+  }
+
+  virtual void CancelAll() {
+  }
+
+  virtual bool CancelForFilePath(const FilePath& file_path) {
+    return true;
+  }
+
+  virtual OperationProgressStatusList GetProgressStatusList() const {
+    return OperationProgressStatusList();
+  }
+
+  virtual bool HasAccessToken() const {
+    return true;
+  }
+
+  virtual bool HasRefreshToken() const {
+    return true;
+  }
+
+  virtual void GetDocuments(const GURL& feed_url,
+                            int64 start_changestamp,
+                            const std::string& search_query,
+                            bool shared_with_me,
+                            const std::string& directory_resource_id,
+                            const GetDataCallback& callback) {
+    // TODO: Make this more flexible.
+    if (feed_url == GURL("http://example.com/gdata/root_feed.json")) {
+      // Make some sample data.
+      const FilePath feed_path =
+          test_util::GetTestFilePath("gdata/root_feed.json");
+      std::string feed_contents;
+      file_util::ReadFileToString(feed_path, &feed_contents);
+      scoped_ptr<base::Value> feed_data(
+          base::JSONReader::Read(feed_contents));
+
+      base::MessageLoopProxy::current()->PostTask(FROM_HERE,
+          base::Bind(callback,
+                     HTTP_SUCCESS,
+                     base::Passed(&feed_data)));
+    } else {
+      scoped_ptr<base::Value> feed_data;
+      base::MessageLoopProxy::current()->PostTask(FROM_HERE,
+          base::Bind(callback,
+                     GDATA_PARSE_ERROR,
+                     base::Passed(&feed_data)));
+    }
+  }
+
+  virtual void GetDocumentEntry(const std::string& resource_id,
+                                const GetDataCallback& callback) {
+  }
+
+  virtual void GetAccountMetadata(const GetDataCallback& callback) {
+  }
+
+  virtual void GetApplicationInfo(const GetDataCallback& callback) {
+  }
+
+  virtual void DeleteDocument(const GURL& document_url,
+                              const EntryActionCallback& callback) {
+  }
+
+  virtual void DownloadDocument(const FilePath& virtual_path,
+                                const FilePath& local_cache_path,
+                                const GURL& content_url,
+                                DocumentExportFormat format,
+                                const DownloadActionCallback& callback) {
+  }
+
+  virtual void CopyDocument(const std::string& resource_id,
+                            const FilePath::StringType& new_name,
+                            const GetDataCallback& callback) {
+  }
+
+  virtual void RenameResource(const GURL& resource_url,
+                              const FilePath::StringType& new_name,
+                              const EntryActionCallback& callback) {
+  }
+
+  virtual void AddResourceToDirectory(const GURL& parent_content_url,
+                                      const GURL& resource_url,
+                                      const EntryActionCallback& callback) {
+  }
+
+  virtual void RemoveResourceFromDirectory(
+      const GURL& parent_content_url,
+      const std::string& resource_id,
+      const EntryActionCallback& callback) {
+  }
+
+  virtual void AddNewDirectory(const GURL& parent_content_url,
+                               const FilePath::StringType& directory_name,
+                               const GetDataCallback& callback) {
+  }
+
+  virtual void DownloadFile(
+      const FilePath& virtual_path,
+      const FilePath& local_cache_path,
+      const GURL& content_url,
+      const DownloadActionCallback& download_action_callback,
+      const GetContentCallback& get_content_callback) {
+  }
+
+  virtual void InitiateUpload(const InitiateUploadParams& params,
+                              const InitiateUploadCallback& callback) {
+  }
+
+  virtual void ResumeUpload(const ResumeUploadParams& params,
+                            const ResumeUploadCallback& callback) {
+  }
+
+  virtual void AuthorizeApp(const GURL& resource_url,
+                            const std::string& app_id,
+                            const GetDataCallback& callback) {
+  }
+
+};
+
+} // namespace google_apis
+
 namespace drive {
 
 namespace {
@@ -100,6 +239,7 @@ class DriveSchedulerTest : public testing::Test {
   virtual void SetUp() OVERRIDE {
     mock_network_change_notifier_.reset(new MockNetworkChangeNotifier);
 
+    fake_drive_service_.reset(new google_apis::FakeDriveService());
     mock_copy_operation_ = new StrictMock<MockCopyOperation>();
     mock_move_operation_ = new StrictMock<MockMoveOperation>();
     mock_remove_operation_ = new StrictMock<MockRemoveOperation>();
@@ -108,6 +248,7 @@ class DriveSchedulerTest : public testing::Test {
                                      mock_remove_operation_,
                                      NULL);
     scheduler_.reset(new DriveScheduler(profile_.get(),
+                                        fake_drive_service_.get(),
                                         &drive_operations_));
 
     scheduler_->Initialize();
@@ -119,6 +260,7 @@ class DriveSchedulerTest : public testing::Test {
     // registers itself as observer of NetworkLibrary.
     scheduler_.reset();
     google_apis::test_util::RunBlockingPoolTask();
+    fake_drive_service_.reset();
     mock_network_change_notifier_.reset();
   }
 
@@ -159,6 +301,7 @@ class DriveSchedulerTest : public testing::Test {
   scoped_ptr<TestingProfile> profile_;
   scoped_ptr<DriveScheduler> scheduler_;
   scoped_ptr<MockNetworkChangeNotifier> mock_network_change_notifier_;
+  scoped_ptr<google_apis::FakeDriveService> fake_drive_service_;
 
   file_system::DriveOperations drive_operations_;
   StrictMock<MockCopyOperation>* mock_copy_operation_;
@@ -237,6 +380,27 @@ TEST_F(DriveSchedulerTest, TransferRegularFileFile) {
   ASSERT_EQ(DRIVE_FILE_OK, error);
 }
 
+TEST_F(DriveSchedulerTest, GetDocuments) {
+  ConnectToWifi();
+
+  google_apis::GDataErrorCode error = google_apis::GDATA_OTHER_ERROR;
+  scoped_ptr<base::Value> value;
+
+  scheduler_->GetDocuments(
+      GURL("http://example.com/gdata/root_feed.json"),
+      0,
+      std::string(),
+      true,
+      std::string(),
+      base::Bind(&google_apis::test_util::CopyResultsFromGetDataCallback,
+                 &error,
+                 &value));
+  google_apis::test_util::RunBlockingPoolTask();
+
+  ASSERT_EQ(google_apis::HTTP_SUCCESS, error);
+  ASSERT_TRUE(value);
+}
+
 TEST_F(DriveSchedulerTest, MoveFile) {
   ConnectToWifi();
 

chrome/browser/google_apis/base_operations_unittest.cc

@@ -43,15 +43,6 @@ class JsonParseTestGetDataOperation : public GetDataOperation {
   }
 };
 
-// Copies the results from GetDataCallback.
-void CopyResultsFromGetDataCallback(GDataErrorCode* error_out,
-                                    scoped_ptr<base::Value>* value_out,
-                                    GDataErrorCode error_in,
-                                    scoped_ptr<base::Value> value_in) {
-  value_out->swap(value_in);
-  *error_out = error_in;
-}
-
 }  // namespace
 
 class BaseOperationsTest : public testing::Test {
@@ -81,7 +72,9 @@ TEST_F(BaseOperationsTest, GetDataOperation_ParseValidJson) {
   JsonParseTestGetDataOperation* get_data =
       new JsonParseTestGetDataOperation(
           runner_->operation_registry(),
-          base::Bind(&CopyResultsFromGetDataCallback, &error, &value));
+          base::Bind(&test_util::CopyResultsFromGetDataCallback,
+                     &error,
+                     &value));
   get_data->NotifyStart();
 
   const std::string valid_json_str = "{ \"test\": 123 }";
@@ -108,7 +101,9 @@ TEST_F(BaseOperationsTest, GetDataOperation_ParseInvalidJson) {
   JsonParseTestGetDataOperation* get_data =
       new JsonParseTestGetDataOperation(
           runner_->operation_registry(),
-          base::Bind(&CopyResultsFromGetDataCallback, &error, &value));
+          base::Bind(&test_util::CopyResultsFromGetDataCallback,
+                     &error,
+                     &value));
   get_data->NotifyStart();
 
   const std::string invalid_json_str = "$$$";

chrome/browser/google_apis/test_util.cc

@@ -69,5 +69,13 @@ scoped_ptr<base::Value> LoadJSONFile(const std::string& relative_path) {
   return value.Pass();
 }
 
+void CopyResultsFromGetDataCallback(GDataErrorCode* error_out,
+                                    scoped_ptr<base::Value>* value_out,
+                                    GDataErrorCode error_in,
+                                    scoped_ptr<base::Value> value_in) {
+  value_out->swap(value_in);
+  *error_out = error_in;
+}
+
 }  // namespace test_util
 }  // namespace google_apis

chrome/browser/google_apis/test_util.h

@@ -5,11 +5,10 @@
 #ifndef CHROME_BROWSER_GOOGLE_APIS_TEST_UTIL_H_
 #define CHROME_BROWSER_GOOGLE_APIS_TEST_UTIL_H_
 
-#include "chrome/browser/google_apis/test_util.h"
-
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
+#include "chrome/browser/google_apis/gdata_errorcode.h"
 
 class FilePath;
 
@@ -37,6 +36,12 @@ FilePath GetTestFilePath(const std::string& relative_path);
 // chrome/test/data/chromeos.
 scoped_ptr<base::Value> LoadJSONFile(const std::string& relative_path);
 
+// Copies the results from GetDataCallback.
+void CopyResultsFromGetDataCallback(GDataErrorCode* error_out,
+                                    scoped_ptr<base::Value>* value_out,
+                                    GDataErrorCode error_in,
+                                    scoped_ptr<base::Value> value_in);
+
 }  // namespace test_util
 }  // namespace google_apis