diff options
10 files changed, 51 insertions, 113 deletions
diff --git a/chrome/browser/chromeos/drive/debug_info_collector.cc b/chrome/browser/chromeos/drive/debug_info_collector.cc index 6504f50..2705828 100644 --- a/chrome/browser/chromeos/drive/debug_info_collector.cc +++ b/chrome/browser/chromeos/drive/debug_info_collector.cc @@ -16,11 +16,15 @@ namespace drive { namespace { void IterateFileCacheInternal( - internal::FileCache* file_cache, + internal::ResourceMetadata* metadata, const DebugInfoCollector::IterateFileCacheCallback& iteration_callback) { - scoped_ptr<internal::FileCache::Iterator> it = file_cache->GetIterator(); - for (; !it->IsAtEnd(); it->Advance()) - iteration_callback.Run(it->GetID(), it->GetValue()); + scoped_ptr<internal::ResourceMetadata::Iterator> it = metadata->GetIterator(); + for (; !it->IsAtEnd(); it->Advance()) { + if (it->GetValue().file_specific_info().has_cache_state()) { + iteration_callback.Run(it->GetID(), + it->GetValue().file_specific_info().cache_state()); + } + } DCHECK(!it->HasError()); } @@ -48,15 +52,12 @@ void RunReadDirectoryCallback( } // namespace DebugInfoCollector::DebugInfoCollector( - internal::FileCache* file_cache, internal::ResourceMetadata* metadata, FileSystemInterface* file_system, base::SequencedTaskRunner* blocking_task_runner) - : file_cache_(file_cache), - metadata_(metadata), + : metadata_(metadata), file_system_(file_system), blocking_task_runner_(blocking_task_runner) { - DCHECK(file_cache_); DCHECK(metadata_); DCHECK(file_system_); } @@ -110,7 +111,7 @@ void DebugInfoCollector::IterateFileCache( blocking_task_runner_->PostTaskAndReply( FROM_HERE, base::Bind(&IterateFileCacheInternal, - file_cache_, + metadata_, google_apis::CreateRelayCallback(iteration_callback)), completion_callback); } diff --git a/chrome/browser/chromeos/drive/debug_info_collector.h b/chrome/browser/chromeos/drive/debug_info_collector.h index d4492d0..65aded9 100644 --- a/chrome/browser/chromeos/drive/debug_info_collector.h +++ b/chrome/browser/chromeos/drive/debug_info_collector.h @@ -7,7 +7,6 @@ #include "base/basictypes.h" #include "base/callback_forward.h" -#include "chrome/browser/chromeos/drive/file_cache.h" #include "chrome/browser/chromeos/drive/file_system_interface.h" namespace drive { @@ -27,8 +26,7 @@ class DebugInfoCollector { const FileCacheEntry& cache_entry)> IterateFileCacheCallback; - DebugInfoCollector(internal::FileCache* file_cache, - internal::ResourceMetadata* metadata, + DebugInfoCollector(internal::ResourceMetadata* metadata, FileSystemInterface* file_system, base::SequencedTaskRunner* blocking_task_runner); ~DebugInfoCollector(); @@ -55,7 +53,6 @@ class DebugInfoCollector { void GetMetadata(const GetFilesystemMetadataCallback& callback); private: - internal::FileCache* file_cache_; // Not owned. internal::ResourceMetadata* metadata_; // No owned. FileSystemInterface* file_system_; // Not owned. scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_; diff --git a/chrome/browser/chromeos/drive/drive_integration_service.cc b/chrome/browser/chromeos/drive/drive_integration_service.cc index 152d9b7..9ea18aa 100644 --- a/chrome/browser/chromeos/drive/drive_integration_service.cc +++ b/chrome/browser/chromeos/drive/drive_integration_service.cc @@ -272,8 +272,7 @@ DriveIntegrationService::DriveIntegrationService( cache_root_directory_.Append(kTemporaryFileDirectory))); download_handler_.reset(new DownloadHandler(file_system())); debug_info_collector_.reset(new DebugInfoCollector( - cache_.get(), resource_metadata_.get(), file_system(), - blocking_task_runner_.get())); + resource_metadata_.get(), file_system(), blocking_task_runner_.get())); if (preference_watcher) { preference_watcher_.reset(preference_watcher); diff --git a/chrome/browser/chromeos/drive/file_cache.cc b/chrome/browser/chromeos/drive/file_cache.cc index 98a54ba..2cc936c 100644 --- a/chrome/browser/chromeos/drive/file_cache.cc +++ b/chrome/browser/chromeos/drive/file_cache.cc @@ -78,11 +78,6 @@ FileError FileCache::GetCacheEntry(const std::string& id, return storage_->GetCacheEntry(id, entry); } -scoped_ptr<FileCache::Iterator> FileCache::GetIterator() { - AssertOnSequencedWorkerPool(); - return storage_->GetCacheEntryIterator(); -} - bool FileCache::FreeDiskSpaceIfNeededFor(int64 num_bytes) { AssertOnSequencedWorkerPool(); diff --git a/chrome/browser/chromeos/drive/file_cache.h b/chrome/browser/chromeos/drive/file_cache.h index a39e840..2a18172 100644 --- a/chrome/browser/chromeos/drive/file_cache.h +++ b/chrome/browser/chromeos/drive/file_cache.h @@ -45,8 +45,6 @@ class FileCache { FILE_OPERATION_COPY, }; - typedef ResourceMetadataStorage::CacheEntryIterator Iterator; - // |cache_file_directory| stores cached files. // // |blocking_task_runner| indicates the blocking worker pool for cache @@ -72,9 +70,6 @@ class FileCache { // entry exists in cache map. FileError GetCacheEntry(const std::string& id, FileCacheEntry* entry); - // Returns an object to iterate over entries. - scoped_ptr<Iterator> GetIterator(); - // Frees up disk space to store a file with |num_bytes| size content, while // keeping cryptohome::kMinFreeSpaceInBytes bytes on the disk, if needed. // Returns true if we successfully manage to have enough space, otherwise diff --git a/chrome/browser/chromeos/drive/file_cache_unittest.cc b/chrome/browser/chromeos/drive/file_cache_unittest.cc index 92a1a60..633552e 100644 --- a/chrome/browser/chromeos/drive/file_cache_unittest.cc +++ b/chrome/browser/chromeos/drive/file_cache_unittest.cc @@ -119,31 +119,6 @@ TEST_F(FileCacheTest, RecoverFilesFromCacheDirectory) { dest_directory.AppendASCII("image00000003.png"))); } -TEST_F(FileCacheTest, Iterator) { - base::FilePath src_file; - ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir_.path(), &src_file)); - - // Prepare entries. - std::map<std::string, std::string> md5s; - md5s["id1"] = "md5-1"; - md5s["id2"] = "md5-2"; - md5s["id3"] = "md5-3"; - md5s["id4"] = "md5-4"; - for (std::map<std::string, std::string>::iterator it = md5s.begin(); - it != md5s.end(); ++it) { - EXPECT_EQ(FILE_ERROR_OK, cache_->Store( - it->first, it->second, src_file, FileCache::FILE_OPERATION_COPY)); - } - - // Iterate. - std::map<std::string, std::string> result; - scoped_ptr<FileCache::Iterator> it = cache_->GetIterator(); - for (; !it->IsAtEnd(); it->Advance()) - result[it->GetID()] = it->GetValue().md5(); - EXPECT_EQ(md5s, result); - EXPECT_FALSE(it->HasError()); -} - TEST_F(FileCacheTest, FreeDiskSpaceIfNeededFor) { base::FilePath src_file; ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir_.path(), &src_file)); diff --git a/chrome/browser/chromeos/drive/remove_stale_cache_files.cc b/chrome/browser/chromeos/drive/remove_stale_cache_files.cc index 56296ca0..533e9fb 100644 --- a/chrome/browser/chromeos/drive/remove_stale_cache_files.cc +++ b/chrome/browser/chromeos/drive/remove_stale_cache_files.cc @@ -4,10 +4,6 @@ #include "chrome/browser/chromeos/drive/remove_stale_cache_files.h" -#include <string> -#include <vector> - -#include "base/bind.h" #include "base/logging.h" #include "chrome/browser/chromeos/drive/drive.pb.h" #include "chrome/browser/chromeos/drive/file_cache.h" @@ -18,17 +14,14 @@ namespace internal { void RemoveStaleCacheFiles(FileCache* cache, ResourceMetadata* resource_metadata) { - std::vector<std::string> resource_ids_to_be_removed; - - scoped_ptr<FileCache::Iterator> it = cache->GetIterator(); + scoped_ptr<ResourceMetadata::Iterator> it = resource_metadata->GetIterator(); for (; !it->IsAtEnd(); it->Advance()) { - ResourceEntry entry; - FileError error = resource_metadata->GetResourceEntryById(it->GetID(), - &entry); - // Stale = the entry is not found, or not dirty but the MD5 does not match. - if (error != FILE_ERROR_OK || - (!it->GetValue().is_dirty() && - it->GetValue().md5() != entry.file_specific_info().md5())) { + const ResourceEntry& entry = it->GetValue(); + const FileCacheEntry& cache_state = + entry.file_specific_info().cache_state(); + // Stale = not dirty but the MD5 does not match. + if (!cache_state.is_dirty() && + cache_state.md5() != entry.file_specific_info().md5()) { FileError error = cache->Remove(it->GetID()); LOG_IF(WARNING, error != FILE_ERROR_OK) << "Failed to remove a stale cache file. resource_id: " diff --git a/chrome/browser/chromeos/drive/remove_stale_cache_files.h b/chrome/browser/chromeos/drive/remove_stale_cache_files.h index eba4bae..4e5fddc 100644 --- a/chrome/browser/chromeos/drive/remove_stale_cache_files.h +++ b/chrome/browser/chromeos/drive/remove_stale_cache_files.h @@ -11,7 +11,7 @@ namespace internal { class FileCache; class ResourceMetadata; -// Removes files from |cache| which are not present in |resource_metadata|. +// Removes files from |cache| which are not dirty but the MD5 is obsolete. // Must be run on the same task runner as |cache| and |resource_metadata| use. void RemoveStaleCacheFiles(FileCache* cache, ResourceMetadata* resource_metadata); diff --git a/chrome/browser/chromeos/drive/remove_stale_cache_files_unittest.cc b/chrome/browser/chromeos/drive/remove_stale_cache_files_unittest.cc index 4b6a420..f96693a 100644 --- a/chrome/browser/chromeos/drive/remove_stale_cache_files_unittest.cc +++ b/chrome/browser/chromeos/drive/remove_stale_cache_files_unittest.cc @@ -8,7 +8,6 @@ #include "base/files/file_path.h" #include "base/files/scoped_temp_dir.h" #include "base/memory/scoped_ptr.h" -#include "base/run_loop.h" #include "chrome/browser/chromeos/drive/drive.pb.h" #include "chrome/browser/chromeos/drive/fake_free_disk_space_getter.h" #include "chrome/browser/chromeos/drive/file_system_util.h" @@ -60,65 +59,54 @@ class RemoveStaleCacheFilesTest : public testing::Test { TEST_F(RemoveStaleCacheFilesTest, RemoveStaleCacheFiles) { base::FilePath dummy_file; ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir_.path(), &dummy_file)); - std::string local_id("pdf:1a2b3c"); - std::string md5("abcdef0123456789"); + std::string md5_metadata("abcdef0123456789"), md5_cache("ABCDEF9876543210"); // Create a stale cache file. + ResourceEntry entry; + std::string local_id; + entry.mutable_file_specific_info()->set_md5(md5_metadata); + entry.set_parent_local_id(util::kDriveGrandRootLocalId); + entry.set_title("File.txt"); + EXPECT_EQ(FILE_ERROR_OK, resource_metadata_->AddEntry(entry, &local_id)); + EXPECT_EQ(FILE_ERROR_OK, - cache_->Store(local_id, md5, dummy_file, + cache_->Store(local_id, md5_cache, dummy_file, FileCache::FILE_OPERATION_COPY)); - // Verify that the cache entry exists. - FileCacheEntry cache_entry; - EXPECT_EQ(FILE_ERROR_OK, cache_->GetCacheEntry(local_id, &cache_entry)); - - ResourceEntry entry; - EXPECT_EQ(FILE_ERROR_NOT_FOUND, - resource_metadata_->GetResourceEntryById(local_id, &entry)); - // Remove stale cache files. RemoveStaleCacheFiles(cache_.get(), resource_metadata_.get()); - // Verify that the cache entry is deleted. - EXPECT_EQ(FILE_ERROR_NOT_FOUND, - cache_->GetCacheEntry(local_id, &cache_entry)); + // Verify that the cache is deleted. + EXPECT_EQ(FILE_ERROR_OK, + resource_metadata_->GetResourceEntryById(local_id, &entry)); + EXPECT_FALSE(entry.file_specific_info().cache_state().is_present()); } TEST_F(RemoveStaleCacheFilesTest, DirtyCacheFiles) { base::FilePath dummy_file; ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir_.path(), &dummy_file)); - // Dirty and deleted (= absent in resource_metada) cache entry. - std::string local_id_1("file:1"); - EXPECT_EQ(FILE_ERROR_OK, - cache_->Store(local_id_1, std::string(), dummy_file, - FileCache::FILE_OPERATION_COPY)); - - // Dirty and mismatching-MD5 entry. - std::string md5_2_cache("0123456789abcdef"); - std::string md5_2_metadata("abcdef0123456789"); + // Dirty entry. + std::string md5_metadata("abcdef0123456789"); ResourceEntry entry; - std::string local_id_2; - entry.set_resource_id("resource_id"); - entry.mutable_file_specific_info()->set_md5(md5_2_metadata); + std::string local_id; + entry.mutable_file_specific_info()->set_md5(md5_metadata); entry.set_parent_local_id(util::kDriveGrandRootLocalId); entry.set_title("file.txt"); - resource_metadata_->AddEntry(entry, &local_id_2); + EXPECT_EQ(FILE_ERROR_OK, resource_metadata_->AddEntry(entry, &local_id)); EXPECT_EQ(FILE_ERROR_OK, - cache_->Store(local_id_2, std::string(), dummy_file, + cache_->Store(local_id, std::string(), dummy_file, FileCache::FILE_OPERATION_COPY)); // Remove stale cache files. RemoveStaleCacheFiles(cache_.get(), resource_metadata_.get()); - // Dirty cache should be removed if and only if the entry does not exist in - // resource_metadata. - FileCacheEntry cache_entry; - EXPECT_EQ(FILE_ERROR_NOT_FOUND, - cache_->GetCacheEntry(local_id_1, &cache_entry)); - EXPECT_EQ(FILE_ERROR_OK, cache_->GetCacheEntry(local_id_2, &cache_entry)); + // Dirty cache should not be removed even though its MD5 doesn't match. + EXPECT_EQ(FILE_ERROR_OK, + resource_metadata_->GetResourceEntryById(local_id, &entry)); + EXPECT_TRUE(entry.file_specific_info().cache_state().is_present()); } } // namespace internal diff --git a/chrome/browser/chromeos/drive/sync_client.cc b/chrome/browser/chromeos/drive/sync_client.cc index 23ba95e..a785201 100644 --- a/chrome/browser/chromeos/drive/sync_client.cc +++ b/chrome/browser/chromeos/drive/sync_client.cc @@ -88,28 +88,23 @@ void CollectBacklog(ResourceMetadata* metadata, void CheckExistingPinnedFiles(ResourceMetadata* metadata, FileCache* cache, std::vector<std::string>* local_ids) { - scoped_ptr<FileCache::Iterator> it = cache->GetIterator(); + scoped_ptr<ResourceMetadata::Iterator> it = metadata->GetIterator(); for (; !it->IsAtEnd(); it->Advance()) { - const FileCacheEntry& cache_entry = it->GetValue(); + const ResourceEntry& entry = it->GetValue(); + const FileCacheEntry& cache_state = + entry.file_specific_info().cache_state(); const std::string& local_id = it->GetID(); - if (!cache_entry.is_pinned() || !cache_entry.is_present()) - continue; - - ResourceEntry entry; - FileError error = metadata->GetResourceEntryById(local_id, &entry); - if (error != FILE_ERROR_OK) { - LOG(WARNING) << "Entry not found: " << local_id; + if (!cache_state.is_pinned() || !cache_state.is_present()) continue; - } // If MD5s don't match, it indicates the local cache file is stale, unless // the file is dirty (the MD5 is "local"). We should never re-fetch the // file when we have a locally modified version. - if (entry.file_specific_info().md5() == cache_entry.md5() || - cache_entry.is_dirty()) + if (entry.file_specific_info().md5() == cache_state.md5() || + cache_state.is_dirty()) continue; - error = cache->Remove(local_id); + FileError error = cache->Remove(local_id); if (error != FILE_ERROR_OK) { LOG(WARNING) << "Failed to remove cache entry: " << local_id; continue; |