Disk Cache: cleanup of OpenBlockFile.

Don't add the file to the list of open files until
it passes all sanity checks.

BUG=18174
TEST=unittest

Review URL: http://codereview.chromium.org/164336

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@23090 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 31 insertions, 7 deletions

diff --git a/net/disk_cache/block_files.cc b/net/disk_cache/block_files.cc
index 0e9eb04..ffb74db 100644
--- a/net/disk_cache/block_files.cc
+++ b/net/disk_cache/block_files.cc
@@ -237,24 +237,18 @@ bool BlockFiles::OpenBlockFile(int index) {
   }
 
   std::wstring name = Name(index);
-  MappedFile* file = new MappedFile();
-  file->AddRef();
+  scoped_refptr<MappedFile> file(new MappedFile());
 
   if (!file->Init(name, kBlockHeaderSize)) {
-    NOTREACHED();
     LOG(ERROR) << "Failed to open " << name;
-    file->Release();
     return false;
   }
 
   if (file->GetLength() < static_cast<size_t>(kBlockHeaderSize)) {
     LOG(ERROR) << "File too small " << name;
-    file->Release();
     return false;
   }
 
-  block_files_[index] = file;
-
   BlockFileHeader* header = reinterpret_cast<BlockFileHeader*>(file->buffer());
   if (kBlockMagic != header->magic || kCurrentVersion != header->version) {
     LOG(ERROR) << "Invalid file version or magic";
@@ -266,6 +260,9 @@ bool BlockFiles::OpenBlockFile(int index) {
     if (!FixBlockFileHeader(file))
       return false;
   }
+
+  DCHECK(!block_files_[index]);
+  file.swap(&block_files_[index]);
   return true;
 }
 
diff --git a/net/disk_cache/block_files.h b/net/disk_cache/block_files.h
index e86546e..faf38ef 100644
--- a/net/disk_cache/block_files.h
+++ b/net/disk_cache/block_files.h
@@ -77,6 +77,7 @@ class BlockFiles {
   std::vector<MappedFile*> block_files_;  // The actual files.
 
   FRIEND_TEST(DiskCacheTest, BlockFiles_ZeroSizeFile);
+  FRIEND_TEST(DiskCacheTest, BlockFiles_InvalidFile);
 
   DISALLOW_EVIL_CONSTRUCTORS(BlockFiles);
 };
diff --git a/net/disk_cache/block_files_unittest.cc b/net/disk_cache/block_files_unittest.cc
index e7647d7..6256a3d 100644
--- a/net/disk_cache/block_files_unittest.cc
+++ b/net/disk_cache/block_files_unittest.cc
@@ -178,4 +178,30 @@ TEST_F(DiskCacheTest, BlockFiles_ZeroSizeFile) {
   ASSERT_FALSE(files.Init(false));
 }
 
+// An invalid file can be detected after init.
+TEST_F(DiskCacheTest, BlockFiles_InvalidFile) {
+  std::wstring path = GetCachePath();
+  ASSERT_TRUE(DeleteCache(path.c_str()));
+  ASSERT_TRUE(file_util::CreateDirectory(path));
+
+  BlockFiles files(path);
+  ASSERT_TRUE(files.Init(true));
+
+  // Let's access block 10 of file 5. (There is no file).
+  Addr addr(BLOCK_256, 1, 5, 10);
+  EXPECT_TRUE(NULL == files.GetFile(addr));
+
+  // Let's create an invalid file.
+  FilePath filename(FilePath::FromWStringHack(files.Name(5)));
+  char header[kBlockHeaderSize];
+  memset(header, 'a', kBlockHeaderSize);
+  EXPECT_EQ(kBlockHeaderSize,
+            file_util::WriteFile(filename, header, kBlockHeaderSize));
+
+  EXPECT_TRUE(NULL == files.GetFile(addr));
+
+  // The file should not have been cached (it is still invalid).
+  EXPECT_TRUE(NULL == files.GetFile(addr));
+}
+
 }  // namespace disk_cache