Skip to content

GitLab

T
tangled
Commit 4e248d14 authored by satorux@chromium.org's avatar satorux@chromium.org
Browse files
Options

gdata: Remove sub_dir_type from CacheEntry for simplicity 

Having this member was awkward, as it's inconsistent
with other states, which are all stored in |cache_state|.

BUG=136390
TEST=out/Release/unit_tests --gtest_filter=GData*

Review URL: https://chromiumcodereview.appspot.com/10698124

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@145933 0039d316-1c4b-4281-b951-d872f2087c98
parent 85537005
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 223 additions and 158 deletions
chrome/browser/chromeos/gdata/gdata_cache.h
View file @ 4e248d14
......@@ -80,11 +80,12 @@ class GDataCache {
// This is used as a bitmask for the cache state.
enum CacheState {
CACHE_STATE_NONE = 0x0,
CACHE_STATE_PINNED = 0x1 << 0,
CACHE_STATE_PRESENT = 0x1 << 1,
CACHE_STATE_DIRTY = 0x1 << 2,
CACHE_STATE_MOUNTED = 0x1 << 3,
CACHE_STATE_NONE = 0x0,
CACHE_STATE_PINNED = 0x1 << 0,
CACHE_STATE_PRESENT = 0x1 << 1,
CACHE_STATE_DIRTY = 0x1 << 2,
CACHE_STATE_MOUNTED = 0x1 << 3,
CACHE_STATE_PERSISTENT = 0x1 << 4,
};
// Enum defining origin of a cached file.
......@@ -120,27 +121,29 @@ class GDataCache {
// Structure to store information of an existing cache file.
struct CacheEntry {
CacheEntry() : sub_dir_type(CACHE_TYPE_META),
cache_state(0) {}
CacheEntry() : cache_state(0) {}
CacheEntry(const std::string& md5,
CacheSubDirectoryType sub_dir_type,
int cache_state)
: md5(md5),
sub_dir_type(sub_dir_type),
cache_state(cache_state) {
}
bool IsPresent() const { return IsCachePresent(cache_state); }
bool IsPinned() const { return IsCachePinned(cache_state); }
bool IsDirty() const { return IsCacheDirty(cache_state); }
bool IsMounted() const { return IsCacheMounted(cache_state); }
bool IsMounted() const { return IsCacheMounted(cache_state); }
bool IsPersistent() const { return IsCachePersistent(cache_state); }
// Returns the type of the sub directory where the cache file is stored.
CacheSubDirectoryType GetSubDirectoryType() const {
return IsPersistent() ? CACHE_TYPE_PERSISTENT : CACHE_TYPE_TMP;
}
// For debugging purposes.
std::string ToString() const;
std::string md5;
CacheSubDirectoryType sub_dir_type;
int cache_state;
};
......@@ -166,6 +169,9 @@ class GDataCache {
static bool IsCacheMounted(int cache_state) {
return cache_state & CACHE_STATE_MOUNTED;
}
static bool IsCachePersistent(int cache_state) {
return cache_state & CACHE_STATE_PERSISTENT;
}
static int SetCachePresent(int cache_state) {
return cache_state |= CACHE_STATE_PRESENT;
}
......@@ -178,6 +184,9 @@ class GDataCache {
static int SetCacheMounted(int cache_state) {
return cache_state |= CACHE_STATE_MOUNTED;
}
static int SetCachePersistent(int cache_state) {
return cache_state |= CACHE_STATE_PERSISTENT;
}
static int ClearCachePresent(int cache_state) {
return cache_state &= ~CACHE_STATE_PRESENT;
}
......@@ -190,6 +199,9 @@ class GDataCache {
static int ClearCacheMounted(int cache_state) {
return cache_state &= ~CACHE_STATE_MOUNTED;
}
static int ClearCachePersistent(int cache_state) {
return cache_state &= ~CACHE_STATE_PERSISTENT;
}
// Returns the sub-directory under gdata cache directory for the given sub
// directory type. Example: <user_profile_dir>/GCache/v1/tmp
......@@ -468,6 +480,13 @@ class GDataCache {
bool* success,
GDataCache::CacheEntry* cache_entry);
// Wrapper around GDataCacheMetadata::UpdateCache(). This function takes
// |sub_dir_type| and modifies |cache_state| per the sub directory type.
void UpdateCacheWithSubDirectoryType(const std::string& resource_id,
const std::string& md5,
CacheSubDirectoryType sub_dir_type,
int cache_state);
// The root directory of the cache (i.e. <user_profile_dir>/GCache/v1).
const FilePath cache_root_path_;
// Paths for all subdirectories of GCache, one for each
......
chrome/browser/chromeos/gdata/gdata_cache_metadata.cc
View file @ 4e248d14
......@@ -184,9 +184,8 @@ void GDataCacheMetadataMap::Initialize(
}
void GDataCacheMetadataMap::UpdateCache(const std::string& resource_id,
const std::string& md5,
GDataCache::CacheSubDirectoryType subdir,
int cache_state) {
const std::string& md5,
int cache_state) {
AssertOnSequencedWorkerPool();
CacheMap::iterator iter = cache_map_.find(resource_id);
......@@ -194,7 +193,7 @@ void GDataCacheMetadataMap::UpdateCache(const std::string& resource_id,
// Makes no sense to create new entry if cache state is NONE.
DCHECK(cache_state != GDataCache::CACHE_STATE_NONE);
if (cache_state != GDataCache::CACHE_STATE_NONE) {
GDataCache::CacheEntry cache_entry(md5, subdir, cache_state);
GDataCache::CacheEntry cache_entry(md5, cache_state);
cache_map_.insert(std::make_pair(resource_id, cache_entry));
DVLOG(1) << "Added res_id=" << resource_id
<< ", " << cache_entry.ToString();
......@@ -207,7 +206,6 @@ void GDataCacheMetadataMap::UpdateCache(const std::string& resource_id,
cache_map_.erase(iter);
} else { // Otherwise, update entry in cache map.
iter->second.md5 = md5;
iter->second.sub_dir_type = subdir;
iter->second.cache_state = cache_state;
DVLOG(1) << "Updated res_id=" << resource_id
<< ", " << iter->second.ToString();
......@@ -258,7 +256,7 @@ void GDataCacheMetadataMap::RemoveTemporaryFiles() {
CacheMap::iterator iter = cache_map_.begin();
while (iter != cache_map_.end()) {
if (iter->second.sub_dir_type == GDataCache::CACHE_TYPE_TMP) {
if (!iter->second.IsPersistent()) {
// Post-increment the iterator to avoid iterator invalidation.
cache_map_.erase(iter++);
} else {
......@@ -354,6 +352,9 @@ void GDataCacheMetadataMap::ScanCacheDirectory(
continue;
} else if (sub_dir_type == GDataCache::CACHE_TYPE_PERSISTENT ||
sub_dir_type == GDataCache::CACHE_TYPE_TMP) {
if (sub_dir_type == GDataCache::CACHE_TYPE_PERSISTENT)
cache_state = GDataCache::SetCachePersistent(cache_state);
if (file_util::IsLink(current)) {
LOG(WARNING) << "Removing a symlink in persistent/tmp directory"
<< current.value();
......@@ -374,7 +375,7 @@ void GDataCacheMetadataMap::ScanCacheDirectory(
// the file is in the persistent directory.
if (md5 == util::kLocallyModifiedFileExtension) {
if (sub_dir_type == GDataCache::CACHE_TYPE_PERSISTENT) {
cache_state |= GDataCache::SetCacheDirty(cache_state);
cache_state = GDataCache::SetCacheDirty(cache_state);
} else {
LOG(WARNING) << "Removing a dirty file in tmp directory: "
<< current.value();
......@@ -389,8 +390,7 @@ void GDataCacheMetadataMap::ScanCacheDirectory(
// Create and insert new entry into cache map.
cache_map->insert(std::make_pair(
resource_id, GDataCache::CacheEntry(
md5, real_sub_dir_type, cache_state)));
resource_id, GDataCache::CacheEntry(md5, cache_state)));
processed_file_map->insert(std::make_pair(resource_id, current));
}
}
......
chrome/browser/chromeos/gdata/gdata_cache_metadata.h
View file @ 4e248d14
......@@ -40,7 +40,6 @@ class GDataCacheMetadata {
// Creates new entry if it doesn't exist, otherwise update the entry.
virtual void UpdateCache(const std::string& resource_id,
const std::string& md5,
GDataCache::CacheSubDirectoryType subdir,
int cache_state) = 0;
// Removes entry corresponding to |resource_id| from cache map.
......@@ -88,7 +87,6 @@ class GDataCacheMetadataMap : public GDataCacheMetadata {
// GDataCacheMetadata overrides:
virtual void UpdateCache(const std::string& resource_id,
const std::string& md5,
GDataCache::CacheSubDirectoryType subdir,
int cache_state) OVERRIDE;
virtual void RemoveFromCache(const std::string& resource_id) OVERRIDE;
virtual scoped_ptr<GDataCache::CacheEntry> GetCacheEntry(
......
chrome/browser/chromeos/gdata/gdata_cache_metadata_unittest.cc
View file @ 4e248d14
......@@ -116,10 +116,9 @@ class GDataCacheMetadataMapTest : public testing::Test {
void InsertIntoMap(GDataCacheMetadataMap::CacheMap* cache_map,
const std::string& resource_id,
const std::string& md5,
GDataCache::CacheSubDirectoryType sub_dir_type,
int cache_state) {
cache_map->insert(std::make_pair(
resource_id, GDataCache::CacheEntry(md5, sub_dir_type, cache_state)));
resource_id, GDataCache::CacheEntry(md5, cache_state)));
}
ScopedTempDir temp_dir_;
......@@ -139,18 +138,18 @@ TEST_F(GDataCacheMetadataMapTest, CacheTest) {
// Save an initial entry.
std::string test_resource_id("test_resource_id");
std::string test_file_md5("test_file_md5");
GDataCache::CacheSubDirectoryType test_sub_dir_type(
GDataCache::CACHE_TYPE_PERSISTENT);
int test_cache_state(GDataCache::CACHE_STATE_PRESENT);
metadata_->UpdateCache(test_resource_id, test_file_md5,
test_sub_dir_type, test_cache_state);
GDataCache::CacheSubDirectoryType test_sub_dir_type =
GDataCache::CACHE_TYPE_PERSISTENT;
int test_cache_state = (GDataCache::CACHE_STATE_PRESENT |
GDataCache::CACHE_STATE_PERSISTENT);
metadata_->UpdateCache(test_resource_id, test_file_md5, test_cache_state);
// Test that the entry can be retrieved.
scoped_ptr<GDataCache::CacheEntry> cache_entry =
metadata_->GetCacheEntry(test_resource_id, test_file_md5);
ASSERT_TRUE(cache_entry.get());
EXPECT_EQ(test_file_md5, cache_entry->md5);
EXPECT_EQ(test_sub_dir_type, cache_entry->sub_dir_type);
EXPECT_EQ(test_sub_dir_type, cache_entry->GetSubDirectoryType());
EXPECT_EQ(test_cache_state, cache_entry->cache_state);
// Empty md5 should also work.
......@@ -171,17 +170,16 @@ TEST_F(GDataCacheMetadataMapTest, CacheTest) {
// Update all attributes.
test_file_md5 = "test_file_md5_2";
test_sub_dir_type = GDataCache::CACHE_TYPE_PINNED;
test_sub_dir_type = GDataCache::CACHE_TYPE_TMP;
test_cache_state = GDataCache::CACHE_STATE_PINNED;
metadata_->UpdateCache(test_resource_id, test_file_md5, test_sub_dir_type,
test_cache_state);
metadata_->UpdateCache(test_resource_id, test_file_md5, test_cache_state);
// Make sure the values took.
cache_entry =
metadata_->GetCacheEntry(test_resource_id, test_file_md5).Pass();
ASSERT_TRUE(cache_entry.get());
EXPECT_EQ(test_file_md5, cache_entry->md5);
EXPECT_EQ(test_sub_dir_type, cache_entry->sub_dir_type);
EXPECT_EQ(test_sub_dir_type, cache_entry->GetSubDirectoryType());
EXPECT_EQ(test_cache_state, cache_entry->cache_state);
// Empty m5 should work.
......@@ -194,15 +192,14 @@ TEST_F(GDataCacheMetadataMapTest, CacheTest) {
test_file_md5 = "test_file_md5_3";
test_sub_dir_type = GDataCache::CACHE_TYPE_TMP;
test_cache_state = GDataCache::CACHE_STATE_DIRTY;
metadata_->UpdateCache(test_resource_id, test_file_md5, test_sub_dir_type,
test_cache_state);
metadata_->UpdateCache(test_resource_id, test_file_md5, test_cache_state);
// Make sure the values took.
cache_entry =
metadata_->GetCacheEntry(test_resource_id, test_file_md5).Pass();
ASSERT_TRUE(cache_entry.get());
EXPECT_EQ(test_file_md5, cache_entry->md5);
EXPECT_EQ(test_sub_dir_type, cache_entry->sub_dir_type);
EXPECT_EQ(test_sub_dir_type, cache_entry->GetSubDirectoryType());
EXPECT_EQ(test_cache_state, cache_entry->cache_state);
// Empty md5 should work.
......@@ -228,23 +225,21 @@ TEST_F(GDataCacheMetadataMapTest, CacheTest) {
test_file_md5 = "test_file_md5_4";
test_sub_dir_type = GDataCache::CACHE_TYPE_TMP;
test_cache_state = GDataCache::CACHE_STATE_PRESENT;
metadata_->UpdateCache(test_resource_id, test_file_md5, test_sub_dir_type,
test_cache_state);
metadata_->UpdateCache(test_resource_id, test_file_md5, test_cache_state);
// Make sure the values took.
cache_entry =
metadata_->GetCacheEntry(test_resource_id, test_file_md5).Pass();
ASSERT_TRUE(cache_entry.get());
EXPECT_EQ(test_file_md5, cache_entry->md5);
EXPECT_EQ(test_sub_dir_type, cache_entry->sub_dir_type);
EXPECT_EQ(test_sub_dir_type, cache_entry->GetSubDirectoryType());
EXPECT_EQ(test_cache_state, cache_entry->cache_state);
// Update with CACHE_STATE_NONE should evict the entry.
test_file_md5 = "test_file_md5_5";
test_sub_dir_type = GDataCache::CACHE_TYPE_TMP;
test_cache_state = GDataCache::CACHE_STATE_NONE;
metadata_->UpdateCache(test_resource_id, test_file_md5, test_sub_dir_type,
test_cache_state);
metadata_->UpdateCache(test_resource_id, test_file_md5, test_cache_state);
cache_entry =
metadata_->GetCacheEntry(test_resource_id, std::string()).Pass();
......@@ -277,8 +272,10 @@ TEST_F(GDataCacheMetadataMapTest, Initialization) {
cache_entry = metadata_->GetCacheEntry("id_foo", "md5foo");
ASSERT_TRUE(cache_entry.get());
EXPECT_EQ("md5foo", cache_entry->md5);
EXPECT_EQ(GDataCache::CACHE_TYPE_PERSISTENT, cache_entry->sub_dir_type);
EXPECT_EQ(GDataCache::CACHE_STATE_PRESENT | GDataCache::CACHE_STATE_PINNED,
EXPECT_EQ(GDataCache::CACHE_TYPE_PERSISTENT,
cache_entry->GetSubDirectoryType());
EXPECT_EQ(GDataCache::CACHE_STATE_PRESENT | GDataCache::CACHE_STATE_PINNED |
GDataCache::CACHE_STATE_PERSISTENT,
cache_entry->cache_state);
EXPECT_TRUE(PathExists(persistent_directory_.AppendASCII("id_foo.md5foo")));
EXPECT_TRUE(PathExists(pinned_directory_.AppendASCII("id_foo")));
......@@ -289,8 +286,10 @@ TEST_F(GDataCacheMetadataMapTest, Initialization) {
cache_entry = metadata_->GetCacheEntry("id_bar", "");
ASSERT_TRUE(cache_entry.get());
EXPECT_EQ("local", cache_entry->md5);
EXPECT_EQ(GDataCache::CACHE_TYPE_PERSISTENT, cache_entry->sub_dir_type);
EXPECT_EQ(GDataCache::CACHE_STATE_PRESENT | GDataCache::CACHE_STATE_DIRTY,
EXPECT_EQ(GDataCache::CACHE_TYPE_PERSISTENT,
cache_entry->GetSubDirectoryType());
EXPECT_EQ(GDataCache::CACHE_STATE_PRESENT | GDataCache::CACHE_STATE_DIRTY |
GDataCache::CACHE_STATE_PERSISTENT,
cache_entry->cache_state);
EXPECT_TRUE(PathExists(persistent_directory_.AppendASCII("id_bar.local")));
EXPECT_TRUE(PathExists(outgoing_directory_.AppendASCII("id_bar")));
......@@ -316,7 +315,7 @@ TEST_F(GDataCacheMetadataMapTest, Initialization) {
cache_entry = metadata_->GetCacheEntry("id_qux", "md5qux");
ASSERT_TRUE(cache_entry.get());
EXPECT_EQ("md5qux", cache_entry->md5);
EXPECT_EQ(GDataCache::CACHE_TYPE_TMP, cache_entry->sub_dir_type);
EXPECT_EQ(GDataCache::CACHE_TYPE_TMP, cache_entry->GetSubDirectoryType());
EXPECT_EQ(GDataCache::CACHE_STATE_PRESENT, cache_entry->cache_state);
EXPECT_TRUE(PathExists(tmp_directory_.AppendASCII("id_qux.md5qux")));
......@@ -336,7 +335,6 @@ TEST_F(GDataCacheMetadataMapTest, Initialization) {
cache_entry = metadata_->GetCacheEntry("id_corge", "");
ASSERT_TRUE(cache_entry.get());
EXPECT_EQ("", cache_entry->md5);
EXPECT_EQ(GDataCache::CACHE_TYPE_TMP, cache_entry->sub_dir_type);
EXPECT_EQ(GDataCache::CACHE_STATE_PINNED, cache_entry->cache_state);
EXPECT_TRUE(IsLink(pinned_directory_.AppendASCII("id_corge")));
......@@ -364,27 +362,25 @@ TEST_F(GDataCacheMetadataMapTest, RemoveTemporaryFilesTest) {
InsertIntoMap(&cache_map,
"<resource_id_1>",
"<md5>",
GDataCache::CACHE_TYPE_TMP,
GDataCache::CACHE_STATE_PRESENT);
InsertIntoMap(&cache_map,
"<resource_id_2>",
"<md5>",
GDataCache::CACHE_TYPE_PERSISTENT,
GDataCache::CACHE_STATE_PRESENT);
GDataCache::CACHE_STATE_PRESENT |
GDataCache::CACHE_STATE_PERSISTENT);
InsertIntoMap(&cache_map,
"<resource_id_3>",
"<md5>",
GDataCache::CACHE_TYPE_PERSISTENT,
GDataCache::CACHE_STATE_PRESENT);
GDataCache::CACHE_STATE_PRESENT |
GDataCache::CACHE_STATE_PERSISTENT);
InsertIntoMap(&cache_map,
"<resource_id_4>",
"<md5>",
GDataCache::CACHE_TYPE_TMP,
GDataCache::CACHE_STATE_PRESENT);
metadata_->cache_map_ = cache_map;
metadata_->RemoveTemporaryFiles();
// resource 1 and 4 should be gone, as these are CACHE_TYPE_TMP.
// resource 1 and 4 should be gone, as these are temporary.
EXPECT_FALSE(metadata_->GetCacheEntry("<resource_id_1>", "").get());
EXPECT_TRUE(metadata_->GetCacheEntry("<resource_id_2>", "").get());
EXPECT_TRUE(metadata_->GetCacheEntry("<resource_id_3>", "").get());
......
chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
View file @ 4e248d14
......@@ -666,8 +666,9 @@ class GDataFileSystemTest : public testing::Test {
const std::string& md5,
const FilePath& cache_file_path) {
expected_error_ = base::PLATFORM_FILE_OK;
expected_cache_state_ =
GDataCache::CACHE_STATE_PRESENT | GDataCache::CACHE_STATE_DIRTY;
expected_cache_state_ = (GDataCache::CACHE_STATE_PRESENT |
GDataCache::CACHE_STATE_DIRTY |
GDataCache::CACHE_STATE_PERSISTENT);
expected_sub_dir_type_ = GDataCache::CACHE_TYPE_PERSISTENT;
expect_outgoing_symlink_ = false;
VerifyMarkDirty(error, resource_id, md5, cache_file_path);
......@@ -680,8 +681,9 @@ class GDataFileSystemTest : public testing::Test {
const std::string& resource_id,
const std::string& md5) {
expected_error_ = base::PLATFORM_FILE_OK;
expected_cache_state_ =
GDataCache::CACHE_STATE_PRESENT | GDataCache::CACHE_STATE_DIRTY;
expected_cache_state_ = (GDataCache::CACHE_STATE_PRESENT |
GDataCache::CACHE_STATE_DIRTY |
GDataCache::CACHE_STATE_PERSISTENT);
expected_sub_dir_type_ = GDataCache::CACHE_TYPE_PERSISTENT;
expect_outgoing_symlink_ = true;
VerifyCacheFileState(error, resource_id, md5);
......@@ -701,7 +703,7 @@ class GDataFileSystemTest : public testing::Test {
GDataCache::IsCachePinned(expected_cache_state_)) {
ASSERT_TRUE(cache_entry.get());
EXPECT_EQ(expected_cache_state_, cache_entry->cache_state);
EXPECT_EQ(expected_sub_dir_type_, cache_entry->sub_dir_type);
EXPECT_EQ(expected_sub_dir_type_, cache_entry->GetSubDirectoryType());
} else {
EXPECT_FALSE(cache_entry.get());
}
......@@ -2376,7 +2378,8 @@ TEST_F(GDataFileSystemTest, UpdateFileByResourceId_PersistentFile) {
GetTestFilePath("root_feed.json"), // Anything works.
base::PLATFORM_FILE_OK,
GDataCache::CACHE_STATE_PRESENT |
GDataCache::CACHE_STATE_PINNED,
GDataCache::CACHE_STATE_PINNED |
GDataCache::CACHE_STATE_PERSISTENT,
GDataCache::CACHE_TYPE_PERSISTENT);
ASSERT_TRUE(file_util::PathExists(original_cache_file_path));
......@@ -2387,7 +2390,8 @@ TEST_F(GDataFileSystemTest, UpdateFileByResourceId_PersistentFile) {
base::PLATFORM_FILE_OK,
GDataCache::CACHE_STATE_PRESENT |
GDataCache::CACHE_STATE_PINNED |
GDataCache::CACHE_STATE_DIRTY,
GDataCache::CACHE_STATE_DIRTY |
GDataCache::CACHE_STATE_PERSISTENT,
GDataCache::CACHE_TYPE_PERSISTENT);
const FilePath dirty_cache_file_path =
GDataCache::GetCacheRootPath(profile_.get())
......@@ -2405,7 +2409,8 @@ TEST_F(GDataFileSystemTest, UpdateFileByResourceId_PersistentFile) {
base::PLATFORM_FILE_OK,
GDataCache::CACHE_STATE_PRESENT |
GDataCache::CACHE_STATE_PINNED |
GDataCache::CACHE_STATE_DIRTY,
GDataCache::CACHE_STATE_DIRTY |
GDataCache::CACHE_STATE_PERSISTENT,
GDataCache::CACHE_TYPE_PERSISTENT);
const FilePath outgoing_symlink_path =
GDataCache::GetCacheRootPath(profile_.get())
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment