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

chrome/browser/chromeos/gdata/gdata_file_system.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/chromeos/gdata/gdata_file_system.h"
 
 #include <set>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 574 matching lines @@
     base::SequencedTaskRunner* blocking_task_runner,
     const base::Closure& request_task,
     const base::Closure& reply_task) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   const bool posted = blocking_task_runner->PostTaskAndReply(
       from_here, request_task, reply_task);
   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.
 struct GDataFileSystem::GetDocumentsParams {
   GetDocumentsParams(int start_changestamp,
                      int root_feed_changestamp,
                      std::vector<DocumentFeed*>* feed_list,
                      bool should_fetch_multiple_feeds,
                      const FilePath& search_file_path,
                      const std::string& search_query,
@@ skipping 282 matching lines @@
 }
 
 void GDataFileSystem::GetEntryInfoByEntryOnUIThread(
     const GetEntryInfoWithFilePathCallback& callback,
     GDataEntry* entry) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   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(),
                  scoped_ptr<GDataEntryProto>());
   }
 }
 
 void GDataFileSystem::FindEntryByPathAsyncOnUIThread(
     const FilePath& search_file_path,
     const FindEntryCallback& callback) {
@@ skipping 1333 matching lines @@
   if (error != GDATA_FILE_OK) {
     if (!callback.is_null())
       callback.Run(error, scoped_ptr<GDataEntryProto>());
     return;
   }
   DCHECK(entry);
 
   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(
     const FilePath& file_path,
     const ReadDirectoryCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::IO));
   RunTaskOnUIThread(
       base::Bind(&GDataFileSystem::ReadDirectoryByPathAsyncOnUIThread,
                  ui_weak_ptr_,
@@ skipping 1847 matching lines @@
   // All the invocation of |callback| from operations initiated from CloseFile
   // must go through here. Removes the |file_path| from the remembered set so
   // that subsequent operations can open the file again.
   open_files_.erase(file_path);
 
   // Then invokes the user-supplied callback function.
   if (!callback.is_null())
     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