gdata: File info of dirty files should be get from the cached file.

chrome/browser/chromeos/gdata/gdata_file_system.cc

Index: chrome/browser/chromeos/gdata/gdata_file_system.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system.cc b/chrome/browser/chromeos/gdata/gdata_file_system.cc
index f78bff979290c849bd99220d1b4cc870a269f167..11d741e5ec44afee4f37a5bfe63d3baf4a99f64f 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system.cc
+++ b/chrome/browser/chromeos/gdata/gdata_file_system.cc
@@ -592,6 +592,17 @@ void PostBlockingPoolSequencedTaskAndReply(
   DCHECK(posted);
 }
 
+// Helper function for binding |path| to GetEntryInfoWithFilePathCallback and
+// create GetEntryInfoCallback.
+void RunGetEntryInfoWithFilePathCallback(
+    const GetEntryInfoWithFilePathCallback& callback,
+    const FilePath& path,
+    GDataFileError error,
+    scoped_ptr<GDataEntryProto> entry_proto) {
+  if (!callback.is_null())
+    callback.Run(error, path, entry_proto.Pass());
+}
+
 }  // namespace
 
 // GDataFileSystem::GetDocumentsParams struct implementation.
@@ -894,7 +905,10 @@ void GDataFileSystem::GetEntryInfoByEntryOnUIThread(
   if (entry) {
     scoped_ptr<GDataEntryProto> entry_proto(new GDataEntryProto);
     entry->ToProtoFull(entry_proto.get());
-    callback.Run(GDATA_FILE_OK, entry->GetFilePath(), entry_proto.Pass());
+    ReflectLocalModificationToGDataEntryProto(
+        base::Bind(&RunGetEntryInfoWithFilePathCallback,
+                   callback, entry->GetFilePath()),
+        entry_proto.Pass());
   } else {
     callback.Run(GDATA_FILE_ERROR_NOT_FOUND,
                  FilePath(),
@@ -2248,8 +2262,7 @@ void GDataFileSystem::OnGetEntryInfo(const GetEntryInfoCallback& callback,
   scoped_ptr<GDataEntryProto> entry_proto(new GDataEntryProto);
   entry->ToProtoFull(entry_proto.get());
 
-  if (!callback.is_null())
-    callback.Run(GDATA_FILE_OK, entry_proto.Pass());
+  ReflectLocalModificationToGDataEntryProto(callback, entry_proto.Pass());
 }
 
 void GDataFileSystem::ReadDirectoryByPath(
@@ -4117,4 +4130,102 @@ void GDataFileSystem::OnCloseFileFinished(
     callback.Run(result);
 }
 
+void GDataFileSystem::ReflectLocalModificationToGDataEntryProto(

[satorux1, 2012/07/25 19:28:45]

maybe CheckLocalModificationAndRun() would describe what the function does a bit
more nicely.

[kinaba, 2012/07/26 03:17:53]

Done.

+    const GetEntryInfoCallback& callback,
+    scoped_ptr<GDataEntryProto> entry_proto) {

[satorux1, 2012/07/25 19:28:45]

I'd swap these parameters with the new function name.

[kinaba, 2012/07/26 03:17:53]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[satorux1, 2012/07/25 19:28:45]

DCHECK(entry_proto.get()); This should be guaranteed by callers, right?

[kinaba, 2012/07/26 03:17:53]

Yes. Done.

+
+  // For entries that will never be cached, use the original entry info as is.
+  if (!entry_proto.get() || !entry_proto->has_file_specific_info() ||

[satorux1, 2012/07/25 19:28:45]

I'd remove !entry_proto.get()

[kinaba, 2012/07/26 03:17:53]

Done.

+      entry_proto->file_specific_info().file_md5().empty() ||

[satorux1, 2012/07/25 19:28:45]

I think we can remove this line.

[kinaba, 2012/07/26 03:17:53]

Done.

+      entry_proto->file_specific_info().is_hosted_document()) {
+    if (!callback.is_null())
+      callback.Run(GDATA_FILE_OK, entry_proto.Pass());
+    return;
+  }
+
+  // Checks if the file is cached and modified locally.
+  const std::string resource_id = entry_proto->resource_id();
+  const std::string md5 = entry_proto->file_specific_info().file_md5();
+  cache_->GetCacheEntryOnUIThread(
+      resource_id,
+      md5,
+      base::Bind(&GDataFileSystem::OnGetCacheEntryForReflectLocalModification,
+                 ui_weak_ptr_, callback, base::Passed(&entry_proto)));
+}
+
+void GDataFileSystem::OnGetCacheEntryForReflectLocalModification(

[satorux1, 2012/07/25 19:28:45]

ReflectLocalModificationAfterGetCacheEntry() may be easier to read. I started
using this notation recently to implement a set of functions which logically
implement one function. I found it a bit easier to follow.

[kinaba, 2012/07/26 03:17:53]

Done.

+    const GetEntryInfoCallback& callback,
+    scoped_ptr<GDataEntryProto> entry_proto,
+    bool success,
+    const GDataCacheEntry& cache_entry) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // When no dirty cache is found, use the original entry info as is.
+  if (!success || !cache_entry.is_dirty()) {
+    if (!callback.is_null())
+      callback.Run(GDATA_FILE_OK, entry_proto.Pass());
+    return;
+  }
+
+  // Gets the cache file path.
+  const std::string resource_id = entry_proto->resource_id();
+  const std::string md5 = entry_proto->file_specific_info().file_md5();

[satorux1, 2012/07/25 19:28:45]

make them const reference?

[kinaba, 2012/07/26 03:17:53]

Done.

+  cache_->GetFileOnUIThread(
+      resource_id,
+      md5,
+      base::Bind(&GDataFileSystem::OnGetCacheFileForReflectLocalModification,
+                 ui_weak_ptr_, callback, base::Passed(&entry_proto)));
+}
+
+void GDataFileSystem::OnGetCacheFileForReflectLocalModification(
+    const GetEntryInfoCallback& callback,
+    scoped_ptr<GDataEntryProto> entry_proto,
+    GDataFileError error,
+    const std::string& resource_id,
+    const std::string& md5,
+    const FilePath& local_cache_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // When no dirty cache is found, use the original entry info as is.
+  if (error != GDATA_FILE_OK) {
+    if (!callback.is_null())
+      callback.Run(GDATA_FILE_OK, entry_proto.Pass());
+    return;
+  }
+
+  // If the cache is dirty, obtain the file info from the cache file itself.
+  base::PlatformFileInfo* file_info = new base::PlatformFileInfo;
+  bool* get_file_info_result = new bool(false);
+  PostBlockingPoolSequencedTaskAndReply(
+      FROM_HERE,
+      blocking_task_runner_,
+      base::Bind(&GetFileInfoOnBlockingPool,
+                 local_cache_path,
+                 base::Unretained(file_info),
+                 base::Unretained(get_file_info_result)),
+      base::Bind(&GDataFileSystem::OnGetFileInfoForReflectLocalModification,
+                 ui_weak_ptr_,
+                 callback,
+                 base::Passed(&entry_proto),
+                 base::Owned(file_info),
+                 base::Owned(get_file_info_result)));
+}
+
+void GDataFileSystem::OnGetFileInfoForReflectLocalModification(
+    const GetEntryInfoCallback& callback,
+    scoped_ptr<GDataEntryProto> entry_proto,
+    base::PlatformFileInfo* file_info,
+    bool* get_file_info_result) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (*get_file_info_result) {
+    PlatformFileInfoProto entry_file_info;
+    GDataEntry::ConvertPlatformFileInfoToProto(*file_info, &entry_file_info);
+    *entry_proto->mutable_file_info() = entry_file_info;
+  }
+  if (!callback.is_null())
+    callback.Run(GDATA_FILE_OK, entry_proto.Pass());

[satorux1, 2012/07/25 19:28:45]

This looks wrong. The error code should be GDATA_FILE_NOT_FOUND if
*get_file_info_result == false.

[kinaba, 2012/07/26 03:17:53]

Done.

+}
+
 }  // namespace gdata