Don't delete IndexedDB on some non-corruption I/O errors.

This patch only discounts the low hanging fruit of No Space, No Memory, and
Too Many Open Files, as those are somewhat common occurrences and definitely
will not be helped by deleting the on-disk IDB. There are comments on the bug
about other errors that don't have to cause data loss.

This is also the first IDB patch that makes use of stuff in the leveldb namespace
outside of the indexed_db/leveldb subdirectory.

BUG=239882

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

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

6 files changed, 162 insertions, 27 deletions

diff --git a/content/browser/indexed_db/indexed_db_backing_store.cc b/content/browser/indexed_db/indexed_db_backing_store.cc
index 8267a626..5836911 100644
--- a/content/browser/indexed_db/indexed_db_backing_store.cc
+++ b/content/browser/indexed_db/indexed_db_backing_store.cc
@@ -21,6 +21,7 @@
 #include "content/common/indexed_db/indexed_db_key_path.h"
 #include "content/common/indexed_db/indexed_db_key_range.h"
 #include "third_party/WebKit/public/platform/WebIDBTypes.h"
+#include "third_party/leveldatabase/env_chromium.h"
 
 using base::StringPiece;
 
@@ -347,11 +348,12 @@ WARN_UNUSED_RESULT static bool GetMaxObjectStoreId(
 
 class DefaultLevelDBFactory : public LevelDBFactory {
  public:
-  virtual scoped_ptr<LevelDBDatabase> OpenLevelDB(
+  virtual leveldb::Status OpenLevelDB(
       const base::FilePath& file_name,
       const LevelDBComparator* comparator,
+      scoped_ptr<LevelDBDatabase>* db,
       bool* is_disk_full) OVERRIDE {
-    return LevelDBDatabase::Open(file_name, comparator, is_disk_full);
+    return LevelDBDatabase::Open(file_name, comparator, db, is_disk_full);
   }
   virtual bool DestroyLevelDB(const base::FilePath& file_name) OVERRIDE {
     return LevelDBDatabase::Destroy(file_name);
@@ -400,9 +402,45 @@ enum IndexedDBLevelDBBackingStoreOpenResult {
   INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_ATTEMPT_NON_ASCII,
   INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_DISK_FULL,
   INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_ORIGIN_TOO_LONG,
+  INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_NO_RECOVERY,
   INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_MAX,
 };
 
+bool RecoveryCouldBeFruitful(leveldb::Status status) {
+  leveldb_env::MethodID method;
+  int error = -1;
+  leveldb_env::ErrorParsingResult result = leveldb_env::ParseMethodAndError(
+      status.ToString().c_str(), &method, &error);
+  switch (result) {
+    case leveldb_env::NONE:
+      return true;
+    case leveldb_env::METHOD_AND_PFE: {
+      base::PlatformFileError pfe = static_cast<base::PlatformFileError>(error);
+      switch (pfe) {
+        case base::PLATFORM_FILE_ERROR_TOO_MANY_OPENED:
+        case base::PLATFORM_FILE_ERROR_NO_MEMORY:
+        case base::PLATFORM_FILE_ERROR_NO_SPACE:
+          return false;
+        default:
+          return true;
+      }
+    }
+    case leveldb_env::METHOD_AND_ERRNO: {
+      switch (error) {
+        case EMFILE:
+        case ENOMEM:
+        case ENOSPC:
+          return false;
+        default:
+          return true;
+      }
+    }
+    default:
+      return true;
+  }
+  return true;
+}
+
 scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
     const std::string& origin_identifier,
     const base::FilePath& path_base,
@@ -428,7 +466,6 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
   *data_loss = WebKit::WebIDBCallbacks::DataLossNone;
 
   scoped_ptr<LevelDBComparator> comparator(new Comparator());
-  scoped_ptr<LevelDBDatabase> db;
 
   if (!IsStringASCII(path_base.AsUTF8Unsafe())) {
     base::Histogram::FactoryGet("WebCore.IndexedDB.BackingStore.OpenStatus",
@@ -493,7 +530,9 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
   base::FilePath file_path = path_base.Append(identifier_path);
 
   bool is_disk_full = false;
-  db = leveldb_factory->OpenLevelDB(file_path, comparator.get(), &is_disk_full);
+  scoped_ptr<LevelDBDatabase> db;
+  leveldb::Status status = leveldb_factory->OpenLevelDB(
+      file_path, comparator.get(), &db, &is_disk_full);
 
   if (db) {
     bool known = false;
@@ -539,6 +578,16 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
                                 base::HistogramBase::kUmaTargetedHistogramFlag)
         ->Add(INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_DISK_FULL);
     return scoped_refptr<IndexedDBBackingStore>();
+  } else if (!RecoveryCouldBeFruitful(status)) {
+    LOG(ERROR) << "Unable to open backing store, not trying to recover - "
+               << status.ToString();
+    base::Histogram::FactoryGet("WebCore.IndexedDB.BackingStore.OpenStatus",
+                                1,
+                                INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_MAX,
+                                INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_MAX + 1,
+                                base::HistogramBase::kUmaTargetedHistogramFlag)
+        ->Add(INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_NO_RECOVERY);
+    return scoped_refptr<IndexedDBBackingStore>();
   } else {
     LOG(ERROR) << "IndexedDB backing store open failed, attempting cleanup";
     *data_loss = WebKit::WebIDBCallbacks::DataLossTotal;
@@ -556,7 +605,7 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
     }
 
     LOG(ERROR) << "IndexedDB backing store cleanup succeeded, reopening";
-    db = leveldb_factory->OpenLevelDB(file_path, comparator.get(), NULL);
+    leveldb_factory->OpenLevelDB(file_path, comparator.get(), &db, NULL);
     if (!db) {
       LOG(ERROR) << "IndexedDB backing store reopen after recovery failed";
       base::Histogram::FactoryGet(
diff --git a/content/browser/indexed_db/indexed_db_backing_store.h b/content/browser/indexed_db/indexed_db_backing_store.h
index 7dbdc53..4498b08 100644
--- a/content/browser/indexed_db/indexed_db_backing_store.h
+++ b/content/browser/indexed_db/indexed_db_backing_store.h
@@ -31,9 +31,10 @@ class LevelDBDatabase;
 class LevelDBFactory {
  public:
   virtual ~LevelDBFactory() {}
-  virtual scoped_ptr<LevelDBDatabase> OpenLevelDB(
+  virtual leveldb::Status OpenLevelDB(
       const base::FilePath& file_name,
       const LevelDBComparator* comparator,
+      scoped_ptr<LevelDBDatabase>* db,
       bool* is_disk_full) = 0;
   virtual bool DestroyLevelDB(const base::FilePath& file_name) = 0;
 };
diff --git a/content/browser/indexed_db/indexed_db_cleanup_on_io_error_unittest.cc b/content/browser/indexed_db/indexed_db_cleanup_on_io_error_unittest.cc
index 1f37a12..e05ceb9 100644
--- a/content/browser/indexed_db/indexed_db_cleanup_on_io_error_unittest.cc
+++ b/content/browser/indexed_db/indexed_db_cleanup_on_io_error_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <cerrno>
+
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/strings/string16.h"
@@ -9,6 +11,7 @@
 #include "content/browser/indexed_db/indexed_db_backing_store.h"
 #include "content/browser/indexed_db/leveldb/leveldb_database.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/leveldatabase/env_chromium.h"
 
 using base::StringPiece;
 using content::IndexedDBBackingStore;
@@ -38,11 +41,13 @@ class BustedLevelDBDatabase : public LevelDBDatabase {
 class MockLevelDBFactory : public LevelDBFactory {
  public:
   MockLevelDBFactory() : destroy_called_(false) {}
-  virtual scoped_ptr<LevelDBDatabase> OpenLevelDB(
+  virtual leveldb::Status OpenLevelDB(
       const base::FilePath& file_name,
       const LevelDBComparator* comparator,
+      scoped_ptr<LevelDBDatabase>* db,
       bool* is_disk_full = 0) OVERRIDE {
-    return BustedLevelDBDatabase::Open(file_name, comparator);
+    *db = BustedLevelDBDatabase::Open(file_name, comparator);
+    return leveldb::Status::OK();
   }
   virtual bool DestroyLevelDB(const base::FilePath& file_name) OVERRIDE {
     EXPECT_FALSE(destroy_called_);
@@ -72,4 +77,78 @@ TEST(IndexedDBIOErrorTest, CleanUpTest) {
                                   &mock_leveldb_factory);
 }
 
+template <class T>
+class MockErrorLevelDBFactory : public LevelDBFactory {
+ public:
+  MockErrorLevelDBFactory(T error, bool expect_destroy)
+      : error_(error),
+        expect_destroy_(expect_destroy),
+        destroy_called_(false) {}
+  virtual leveldb::Status OpenLevelDB(
+      const base::FilePath& file_name,
+      const LevelDBComparator* comparator,
+      scoped_ptr<LevelDBDatabase>* db,
+      bool* is_disk_full = 0) OVERRIDE {
+    return MakeIOError(
+        "some filename", "some message", leveldb_env::kNewLogger, error_);
+  }
+  virtual bool DestroyLevelDB(const base::FilePath& file_name) OVERRIDE {
+    EXPECT_FALSE(destroy_called_);
+    destroy_called_ = true;
+    return false;
+  }
+  virtual ~MockErrorLevelDBFactory() {
+    EXPECT_EQ(expect_destroy_, destroy_called_);
+  }
+
+ private:
+  T error_;
+  bool expect_destroy_;
+  bool destroy_called_;
+};
+
+TEST(IndexedDBNonRecoverableIOErrorTest, NuancedCleanupTest) {
+  std::string origin_identifier("http_localhost_81");
+  base::ScopedTempDir temp_directory;
+  ASSERT_TRUE(temp_directory.CreateUniqueTempDir());
+  const base::FilePath path = temp_directory.path();
+  std::string dummy_file_identifier;
+  WebKit::WebIDBCallbacks::DataLoss data_loss =
+      WebKit::WebIDBCallbacks::DataLossNone;
+
+  MockErrorLevelDBFactory<int> mock_leveldb_factory(ENOSPC, false);
+  scoped_refptr<IndexedDBBackingStore> backing_store =
+      IndexedDBBackingStore::Open(origin_identifier,
+                                  path,
+                                  dummy_file_identifier,
+                                  &data_loss,
+                                  &mock_leveldb_factory);
+
+  MockErrorLevelDBFactory<base::PlatformFileError> mock_leveldb_factory2(
+      base::PLATFORM_FILE_ERROR_NO_MEMORY, false);
+  scoped_refptr<IndexedDBBackingStore> backing_store2 =
+      IndexedDBBackingStore::Open(origin_identifier,
+                                  path,
+                                  dummy_file_identifier,
+                                  &data_loss,
+                                  &mock_leveldb_factory2);
+
+  MockErrorLevelDBFactory<int> mock_leveldb_factory3(EIO, true);
+  scoped_refptr<IndexedDBBackingStore> backing_store3 =
+      IndexedDBBackingStore::Open(origin_identifier,
+                                  path,
+                                  dummy_file_identifier,
+                                  &data_loss,
+                                  &mock_leveldb_factory3);
+
+  MockErrorLevelDBFactory<base::PlatformFileError> mock_leveldb_factory4(
+      base::PLATFORM_FILE_ERROR_FAILED, true);
+  scoped_refptr<IndexedDBBackingStore> backing_store4 =
+      IndexedDBBackingStore::Open(origin_identifier,
+                                  path,
+                                  dummy_file_identifier,
+                                  &data_loss,
+                                  &mock_leveldb_factory4);
+}
+
 }  // namespace
diff --git a/content/browser/indexed_db/leveldb/leveldb_database.cc b/content/browser/indexed_db/leveldb/leveldb_database.cc
index cb67cd9..03d8ca6 100644
--- a/content/browser/indexed_db/leveldb/leveldb_database.cc
+++ b/content/browser/indexed_db/leveldb/leveldb_database.cc
@@ -268,9 +268,10 @@ static void HistogramLevelDBError(const std::string& histogram_name,
     ParseAndHistogramCorruptionDetails(histogram_name, s);
 }
 
-scoped_ptr<LevelDBDatabase> LevelDBDatabase::Open(
+leveldb::Status LevelDBDatabase::Open(
     const base::FilePath& file_name,
     const LevelDBComparator* comparator,
+    scoped_ptr<LevelDBDatabase>* result,
     bool* is_disk_full) {
   scoped_ptr<ComparatorAdapter> comparator_adapter(
       new ComparatorAdapter(comparator));
@@ -289,17 +290,17 @@ scoped_ptr<LevelDBDatabase> LevelDBDatabase::Open(
 
     LOG(ERROR) << "Failed to open LevelDB database from "
                << file_name.AsUTF8Unsafe() << "," << s.ToString();
-    return scoped_ptr<LevelDBDatabase>();
+    return s;
   }
 
   CheckFreeSpace("Success", file_name);
 
-  scoped_ptr<LevelDBDatabase> result(new LevelDBDatabase);
-  result->db_ = make_scoped_ptr(db);
-  result->comparator_adapter_ = comparator_adapter.Pass();
-  result->comparator_ = comparator;
+  (*result).reset(new LevelDBDatabase);
+  (*result)->db_ = make_scoped_ptr(db);
+  (*result)->comparator_adapter_ = comparator_adapter.Pass();
+  (*result)->comparator_ = comparator;
 
-  return result.Pass();
+  return s;
 }
 
 scoped_ptr<LevelDBDatabase> LevelDBDatabase::OpenInMemory(
diff --git a/content/browser/indexed_db/leveldb/leveldb_database.h b/content/browser/indexed_db/leveldb/leveldb_database.h
index ad1942c..1efeefa 100644
--- a/content/browser/indexed_db/leveldb/leveldb_database.h
+++ b/content/browser/indexed_db/leveldb/leveldb_database.h
@@ -13,6 +13,7 @@
 #include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
 #include "content/common/content_export.h"
+#include "third_party/leveldatabase/src/include/leveldb/status.h"
 
 namespace leveldb {
 class Comparator;
@@ -42,9 +43,10 @@ class LevelDBSnapshot {
 
 class CONTENT_EXPORT LevelDBDatabase {
  public:
-  static scoped_ptr<LevelDBDatabase> Open(const base::FilePath& file_name,
-                                          const LevelDBComparator* comparator,
-                                          bool* is_disk_full = 0);
+  static leveldb::Status Open(const base::FilePath& file_name,
+                              const LevelDBComparator* comparator,
+                              scoped_ptr<LevelDBDatabase>* db,
+                              bool* is_disk_full = 0);
   static scoped_ptr<LevelDBDatabase> OpenInMemory(
       const LevelDBComparator* comparator);
   static bool Destroy(const base::FilePath& file_name);
diff --git a/content/browser/indexed_db/leveldb/leveldb_unittest.cc b/content/browser/indexed_db/leveldb/leveldb_unittest.cc
index ac7ebe0..9ce0664 100644
--- a/content/browser/indexed_db/leveldb/leveldb_unittest.cc
+++ b/content/browser/indexed_db/leveldb/leveldb_unittest.cc
@@ -41,8 +41,8 @@ TEST(LevelDBDatabaseTest, CorruptionTest) {
   std::string got_value;
   SimpleComparator comparator;
 
-  scoped_ptr<LevelDBDatabase> leveldb =
-      LevelDBDatabase::Open(temp_directory.path(), &comparator);
+  scoped_ptr<LevelDBDatabase> leveldb;
+  LevelDBDatabase::Open(temp_directory.path(), &comparator, &leveldb);
   EXPECT_TRUE(leveldb);
   put_value = value;
   bool success = leveldb->Put(key, &put_value);
@@ -50,7 +50,7 @@ TEST(LevelDBDatabaseTest, CorruptionTest) {
   leveldb.Pass();
   EXPECT_FALSE(leveldb);
 
-  leveldb = LevelDBDatabase::Open(temp_directory.path(), &comparator);
+  LevelDBDatabase::Open(temp_directory.path(), &comparator, &leveldb);
   EXPECT_TRUE(leveldb);
   bool found = false;
   success = leveldb->Get(key, &got_value, &found);
@@ -69,13 +69,16 @@ TEST(LevelDBDatabaseTest, CorruptionTest) {
   base::TruncatePlatformFile(handle, 0);
   base::ClosePlatformFile(handle);
 
-  leveldb = LevelDBDatabase::Open(temp_directory.path(), &comparator);
+  leveldb::Status status =
+      LevelDBDatabase::Open(temp_directory.path(), &comparator, &leveldb);
   EXPECT_FALSE(leveldb);
+  EXPECT_FALSE(status.ok());
 
   bool destroyed = LevelDBDatabase::Destroy(temp_directory.path());
   EXPECT_TRUE(destroyed);
 
-  leveldb = LevelDBDatabase::Open(temp_directory.path(), &comparator);
+  status = LevelDBDatabase::Open(temp_directory.path(), &comparator, &leveldb);
+  EXPECT_TRUE(status.ok());
   EXPECT_TRUE(leveldb);
   success = leveldb->Get(key, &got_value, &found);
   EXPECT_TRUE(success);
@@ -91,8 +94,8 @@ TEST(LevelDBDatabaseTest, Transaction) {
   std::string put_value;
   SimpleComparator comparator;
 
-  scoped_ptr<LevelDBDatabase> leveldb =
-      LevelDBDatabase::Open(temp_directory.path(), &comparator);
+  scoped_ptr<LevelDBDatabase> leveldb;
+  LevelDBDatabase::Open(temp_directory.path(), &comparator, &leveldb);
   EXPECT_TRUE(leveldb);
 
   const std::string old_value("value");
@@ -158,8 +161,8 @@ TEST(LevelDBDatabaseTest, TransactionIterator) {
   SimpleComparator comparator;
   bool success;
 
-  scoped_ptr<LevelDBDatabase> leveldb =
-      LevelDBDatabase::Open(temp_directory.path(), &comparator);
+  scoped_ptr<LevelDBDatabase> leveldb;
+  LevelDBDatabase::Open(temp_directory.path(), &comparator, &leveldb);
   EXPECT_TRUE(leveldb);
 
   put_value = value1;