Switched from DeleteFile() to DestroyDB().

DeleteFile ignores db lock file so leveldb_chrome::DestroyDB is preferred.

Bug: 802298
Change-Id: I15af9ae4c402f94d537a5b4495e8a60f6dd906fe
Reviewed-on: https://chromium-review.googlesource.com/1069415
Commit-Queue: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568620}

chrome/browser/android/history_report/delta_file_backend_leveldb.cc

@@ -13,6 +13,7 @@
 #include "base/trace_event/process_memory_dump.h"
 #include "chrome/browser/android/history_report/delta_file_commons.h"
 #include "third_party/leveldatabase/env_chromium.h"
+#include "third_party/leveldatabase/leveldb_chrome.h"
 #include "third_party/leveldatabase/src/include/leveldb/comparator.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "third_party/leveldatabase/src/include/leveldb/iterator.h"
@@ -99,8 +100,13 @@ bool DeltaFileBackend::Init() {
   std::string path = path_.value();
   leveldb::Status status = leveldb_env::OpenDB(options, path, &db_);
   if (status.IsCorruption()) {
-    LOG(WARNING) << "Deleting possibly-corrupt database";
-    base::DeleteFile(path_, true);
+    LOG(WARNING) << "Deleting corrupt database";
+    status = leveldb_chrome::DeleteDB(path_, options);
+    if (!status.ok()) {
+      LOG(ERROR) << "Unable to delete corrupt database " << path_
+                 << ", error: " << status.ToString();
+      return false;
+    }
     status = leveldb_env::OpenDB(options, path, &db_);
   }
   if (status.ok()) {
@@ -218,7 +224,7 @@ std::unique_ptr<std::vector<DeltaFileEntryWithData>> DeltaFileBackend::Query(
 void DeltaFileBackend::Clear() {
   if (!EnsureInitialized()) return;
   db_.reset();
-  base::DeleteFile(path_, true);
+  leveldb_chrome::DeleteDB(path_, leveldb_env::Options());
   Init();
 }
 

chrome/browser/android/history_report/usage_reports_buffer_backend.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/android/history_report/usage_report_util.h"
 #include "chrome/browser/android/proto/delta_file.pb.h"
 #include "third_party/leveldatabase/env_chromium.h"
+#include "third_party/leveldatabase/leveldb_chrome.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "third_party/leveldatabase/src/include/leveldb/iterator.h"
 #include "third_party/leveldatabase/src/include/leveldb/options.h"
@@ -46,15 +47,19 @@ bool UsageReportsBufferBackend::Init() {
                             leveldb_env::LEVELDB_STATUS_MAX);
   if (status.IsCorruption()) {
     LOG(ERROR) << "Deleting corrupt database";
-    base::DeleteFile(db_file_name_, true);
+    status = leveldb_chrome::DeleteDB(db_file_name_, options);
+    if (!status.ok()) {
+      LOG(ERROR) << "Unable to delete " << db_file_name_
+                 << ", error: " << status.ToString();
+      return false;
+    }
     status = leveldb_env::OpenDB(options, path, &db_);
   }
   if (status.ok()) {
     CHECK(db_);
     return true;
   }
-  LOG(WARNING) << "Unable to open " << path << ": "
-               << status.ToString();
+  LOG(WARNING) << "Unable to open " << path << ": " << status.ToString();
   return false;
 }
 
@@ -122,7 +127,7 @@ void UsageReportsBufferBackend::Remove(
 
 void UsageReportsBufferBackend::Clear() {
   db_.reset();
-  base::DeleteFile(db_file_name_, true);
+  leveldb_chrome::DeleteDB(db_file_name_, leveldb_env::Options());
   Init();
 }
 

chrome/browser/sync_file_system/drive_backend/metadata_database.cc

@@ -36,6 +36,7 @@
 #include "google_apis/drive/drive_api_parser.h"
 #include "storage/common/fileapi/file_system_util.h"
 #include "third_party/leveldatabase/env_chromium.h"
+#include "third_party/leveldatabase/leveldb_chrome.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "third_party/leveldatabase/src/include/leveldb/status.h"
 #include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
@@ -583,9 +584,11 @@ void MetadataDatabase::ClearDatabase(
   DCHECK(metadata_database);
   base::FilePath database_path = metadata_database->database_path_;
   DCHECK(!database_path.empty());
+  leveldb::Options options = leveldb_env::Options();
+  if (metadata_database->env_override_)
+    options.env = metadata_database->env_override_;
   metadata_database.reset();
-
-  base::DeleteFile(database_path, true /* recursive */);
+  leveldb_chrome::DeleteDB(database_path, options);
 }
 
 int64_t MetadataDatabase::GetLargestFetchedChangeID() const {

chrome/browser/sync_file_system/drive_backend/metadata_database_unittest.cc

@@ -10,6 +10,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
@@ -217,10 +218,16 @@ class MetadataDatabaseTest : public testing::TestWithParam<bool> {
     return 0;
   }
 
+  bool enable_on_disk_index() const { return GetParam(); }
+
+  leveldb::Env* in_memory_env() const { return in_memory_env_.get(); }
+
+  const base::FilePath& DatabasePath() const { return database_dir_.GetPath(); }
+
   SyncStatusCode InitializeMetadataDatabase() {
     SyncStatusCode status = SYNC_STATUS_UNKNOWN;
     metadata_database_ = MetadataDatabase::CreateInternal(
-        database_dir_.GetPath(), in_memory_env_.get(), GetParam(), &status);
+        DatabasePath(), in_memory_env_.get(), enable_on_disk_index(), &status);
     return status;
   }
 
@@ -275,8 +282,8 @@ class MetadataDatabaseTest : public testing::TestWithParam<bool> {
     options.create_if_missing = true;
     options.max_open_files = 0;  // Use minimum.
     options.env = in_memory_env_.get();
-    leveldb::Status status = leveldb_env::OpenDB(
-        options, database_dir_.GetPath().AsUTF8Unsafe(), &db);
+    leveldb::Status status =
+        leveldb_env::OpenDB(options, DatabasePath().AsUTF8Unsafe(), &db);
     EXPECT_TRUE(status.ok());
 
     std::unique_ptr<LevelDBWrapper> wrapper(new LevelDBWrapper(std::move(db)));
@@ -538,7 +545,7 @@ class MetadataDatabaseTest : public testing::TestWithParam<bool> {
 
     MetadataDatabaseIndexInterface* index1 = metadata_database_->index_.get();
     MetadataDatabaseIndexInterface* index2 = metadata_database_2->index_.get();
-    if (GetParam()) {
+    if (enable_on_disk_index()) {
       VerifyReloadConsistencyForOnDisk(
           static_cast<MetadataDatabaseIndexOnDisk*>(index1),
           static_cast<MetadataDatabaseIndexOnDisk*>(index2));
@@ -1171,5 +1178,29 @@ TEST_P(MetadataDatabaseTest, DumpFiles) {
   EXPECT_TRUE(file->HasKey("details"));
 }
 
+TEST_P(MetadataDatabaseTest, ClearDatabase) {
+  const bool db_on_disk = enable_on_disk_index();
+  leveldb::Env* env = db_on_disk ? leveldb::Env::Default() : in_memory_env();
+  std::vector<std::string> children;
+  EXPECT_TRUE(env->GetChildren(DatabasePath().AsUTF8Unsafe(), &children).ok());
+  EXPECT_EQ(children.size(), 0ul);
+
+  SyncStatusCode status = SYNC_STATUS_UNKNOWN;
+  std::unique_ptr<MetadataDatabase> metadata_database =
+      MetadataDatabase::CreateInternal(DatabasePath(), env,
+                                       enable_on_disk_index(), &status);
+  ASSERT_EQ(SYNC_STATUS_OK, status);
+  EXPECT_TRUE(env->GetChildren(DatabasePath().AsUTF8Unsafe(), &children).ok());
+  EXPECT_GT(children.size(), 0ul);
+
+  MetadataDatabase::ClearDatabase(std::move(metadata_database));
+
+  if (db_on_disk) {
+    EXPECT_FALSE(base::PathExists(DatabasePath()));
+  } else {
+    EXPECT_TRUE(base::PathExists(DatabasePath()));
+  }
+}
+
 }  // namespace drive_backend
 }  // namespace sync_file_system

components/data_reduction_proxy/core/browser/data_store_impl.cc

@@ -12,6 +12,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "components/data_reduction_proxy/proto/data_store.pb.h"
 #include "third_party/leveldatabase/env_chromium.h"
+#include "third_party/leveldatabase/leveldb_chrome.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "third_party/leveldatabase/src/include/leveldb/options.h"
 #include "third_party/leveldatabase/src/include/leveldb/status.h"
@@ -146,8 +147,11 @@ DataStore::Status DataStoreImpl::OpenDB() {
 DataStore::Status DataStoreImpl::RecreateDB() {
   DCHECK(sequence_checker_.CalledOnValidSequence());
 
-  db_.reset(nullptr);
-  base::DeleteFile(profile_path_.Append(kDBName), true);
+  db_.reset();
+  const base::FilePath db_path = profile_path_.Append(kDBName);
+  leveldb::Status s = leveldb_chrome::DeleteDB(db_path, leveldb::Options());
+  if (!s.ok())
+    return LevelDbToDRPStoreStatus(s);
 
   return OpenDB();
 }

components/drive/resource_metadata_storage.cc

@@ -25,6 +25,7 @@
 #include "components/drive/drive_api_util.h"
 #include "components/drive/file_system_core_util.h"
 #include "third_party/leveldatabase/env_chromium.h"
+#include "third_party/leveldatabase/leveldb_chrome.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
 
@@ -571,11 +572,21 @@ bool ResourceMetadataStorage::UpgradeOldDB(
   const base::FilePath preserved_resource_map_path =
       directory_path.Append(kPreservedResourceMapDBName);
 
+  leveldb_env::Options options;
+  options.max_open_files = 0;  // Use minimum.
+  options.create_if_missing = false;
+
   if (base::PathExists(preserved_resource_map_path)) {
     // Preserved DB is found. The previous attempt to create a new DB should not
     // be successful. Discard the imperfect new DB and restore the old DB.
-    if (!base::DeleteFile(resource_map_path, false /* recursive */) ||
-        !base::Move(preserved_resource_map_path, resource_map_path))
+    leveldb::Status status =
+        leveldb_chrome::DeleteDB(resource_map_path, options);
+    if (!status.ok()) {
+      LOG(ERROR) << "ERROR deleting " << resource_map_path
+                 << ", err:" << status.ToString();
+      return false;
+    }
+    if (!base::Move(preserved_resource_map_path, resource_map_path))
       return false;
   }
 
@@ -584,9 +595,6 @@ bool ResourceMetadataStorage::UpgradeOldDB(
 
   // Open DB.
   std::unique_ptr<leveldb::DB> resource_map;
-  leveldb_env::Options options;
-  options.max_open_files = 0;  // Use minimum.
-  options.create_if_missing = false;
   leveldb::Status status = leveldb_env::OpenDB(
       options, resource_map_path.AsUTF8Unsafe(), &resource_map);
   if (!status.ok())
@@ -661,18 +669,18 @@ bool ResourceMetadataStorage::Initialize() {
   const base::FilePath trashed_resource_map_path =
       directory_path_.Append(kTrashedResourceMapDBName);
 
+  leveldb_env::Options options;
+  options.max_open_files = 0;  // Use minimum.
+  options.create_if_missing = false;
+
   // Discard unneeded DBs.
-  if (!base::DeleteFile(preserved_resource_map_path, true /* recursive */) ||
-      !base::DeleteFile(trashed_resource_map_path, true /* recursive */)) {
+  if (!leveldb_chrome::DeleteDB(preserved_resource_map_path, options).ok() ||
+      !leveldb_chrome::DeleteDB(trashed_resource_map_path, options).ok()) {
     LOG(ERROR) << "Failed to remove unneeded DBs.";
     return false;
   }
 
   // Try to open the existing DB.
-  leveldb_env::Options options;
-  options.max_open_files = 0;  // Use minimum.
-  options.create_if_missing = false;
-
   DBInitStatus open_existing_result = DB_INIT_NOT_FOUND;
   leveldb::Status status;
   if (base::PathExists(resource_map_path)) {

storage/browser/fileapi/sandbox_directory_database.cc

@@ -24,6 +24,7 @@
 #include "storage/browser/fileapi/file_system_usage_cache.h"
 #include "storage/common/fileapi/file_system_util.h"
 #include "third_party/leveldatabase/env_chromium.h"
+#include "third_party/leveldatabase/leveldb_chrome.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
 
@@ -760,7 +761,7 @@ bool SandboxDirectoryDatabase::Init(RecoveryOption recovery_option) {
       FALLTHROUGH;
     case DELETE_ON_CORRUPTION:
       LOG(WARNING) << "Clearing SandboxDirectoryDatabase.";
-      if (!base::DeleteFile(filesystem_data_directory_, true))
+      if (!leveldb_chrome::DeleteDB(filesystem_data_directory_, options).ok())
         return false;
       if (!base::CreateDirectory(filesystem_data_directory_))
         return false;