Drive: Removes unused cache files after the initial feed fetch.

chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc

Index: chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc b/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
index c9413b99475da0afbf240e0cf047ec868e4ce826..264c0c0b3a7dc0b3cd615f05bf67f2b99d22a0c5 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
+++ b/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
+#include "base/run_loop.h"

[satorux1, 2012/08/19 05:24:59]

please remove. see below.

[yoshiki, 2012/08/20 02:39:37]

Done.

 #include "base/scoped_temp_dir.h"
 #include "base/stringprintf.h"
 #include "base/threading/sequenced_worker_pool.h"
@@ -73,6 +74,12 @@ void DriveSearchCallback(
   message_loop->Quit();
 }
 
+void VerifyCacheFileState(GDataFileError error,

[satorux1, 2012/08/16 11:03:53]

function comment is missing.

[yoshiki, 2012/08/17 03:49:59]

Done.

+                          const std::string& resource_id,
+                          const std::string& md5) {
+  EXPECT_EQ(GDATA_FILE_OK, error);
+}
+
 // Action used to set mock expectations for
 // DocumentsService::GetDocumentEntry().
 ACTION_P2(MockGetDocumentEntry, status, value) {
@@ -2622,4 +2629,45 @@ TEST_F(GDataFileSystemTest, OpenAndCloseFile) {
   EXPECT_EQ(GDATA_FILE_ERROR_NOT_FOUND, callback_helper_->last_error_);
 }
 
+TEST_F(GDataFileSystemTest, RemoveStaleCacheFiles) {
+  FilePath dummy_file = GetTestFilePath("root_feed.json");
+  std::string resource_id("pdf:1a2b");
+  std::string md5("abcdef0123456789");
+
+  // Create a stale cache file.
+  cache_->StoreOnUIThread(resource_id, md5, dummy_file,
+                          GDataCache::FILE_OPERATION_COPY,
+                          base::Bind(&gdata::VerifyCacheFileState));
+  test_util::RunBlockingPoolTask();
+
+  // Verify that the cache file exists.
+  FilePath path = cache_->GetCacheFilePath(resource_id,
+                                           md5,
+                                           GDataCache::CACHE_TYPE_TMP,
+                                           GDataCache::CACHED_FILE_FROM_SERVER);
+  EXPECT_TRUE(file_util::PathExists(path));
+
+  // Verify that the corresponding file entry doesn't exist.
+  EXPECT_CALL(*mock_doc_service_, GetAccountMetadata(_)).Times(1);
+  EXPECT_CALL(*mock_doc_service_, GetDocuments(Eq(GURL()), _, "", _, _))
+      .Times(1);
+  EXPECT_CALL(*mock_webapps_registry_, UpdateFromFeed(NotNull())).Times(1);
+
+  scoped_ptr<GDataEntryProto> entry = GetEntryInfoByPathSync(path);
+  ASSERT_FALSE(entry.get());
+
+  // Load a root feed.
+  LoadRootFeedDocument("root_feed.json");
+
+  base::RunLoop run_loop;
+  run_loop.RunUntilIdle();

[satorux1, 2012/08/16 11:03:53]

this is new to me. why is this used? shouldn't we use 
test_util::RunBlockingPoolTask() instead to wait for the feed to be loaded?

[yoshiki, 2012/08/17 03:49:59]

We don't need RunBlockingPoolTask(), because in this unit_tests, loading feeds
(including parsing) is done synchronously.

This waits the completion of a series of UI-thread tasks to remove stale cache
files. This is completely same as message_loop_.QuitWhenIdle(). But the comment
of MessageLoop::QuitWhenIdle() suggests that QuitWhenIdle() is deprecated and we
should use RunLoop instead.

[satorux1, 2012/08/17 10:38:41]

I'm  more confused. If feed loading is done synchronously, why do we need to
quit the message loop? why do we need this?

[yoshiki, 2012/08/18 03:31:51]

Sorry for confusion, my comment has a wrong method name. Please
s/QuitWhenIdle/RunUntilIdle/g in my comment.

MessageLoop::RunUntilIdle() is deprecated, according to the comment.
http://code.google.com/searchframe#OAMlx_jo-ck/src/base/message_loop.h&exact_...

[satorux1, 2012/08/18 11:33:16]

Sorry, but I'm still confused. Could you explain why this is needed? If
LoadRootFeedDocument() is synchronous, we don't have to deal with a message
loop, right?

[satorux1, 2012/08/19 05:24:59]

I guess the synchronization is needed to let the StaleCacheFilesRemove do the
job. Is this correct?

If so, please change it to test_util::RunBlockingPoolTask().  As of now, we are
using in-memory cache implementation for tests (crrev.com/151013), but can be
changed in the future.

Anyway, it's not obvious why we need synchronization here, so please add a
comment like:

// Wait for StaleCacheFilesRemover to finish cleaning up the stale file.
test_util::RunBlockingPoolTask();

[yoshiki, 2012/08/20 02:39:37]

Right.
Done. Thanks! 

+
+  // Verify that the cache file is deleted.
+  path = cache_->GetCacheFilePath(resource_id,
+                                  md5,
+                                  GDataCache::CACHE_TYPE_TMP,
+                                  GDataCache::CACHED_FILE_FROM_SERVER);
+  EXPECT_FALSE(file_util::PathExists(path));
+}
+
 }   // namespace gdata