Implement polling part of DriveFileSyncService

chrome/browser/sync_file_system/drive_file_sync_service.cc

Index: chrome/browser/sync_file_system/drive_file_sync_service.cc
diff --git a/chrome/browser/sync_file_system/drive_file_sync_service.cc b/chrome/browser/sync_file_system/drive_file_sync_service.cc
index 194a078e88e5e003b9e504aab53418755b9f8e8b..121400ff1afdde10e42a95539bd75f0666ce61c8 100644
--- a/chrome/browser/sync_file_system/drive_file_sync_service.cc
+++ b/chrome/browser/sync_file_system/drive_file_sync_service.cc
@@ -35,6 +35,11 @@ namespace {
 const FilePath::CharType kTempDirName[] = FILE_PATH_LITERAL("tmp");
 const FilePath::CharType kSyncFileSystemDir[] =
     FILE_PATH_LITERAL("Sync FileSystem");
+const char kResourceLinkPrefix[] =
+    "https://docs.google.com/feeds/default/private/full/";

[kinuko, 2012/11/29 11:40:46]

Can we use GDataWapiUrlGenerator::GenerateDocumentListRootUrl()?

(And I think the URL should be given by DriveFileSyncClient and use the same
base URL?)

[tzik, 2012/12/03 10:00:08]

Done

+const int64 kMinimumPollingDelaySeconds = 10;
+const int64 kMaximumPollingDelaySeconds = 60 * 60;  // 1 hour
+const int64 kDelayMultiplier = 2;
 
 bool CreateTemporaryFile(const FilePath& dir_path, FilePath* temp_file) {
   return file_util::CreateDirectory(dir_path) &&
@@ -60,6 +65,11 @@ void DidUpdateConflictState(fileapi::SyncStatusCode status) {
 
 void EmptyStatusCallback(fileapi::SyncStatusCode code) {}
 
+

[kinuko, 2012/11/29 11:40:46]

nit: extra empty line

[tzik, 2012/12/03 10:00:08]

Done.

+GURL ResourceIdToResourceLink(const std::string& resource_id) {
+  return GURL(kResourceLinkPrefix + net::EscapePath(resource_id));
+}
+
 }  // namespace
 
 const char DriveFileSyncService::kServiceName[] = "drive";
@@ -202,7 +212,9 @@ bool DriveFileSyncService::RemoteChangeComparator::operator()(
 DriveFileSyncService::DriveFileSyncService(Profile* profile)
     : last_operation_status_(fileapi::SYNC_STATUS_OK),
       state_(REMOTE_SERVICE_OK),
-      largest_changestamp_(0),
+      largest_synced_changestamp_(0),

[kinuko, 2012/11/29 11:47:43]

We're not using this?

[tzik, 2012/12/03 10:00:08]

Done. Removed.

+      largest_fetched_changestamp_(0),
+      polling_delay_(base::TimeDelta::FromSeconds(kMinimumPollingDelaySeconds)),
       weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   temporary_file_dir_ =
       profile->GetPath().Append(kSyncFileSystemDir).Append(kTempDirName);
@@ -503,7 +515,8 @@ DriveFileSyncService::DriveFileSyncService(
     scoped_ptr<DriveMetadataStore> metadata_store)
     : last_operation_status_(fileapi::SYNC_STATUS_OK),
       state_(REMOTE_SERVICE_OK),
-      largest_changestamp_(0),
+      largest_synced_changestamp_(0),
+      largest_fetched_changestamp_(0),
       weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   temporary_file_dir_ = base_dir.Append(kTempDirName);
 
@@ -532,7 +545,6 @@ void DriveFileSyncService::NotifyTaskDone(fileapi::SyncStatusCode status,
   last_operation_status_ = status;
   token_ = token.Pass();
 
-  RemoteServiceState old_state = state_;
   if (token_->task_type() != TASK_TYPE_NONE) {
     DVLOG(1) << "NotifyTaskDone: " << token_->description()
              << ": finished with status=" << status
@@ -557,7 +569,7 @@ void DriveFileSyncService::NotifyTaskDone(fileapi::SyncStatusCode status,
     return;
   }
 
-  if (state_ != REMOTE_SERVICE_OK || old_state == state_)
+  if (state_ != REMOTE_SERVICE_OK)
     return;
 
   // If the state has become OK and we have any pending batch sync origins
@@ -567,7 +579,14 @@ void DriveFileSyncService::NotifyTaskDone(fileapi::SyncStatusCode status,
     pending_batch_sync_origins_.erase(pending_batch_sync_origins_.begin());
     std::string resource_id = metadata_store_->GetResourceIdForOrigin(origin);
     StartBatchSyncForOrigin(origin, resource_id);
+    return;
   }
+
+  FOR_EACH_OBSERVER(Observer, observers_,

[kinuko, 2012/11/29 11:40:46]

Can you comment why we do this here?

[tzik, 2012/12/03 10:00:08]

Done.

+                    OnRemoteChangeAvailable(pending_changes_.size()));
+
+  if (pending_changes_.empty())
+    SchedulePolling();
 }
 
 void DriveFileSyncService::UpdateServiceState() {
@@ -629,6 +648,9 @@ void DriveFileSyncService::DidInitializeMetadataStore(
     return;
   }
 
+  largest_synced_changestamp_ = metadata_store_->GetLargestChangeStamp();
+  largest_fetched_changestamp_ = metadata_store_->GetLargestChangeStamp();
+
   if (metadata_store_->sync_root_directory().empty()) {
     GetSyncRootDirectory(token.Pass(), base::Bind(&EmptyStatusCallback));
     return;
@@ -763,7 +785,7 @@ void DriveFileSyncService::DidGetDirectoryContentForBatchSync(
   typedef ScopedVector<google_apis::DocumentEntry>::const_iterator iterator;
   for (iterator itr = feed->entries().begin();
        itr != feed->entries().end(); ++itr) {
-    AppendNewRemoteChange(origin, *itr, largest_changestamp,
+    AppendNewRemoteChange(origin, **itr, largest_changestamp,
                           REMOTE_SYNC_TYPE_BATCH);
   }
 
@@ -1002,6 +1024,7 @@ void DriveFileSyncService::DidPrepareForProcessRemoteChange(
   bool missing_db_entry = (status != fileapi::SYNC_STATUS_OK);
   if (missing_db_entry) {
     param->drive_metadata.set_resource_id(param->remote_change.resource_id);
+    param->drive_metadata.set_md5_checksum(std::string());
     param->drive_metadata.set_conflicted(false);
   }
   bool missing_local_file =
@@ -1081,8 +1104,10 @@ void DriveFileSyncService::DidPrepareForProcessRemoteChange(
     }
     DCHECK(!missing_local_file);
     param->operation_type = fileapi::SYNC_OPERATION_DELETE;
+
+    const fileapi::FileChange& file_change = param->remote_change.change;
     param->processor->ApplyRemoteChange(
-        param->remote_change.change, FilePath(), url,
+        file_change, FilePath(), url,
         base::Bind(&DriveFileSyncService::DidApplyRemoteChange, AsWeakPtr(),
                    base::Passed(&param)));
     return;
@@ -1121,13 +1146,11 @@ void DriveFileSyncService::DidGetTemporaryFileForDownload(
     return;
   }
 
-  DriveMetadata metadata;
-  metadata_store_->ReadEntry(param->remote_change.url, &metadata);
-
   const FilePath& temporary_file_path = param->temporary_file_path;
   std::string resource_id = param->remote_change.resource_id;
+  const DriveMetadata& drive_metadata = param->drive_metadata;
   sync_client_->DownloadFile(
-      resource_id, metadata.md5_checksum(),
+      resource_id, drive_metadata.md5_checksum(),
       temporary_file_path,
       base::Bind(&DriveFileSyncService::DidDownloadFile,
                  AsWeakPtr(), base::Passed(&param)));
@@ -1149,7 +1172,7 @@ void DriveFileSyncService::DidDownloadFile(
     return;
   }
 
-  param->md5_checksum = md5_checksum;
+  param->drive_metadata.set_md5_checksum(md5_checksum);
   const fileapi::FileChange& change = param->remote_change.change;
   const FilePath& temporary_file_path = param->temporary_file_path;
   const fileapi::FileSystemURL& url = param->remote_change.url;
@@ -1173,13 +1196,12 @@ void DriveFileSyncService::DidApplyRemoteChange(
     return;
   }
 
-  DriveMetadata metadata;
-  metadata.set_resource_id(param->remote_change.resource_id);
-  metadata.set_md5_checksum(param->md5_checksum);
-  metadata.set_conflicted(false);
+  const DriveMetadata& drive_metadata = param->drive_metadata;
+  param->drive_metadata.set_resource_id(param->remote_change.resource_id);
+  param->drive_metadata.set_conflicted(false);
 
   metadata_store_->UpdateEntry(
-      url, metadata,
+      url, drive_metadata,
       base::Bind(&DriveFileSyncService::CompleteRemoteSync,
                  AsWeakPtr(), base::Passed(&param)));
 }
@@ -1206,6 +1228,9 @@ void DriveFileSyncService::CompleteRemoteSync(
   GURL origin = param->remote_change.url.origin();
   if (metadata_store_->IsIncrementalSyncOrigin(origin)) {
     int64 changestamp = param->remote_change.changestamp;
+    largest_synced_changestamp_ = changestamp;
+    DCHECK_GE(largest_fetched_changestamp_, largest_synced_changestamp_);
+
     metadata_store_->SetLargestChangeStamp(
         changestamp,
         base::Bind(&DriveFileSyncService::FinalizeRemoteSync,
@@ -1242,11 +1267,11 @@ void DriveFileSyncService::FinalizeRemoteSync(
 
 void DriveFileSyncService::AppendNewRemoteChange(
     const GURL& origin,
-    google_apis::DocumentEntry* entry,
+    const google_apis::DocumentEntry& entry,
     int64 changestamp,
     RemoteSyncType sync_type) {
   // TODO(tzik): Normalize the path here.
-  FilePath path = FilePath::FromUTF8Unsafe(UTF16ToUTF8(entry->title()));
+  FilePath path = FilePath::FromUTF8Unsafe(UTF16ToUTF8(entry.title()));
 
   PathToChange* path_to_change = &url_to_change_[origin];
   PathToChange::iterator found = path_to_change->find(path);
@@ -1261,12 +1286,12 @@ void DriveFileSyncService::AppendNewRemoteChange(
 
   fileapi::FileChange::ChangeType change_type;
   fileapi::SyncFileType file_type;
-  if (entry->deleted()) {
+  if (entry.deleted()) {
     change_type = fileapi::FileChange::FILE_CHANGE_DELETE;
     file_type = fileapi::SYNC_FILE_TYPE_UNKNOWN;
   } else {
     change_type = fileapi::FileChange::FILE_CHANGE_ADD_OR_UPDATE;
-    if (entry->kind() == google_apis::ENTRY_KIND_FOLDER)
+    if (entry.kind() == google_apis::ENTRY_KIND_FOLDER)

[nhiroki, 2012/11/30 06:23:53]

You may want to use "entry.is_folder()".

[tzik, 2012/12/03 10:00:08]

Done.

       file_type = fileapi::SYNC_FILE_TYPE_DIRECTORY;
     else
       file_type = fileapi::SYNC_FILE_TYPE_FILE;
@@ -1279,7 +1304,7 @@ void DriveFileSyncService::AppendNewRemoteChange(
   DCHECK(inserted_to_queue.second);
 
   (*path_to_change)[path] = RemoteChange(
-      changestamp, entry->resource_id(), url, file_change,
+      changestamp, entry.resource_id(), url, file_change,
       inserted_to_queue.first);
 }
 
@@ -1294,19 +1319,22 @@ void DriveFileSyncService::CancelRemoteChange(
   if (found_change == path_to_change->end())
     return;
 
+  int64 changestamp = found_change->second.changestamp;
   pending_changes_.erase(found_change->second.position_in_queue);
   path_to_change->erase(found_change);
   if (path_to_change->empty())
     url_to_change_.erase(found_origin);
 
-  MaybeMarkAsIncrementalSyncOrigin(url.origin());
-}
-
-void DriveFileSyncService::MaybeMarkAsIncrementalSyncOrigin(
-    const GURL& origin) {
-  if (metadata_store_->IsBatchSyncOrigin(origin) &&
-      !ContainsKey(url_to_change_, origin))
-    metadata_store_->MoveBatchSyncOriginToIncremental(origin);
+  if (metadata_store_->IsBatchSyncOrigin(url.origin()) &&
+      !ContainsKey(url_to_change_, url.origin())) {
+    if (metadata_store_->incremental_sync_origins().empty()) {
+      largest_synced_changestamp_ = changestamp;
+      largest_fetched_changestamp_ = changestamp;
+      metadata_store_->SetLargestChangeStamp(changestamp,
+                                             base::Bind(&EmptyStatusCallback));

[kinuko, 2012/11/29 11:40:46]

Let's change this part per discussion!

[tzik, 2012/12/03 10:00:08]

Done.

+    }
+    metadata_store_->MoveBatchSyncOriginToIncremental(url.origin());
+  }
 }
 
 bool DriveFileSyncService::GetPendingChangeForFileSystemURL(
@@ -1324,4 +1352,99 @@ bool DriveFileSyncService::GetPendingChangeForFileSystemURL(
   return true;
 }
 
+void DriveFileSyncService::FetchChangesForIncrementalSync() {
+  scoped_ptr<TaskToken> token(GetToken(FROM_HERE, TASK_TYPE_DRIVE,
+                                       "Fetching remote change list"));
+  if (!token) {
+    pending_tasks_.push_back(base::Bind(
+        &DriveFileSyncService::FetchChangesForIncrementalSync, AsWeakPtr()));
+    return;
+  }
+
+  if (metadata_store_->incremental_sync_origins().empty()) {
+    NotifyTaskDone(fileapi::SYNC_STATUS_OK, token.Pass());
+    return;
+  }
+
+  sync_client_->ListChanges(
+      largest_fetched_changestamp_,
+      base::Bind(&DriveFileSyncService::DidFetchChangesForIncrementalSync,
+                 AsWeakPtr(), base::Passed(&token)));
+}
+
+void DriveFileSyncService::DidFetchChangesForIncrementalSync(
+    scoped_ptr<TaskToken> token,
+    google_apis::GDataErrorCode error,
+    scoped_ptr<google_apis::DocumentFeed> changes) {
+  GURL next_feed;
+
+  typedef ScopedVector<google_apis::DocumentEntry>::const_iterator iterator;
+  for (iterator itr = changes->entries().begin();
+       itr != changes->entries().end(); ++itr) {
+    const google_apis::DocumentEntry& entry = **itr;
+    GURL origin;
+    if (!GetOriginForEntry(entry, &origin))
+      continue;
+
+    AppendNewRemoteChange(origin, entry, entry.changestamp(),
+                          REMOTE_SYNC_TYPE_INCREMENTAL);
+  }
+
+  if (changes->GetNextFeedURL(&next_feed)) {
+    sync_client_->ContinueListing(
+        next_feed,
+        base::Bind(&DriveFileSyncService::DidFetchChangesForIncrementalSync,
+                   AsWeakPtr(), base::Passed(&token)));
+  } else {
+    largest_fetched_changestamp_ = changes->largest_changestamp();
+
+    if (changes->start_index() == 0 && changes->entries().empty()) {

[kinuko, 2012/11/29 11:47:43]

nit: can you add a comment: 'If this set of changes is the first feed and its
empty, update the polling delay to wait longer.'

[tzik, 2012/12/03 10:00:08]

Done.

+      base::TimeDelta max_delay(
+          base::TimeDelta::FromSeconds(kMaximumPollingDelaySeconds));
+
+      polling_delay_ *= kDelayMultiplier;
+      if (polling_delay_ >= max_delay)
+        polling_delay_ = max_delay;
+    } else {
+      polling_delay_ = base::TimeDelta::FromSeconds(
+          kMinimumPollingDelaySeconds);
+    }
+
+    NotifyTaskDone(fileapi::SYNC_STATUS_OK, token.Pass());
+  }
+}
+
+bool DriveFileSyncService::GetOriginForEntry(
+    const google_apis::DocumentEntry& entry,
+    GURL* origin_out) {
+  typedef ScopedVector<google_apis::Link>::const_iterator iterator;
+  for (iterator itr = entry.links().begin();
+       itr != entry.links().end(); ++itr) {
+    if ((*itr)->type() != google_apis::Link::LINK_PARENT)
+      continue;
+    GURL origin(UTF16ToUTF8((*itr)->title()));
+    if (!origin.is_valid())
+      continue;
+
+    if (!metadata_store_->IsBatchSyncOrigin(origin) &&
+        !metadata_store_->IsIncrementalSyncOrigin(origin))
+      continue;
+    std::string resource_id(metadata_store_->GetResourceIdForOrigin(origin));
+    if ((*itr)->href() != ResourceIdToResourceLink(resource_id))
+      continue;
+
+    *origin_out = origin;
+    return true;
+  }
+  return false;
+}
+
+void DriveFileSyncService::SchedulePolling() {
+  if (!polling_timer_.IsRunning())
+    polling_timer_.Start(
+        FROM_HERE, polling_delay_,
+        base::Bind(&DriveFileSyncService::FetchChangesForIncrementalSync,
+                   AsWeakPtr()));
+}
+
 }  // namespace sync_file_system