Sync FileSystem: Drop is_fetching_changes_ flag and completion_callback

chrome/browser/sync_file_system/drive_file_sync_service.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/sync_file_system/drive_file_sync_service.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 
@@ skipping 34 matching lines @@
     FILE_PATH_LITERAL("Sync FileSystem");
 
 // The sync invalidation object ID for Google Drive.
 const char kDriveInvalidationObjectId[] = "CHANGELOG";
 
 // Incremental sync polling interval.
 // TODO(calvinlo): Improve polling algorithm dependent on whether push
 // notifications are on or off.
 const int64 kMinimumPollingDelaySeconds = 5;
 const int64 kMaximumPollingDelaySeconds = 10 * 60;  // 10 min
-const int64 kPollingDelaySecondsWithNotification = 4 * 60 * 60;  // 4 hr
+const int64 kPollingDelaySecondsWithNotification = 10 * 60;  // 10 min
 const double kDelayMultiplier = 1.6;
 
 bool CreateTemporaryFile(const FilePath& dir_path, FilePath* temp_file) {
   return file_util::CreateDirectory(dir_path) &&
       file_util::CreateTemporaryFileInDir(dir_path, temp_file);
 }
 
 void DeleteTemporaryFile(const FilePath& file_path) {
   if (!content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE)) {
     content::BrowserThread::PostTask(
         content::BrowserThread::FILE, FROM_HERE,
         base::Bind(&DeleteTemporaryFile, file_path));
     return;
   }
 
   if (!file_util::Delete(file_path, true))
     LOG(ERROR) << "Leaked temporary file for Sync FileSystem: "
                << file_path.value();
 }
 
 void EmptyStatusCallback(fileapi::SyncStatusCode code) {}
 
-void MarkFetchingChangesCompleted(bool* is_fetching_changes) {
-  *is_fetching_changes = false;
-}
-
 void DidRemoveOrigin(const GURL& origin, fileapi::SyncStatusCode status) {
   // TODO(calvinlo): Disable syncing if status not ok (http://crbug.com/171611).
   DCHECK_EQ(fileapi::SYNC_STATUS_OK, status);
   LOG(WARNING) << "Remove origin failed for: " << origin.spec()
                << " status=" << status;
 }
 
 fileapi::FileChange CreateFileChange(bool is_deleted) {
   if (is_deleted) {
     return fileapi::FileChange(fileapi::FileChange::FILE_CHANGE_DELETE,
@@ skipping 11 matching lines @@
  public:
   explicit TaskToken(const base::WeakPtr<DriveFileSyncService>& sync_service)
       : sync_service_(sync_service),
         task_type_(TASK_TYPE_NONE) {
   }
 
   void ResetTask(const tracked_objects::Location& location) {
     location_ = location;
     task_type_ = TASK_TYPE_NONE;
     description_.clear();
-    if (!completion_callback_.is_null())
-      completion_callback_.Run();
-    completion_callback_.Reset();
   }
 
   void UpdateTask(const tracked_objects::Location& location,
                   TaskType task_type,
                   const std::string& description) {
     location_ = location;
     task_type_ = task_type;
     description_ = description;
 
     DVLOG(2) << "Token updated: " << description_
              << " " << location_.ToString();
   }
 
   const tracked_objects::Location& location() const { return location_; }
   TaskType task_type() const { return task_type_; }
   const std::string& description() const { return description_; }
   std::string done_description() const { return description_ + " done"; }
 
-  void set_completion_callback(const base::Closure& callback) {
-    completion_callback_ = callback;
-  }
-
-  const base::Closure& completion_callback() {
-    return completion_callback_;
-  }
-
   ~TaskToken() {
     // All task on DriveFileSyncService must hold TaskToken instance to ensure
     // no other tasks are running. Also, as soon as a task finishes to work,
     // it must return the token to DriveFileSyncService.
     // Destroying a token with valid |sync_service_| indicates the token was
     // dropped by a task without returning.
     if (sync_service_) {
       LOG(ERROR) << "Unexpected TaskToken deletion from: "
                  << location_.ToString() << " while: " << description_;
     }
     DCHECK(!sync_service_);
   }
 
  private:
   base::WeakPtr<DriveFileSyncService> sync_service_;
   tracked_objects::Location location_;
   TaskType task_type_;
   std::string description_;
-  base::Closure completion_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(TaskToken);
 };
 
 void DriveFileSyncService::OnInvalidatorStateChange(
     syncer::InvalidatorState state) {
   SetPushNotificationEnabled(state);
 }
 
 void DriveFileSyncService::SetPushNotificationEnabled(
     syncer::InvalidatorState state) {
   push_notification_enabled_ = (state == syncer::INVALIDATIONS_ENABLED);
   if (!push_notification_enabled_)
     return;
 
   // Push notifications are enabled so reset polling timer.
   UpdatePollingDelay(kPollingDelaySecondsWithNotification);
 }
 
 void DriveFileSyncService::OnIncomingInvalidation(
     const syncer::ObjectIdInvalidationMap& invalidation_map) {
   DCHECK(push_notification_enabled_);
   DCHECK_EQ(1U, invalidation_map.size());
   const invalidation::ObjectId object_id(
       ipc::invalidation::ObjectSource::COSMO_CHANGELOG,
       kDriveInvalidationObjectId);
   DCHECK_EQ(1U, invalidation_map.count(object_id));
 
+  has_unfetched_remote_change_ = true;
   FetchChangesForIncrementalSync();
 }
 
 struct DriveFileSyncService::ProcessRemoteChangeParam {
   scoped_ptr<TaskToken> token;
   RemoteChangeProcessor* processor;
   RemoteChange remote_change;
   fileapi::SyncFileCallback callback;
 
   DriveMetadata drive_metadata;
@@ skipping 81 matching lines @@
 
 DriveFileSyncService::DriveFileSyncService(Profile* profile)
     : profile_(profile),
       last_operation_status_(fileapi::SYNC_STATUS_OK),
       state_(REMOTE_SERVICE_OK),
       sync_enabled_(true),
       largest_fetched_changestamp_(0),
       push_notification_registered_(false),
       push_notification_enabled_(false),
       polling_delay_seconds_(kMinimumPollingDelaySeconds),
-      is_fetching_changes_(false),
+      has_unfetched_remote_change_(true),
       weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   temporary_file_dir_ =
       profile->GetPath().Append(kSyncFileSystemDir).Append(kTempDirName);
   token_.reset(new TaskToken(AsWeakPtr()));
 
   sync_client_.reset(new DriveFileSyncClient(profile));
   sync_client_->AddObserver(this);
 
   metadata_store_.reset(new DriveMetadataStore(
       profile->GetPath().Append(kSyncFileSystemDir),
@@ skipping 399 matching lines @@
     scoped_ptr<DriveFileSyncClient> sync_client,
     scoped_ptr<DriveMetadataStore> metadata_store)
     : profile_(profile),
       last_operation_status_(fileapi::SYNC_STATUS_OK),
       state_(REMOTE_SERVICE_OK),
       sync_enabled_(true),
       largest_fetched_changestamp_(0),
       push_notification_registered_(false),
       push_notification_enabled_(false),
       polling_delay_seconds_(-1),
-      is_fetching_changes_(false),
+      has_unfetched_remote_change_(true),
       weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   DCHECK(profile);
   temporary_file_dir_ = base_dir.Append(kTempDirName);
 
   token_.reset(new TaskToken(AsWeakPtr()));
   sync_client_ = sync_client.Pass();
   metadata_store_ = metadata_store.Pass();
 
   base::MessageLoopProxy::current()->PostTask(
       FROM_HERE,
@@ skipping 39 matching lines @@
 
     // Notify remote sync service state if the state has been changed.
     if (!token_->description().empty() || old_state != GetCurrentState()) {
       FOR_EACH_OBSERVER(
           Observer, service_observers_,
           OnRemoteServiceStateUpdated(GetCurrentState(),
                                       token_->done_description()));
     }
   }
 
-  if (!token_->completion_callback().is_null())
-    token_->completion_callback().Run();
-
   token_->ResetTask(FROM_HERE);
   if (!pending_tasks_.empty()) {
     base::Closure closure = pending_tasks_.front();
     pending_tasks_.pop_front();
     closure.Run();
     return;
   }
 
+  if (has_unfetched_remote_change_ && push_notification_enabled_) {
+    FetchChangesForIncrementalSync();
+    return;
+  }
+
   SchedulePolling();
 
-  if (GetCurrentState() != REMOTE_SERVICE_OK &&
-      GetCurrentState() != REMOTE_SERVICE_TEMPORARY_UNAVAILABLE)
+  if (GetCurrentState() != REMOTE_SERVICE_OK)
     return;
 
   // If the state has become OK or TEMPORARY_UNAVAILABLE and we have any
   // pending batch sync origins, restart batch sync for them.
   if (!pending_batch_sync_origins_.empty()) {
     GURL origin = *pending_batch_sync_origins_.begin();
     pending_batch_sync_origins_.erase(pending_batch_sync_origins_.begin());
     std::string resource_id = metadata_store_->GetResourceIdForOrigin(origin);
     StartBatchSyncForOrigin(origin, resource_id);
     return;
@@ skipping 1035 matching lines @@
     return false;
   const PathToChangeMap& path_to_change = found_url->second;
   PathToChangeMap::const_iterator found_path = path_to_change.find(url.path());
   if (found_path == path_to_change.end())
     return false;
   *change = found_path->second;
   return true;
 }
 
 void DriveFileSyncService::FetchChangesForIncrementalSync() {
+  if (!sync_enabled_ ||
+      !pending_batch_sync_origins_.empty() ||
+      metadata_store_->incremental_sync_origins().empty() ||
+      !pending_changes_.empty())
+    return;
+
   scoped_ptr<TaskToken> token(GetToken(FROM_HERE, TASK_TYPE_DRIVE,
                                        "Fetching remote change list"));
-  // If we got |token| successfully, |is_fetching_changes_| should be false.
-  // |is_fetching_changes_| is true only when the FetchChanges sequence is
-  // running or is in |pending_queue_|. In both case, the token is owned by
-  // the FetchChanges sequence.
-  DCHECK(!token || !is_fetching_changes_);
-
-  if (!sync_enabled_ ||
-      is_fetching_changes_ ||
-      !pending_batch_sync_origins_.empty() ||
-      metadata_store_->incremental_sync_origins().empty() ||
-      !pending_changes_.empty()) {
-    if (token) {
-      token->ResetTask(FROM_HERE);
-      NotifyTaskDone(last_operation_status_, token.Pass());
-    }
+  if (!token)
     return;
-  }
-
-  is_fetching_changes_ = true;
-
-  if (!token) {
-    pending_tasks_.push_back(base::Bind(
-        &DriveFileSyncService::FetchChangesForIncrementalSync, AsWeakPtr()));
-    return;
-  }
-
-  token->set_completion_callback(
-      base::Bind(&MarkFetchingChangesCompleted, &is_fetching_changes_));
-
-  if (metadata_store_->incremental_sync_origins().empty()) {
-    token->ResetTask(FROM_HERE);
-    NotifyTaskDone(fileapi::SYNC_STATUS_OK, token.Pass());
-    return;
-  }
+  has_unfetched_remote_change_ = false;
 
   DVLOG(1) << "FetchChangesForIncrementalSync (start_changestamp:"
            << (largest_fetched_changestamp_ + 1) << ")";
 
   sync_client_->ListChanges(
       largest_fetched_changestamp_ + 1,
       base::Bind(&DriveFileSyncService::DidFetchChangesForIncrementalSync,
                  AsWeakPtr(), base::Passed(&token), false));
 }
 
@@ skipping 146 matching lines @@
 fileapi::SyncStatusCode
 DriveFileSyncService::GDataErrorCodeToSyncStatusCodeWrapper(
     google_apis::GDataErrorCode error) const {
   fileapi::SyncStatusCode status = GDataErrorCodeToSyncStatusCode(error);
   if (status != fileapi::SYNC_STATUS_OK && !sync_client_->IsAuthenticated())
     return fileapi::SYNC_STATUS_AUTHENTICATION_FAILED;
   return status;
 }
 
 }  // namespace sync_file_system