Commit 995d0586 authored by tzik@chromium.org's avatar tzik@chromium.org

[Storage] Normalize storage partition path before garbage collection

Protect |active_paths| from garbage collection even when they are not direct subdirectories of |storage_root|.

BUG=328637

Review URL: https://codereview.chromium.org/314293003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275662 0039d316-1c4b-4281-b951-d872f2087c98
parent 23c5e6b4
...@@ -266,6 +266,27 @@ void BlockingObliteratePath( ...@@ -266,6 +266,27 @@ void BlockingObliteratePath(
} }
} }
// Ensures each path in |active_paths| is a direct child of storage_root.
void NormalizeActivePaths(const base::FilePath& storage_root,
base::hash_set<base::FilePath>* active_paths) {
base::hash_set<base::FilePath> normalized_active_paths;
for (base::hash_set<base::FilePath>::iterator iter = active_paths->begin();
iter != active_paths->end(); ++iter) {
base::FilePath relative_path;
if (!storage_root.AppendRelativePath(*iter, &relative_path))
continue;
std::vector<base::FilePath::StringType> components;
relative_path.GetComponents(&components);
DCHECK(!relative_path.empty());
normalized_active_paths.insert(storage_root.Append(components.front()));
}
active_paths->swap(normalized_active_paths);
}
// Deletes all entries inside the |storage_root| that are not in the // Deletes all entries inside the |storage_root| that are not in the
// |active_paths|. Deletion is done in 2 steps: // |active_paths|. Deletion is done in 2 steps:
// //
...@@ -289,6 +310,8 @@ void BlockingGarbageCollect( ...@@ -289,6 +310,8 @@ void BlockingGarbageCollect(
scoped_ptr<base::hash_set<base::FilePath> > active_paths) { scoped_ptr<base::hash_set<base::FilePath> > active_paths) {
CHECK(storage_root.IsAbsolute()); CHECK(storage_root.IsAbsolute());
NormalizeActivePaths(storage_root, active_paths.get());
base::FileEnumerator enumerator(storage_root, false, kAllFileTypes); base::FileEnumerator enumerator(storage_root, false, kAllFileTypes);
base::FilePath trash_directory; base::FilePath trash_directory;
if (!base::CreateTemporaryDirInDir(storage_root, kTrashDirname, if (!base::CreateTemporaryDirInDir(storage_root, kTrashDirname,
......
...@@ -25,7 +25,8 @@ namespace content { ...@@ -25,7 +25,8 @@ namespace content {
class BrowserContext; class BrowserContext;
// A std::string to StoragePartition map for use with SupportsUserData APIs. // A std::string to StoragePartition map for use with SupportsUserData APIs.
class StoragePartitionImplMap : public base::SupportsUserData::Data { class CONTENT_EXPORT StoragePartitionImplMap
: public base::SupportsUserData::Data {
public: public:
explicit StoragePartitionImplMap(BrowserContext* browser_context); explicit StoragePartitionImplMap(BrowserContext* browser_context);
...@@ -57,6 +58,7 @@ class StoragePartitionImplMap : public base::SupportsUserData::Data { ...@@ -57,6 +58,7 @@ class StoragePartitionImplMap : public base::SupportsUserData::Data {
private: private:
FRIEND_TEST_ALL_PREFIXES(StoragePartitionConfigTest, OperatorLess); FRIEND_TEST_ALL_PREFIXES(StoragePartitionConfigTest, OperatorLess);
FRIEND_TEST_ALL_PREFIXES(StoragePartitionImplMapTest, GarbageCollect);
// Each StoragePartition is uniquely identified by which partition domain // Each StoragePartition is uniquely identified by which partition domain
// it belongs to (such as an app or the browser itself), the user supplied // it belongs to (such as an app or the browser itself), the user supplied
......
...@@ -3,16 +3,17 @@ ...@@ -3,16 +3,17 @@
// found in the LICENSE file. // found in the LICENSE file.
#include "content/browser/storage_partition_impl_map.h" #include "content/browser/storage_partition_impl_map.h"
#include "base/file_util.h"
#include "base/run_loop.h"
#include "content/public/test/test_browser_context.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace content { namespace content {
class StoragePartitionConfigTest : public testing::Test {
};
// Test that the Less comparison function is implemented properly to uniquely // Test that the Less comparison function is implemented properly to uniquely
// identify storage partitions used as keys in a std::map. // identify storage partitions used as keys in a std::map.
TEST_F(StoragePartitionConfigTest, OperatorLess) { TEST(StoragePartitionConfigTest, OperatorLess) {
StoragePartitionImplMap::StoragePartitionConfig c1( StoragePartitionImplMap::StoragePartitionConfig c1(
std::string(), std::string(), false); std::string(), std::string(), false);
StoragePartitionImplMap::StoragePartitionConfig c2( StoragePartitionImplMap::StoragePartitionConfig c2(
...@@ -60,4 +61,32 @@ TEST_F(StoragePartitionConfigTest, OperatorLess) { ...@@ -60,4 +61,32 @@ TEST_F(StoragePartitionConfigTest, OperatorLess) {
EXPECT_TRUE(!less(c1, c2) && !less(c2, c1)); EXPECT_TRUE(!less(c1, c2) && !less(c2, c1));
} }
TEST(StoragePartitionImplMapTest, GarbageCollect) {
base::MessageLoop message_loop;
TestBrowserContext browser_context;
StoragePartitionImplMap storage_partition_impl_map(&browser_context);
scoped_ptr<base::hash_set<base::FilePath> > active_paths(
new base::hash_set<base::FilePath>);
base::FilePath active_path = browser_context.GetPath().Append(
StoragePartitionImplMap::GetStoragePartitionPath(
"active", std::string()));
ASSERT_TRUE(base::CreateDirectory(active_path));
active_paths->insert(active_path);
base::FilePath inactive_path = browser_context.GetPath().Append(
StoragePartitionImplMap::GetStoragePartitionPath(
"inactive", std::string()));
ASSERT_TRUE(base::CreateDirectory(inactive_path));
base::RunLoop run_loop;
storage_partition_impl_map.GarbageCollect(
active_paths.Pass(), run_loop.QuitClosure());
run_loop.Run();
EXPECT_TRUE(base::PathExists(active_path));
EXPECT_FALSE(base::PathExists(inactive_path));
}
} // namespace content } // namespace content
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