Don't reset the database when there was an I/O error.

content/browser/indexed_db/indexed_db_backing_store.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/indexed_db/indexed_db_backing_store.h"
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_piece.h"
@@ skipping 378 matching lines @@
   INDEXED_DB_BACKING_STORE_OPEN_SUCCESS,
   INDEXED_DB_BACKING_STORE_OPEN_FAILED_DIRECTORY,
   INDEXED_DB_BACKING_STORE_OPEN_FAILED_UNKNOWN_SCHEMA,
   INDEXED_DB_BACKING_STORE_OPEN_CLEANUP_DESTROY_FAILED,
   INDEXED_DB_BACKING_STORE_OPEN_CLEANUP_REOPEN_FAILED,
   INDEXED_DB_BACKING_STORE_OPEN_CLEANUP_REOPEN_SUCCESS,
   INDEXED_DB_BACKING_STORE_OPEN_FAILED_IO_ERROR_CHECKING_SCHEMA,
   INDEXED_DB_BACKING_STORE_OPEN_FAILED_UNKNOWN_ERR,
   INDEXED_DB_BACKING_STORE_OPEN_MEMORY_FAILED,
   INDEXED_DB_BACKING_STORE_OPEN_ATTEMPT_NON_ASCII,
-  INDEXED_DB_BACKING_STORE_OPEN_DISK_FULL,
+  INDEXED_DB_BACKING_STORE_OPEN_DISK_FULL_DEPRECATED,
   INDEXED_DB_BACKING_STORE_OPEN_ORIGIN_TOO_LONG,
   INDEXED_DB_BACKING_STORE_OPEN_NO_RECOVERY,
   INDEXED_DB_BACKING_STORE_OPEN_MAX,
 };
 
-// TODO(dgrogan): Move to leveldb_env.
-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,
     const std::string& file_identifier,
     WebKit::WebIDBCallbacks::DataLoss* data_loss,
     bool* disk_full) {
   *data_loss = WebKit::WebIDBCallbacks::DataLossNone;
   DefaultLevelDBFactory leveldb_factory;
   return IndexedDBBackingStore::Open(origin_identifier,
                                      path_base,
@@ skipping 93 matching lines @@
     } else if (!known) {
       LOG(ERROR) << "IndexedDB backing store had unknown schema, treating it "
                     "as failure to open";
       HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_FAILED_UNKNOWN_SCHEMA);
       db.reset();
     }
   }
 
   if (db) {
     HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_SUCCESS);
-  } else if (!RecoveryCouldBeFruitful(status)) {
+  } else if (!leveldb_env::RecoveryCouldBeFruitful(status)) {
     LOG(ERROR) << "Unable to open backing store, not trying to recover - "
                << status.ToString();
     HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_NO_RECOVERY);
     return scoped_refptr<IndexedDBBackingStore>();
-  } else if (*is_disk_full) {

[jsbell, 2013/10/04 23:32:33]

Just to confirm my reading:

* RecoveryCouldBeFruitful will now return true for ENOSPC (etc) so this would
never be reached.
* We already histogram ENOSPC (etc) via RecordOSError so we're not losing any
logging coverage

[dgrogan, 2013/10/05 01:06:16]

This could be reached if there was <100k free space and none of the IO
operations returned ENOSPC. But its purpose was to allow for full disks to
manifest as all kinds of errors, not just ENOSPC, which supposedly used to
happen on CrOS and it didn't make sense to reset the db in those cases. But now
that we don't reset the db in the case of any IO error, we don't need this.
Correct.

-    LOG(ERROR) << "Unable to open backing store - disk is full.";
-    HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_DISK_FULL);

[jsbell, 2013/10/04 23:32:33]

I'm guessing we're not gaining anything by logging disk full at the IDB layer in
addition to the leveldb layer.

[dgrogan, 2013/10/05 01:06:16]

Yeah, we still have
https://uma.googleplex.com/histograms/?dayCount=1&endDate=10-03-2013&version=...

which shows that full disks still cause a large portion of failures.

-    return scoped_refptr<IndexedDBBackingStore>();
   } else {
     LOG(ERROR) << "IndexedDB backing store open failed, attempting cleanup";
     *data_loss = WebKit::WebIDBCallbacks::DataLossTotal;
     bool success = leveldb_factory->DestroyLevelDB(file_path);
     if (!success) {
       LOG(ERROR) << "IndexedDB backing store cleanup failed";
       HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_CLEANUP_DESTROY_FAILED);
       return scoped_refptr<IndexedDBBackingStore>();
     }
 
@@ skipping 2000 matching lines @@
 }
 
 void IndexedDBBackingStore::Transaction::Rollback() {
   IDB_TRACE("IndexedDBBackingStore::Transaction::Rollback");
   DCHECK(transaction_.get());
   transaction_->Rollback();
   transaction_ = NULL;
 }
 
 }  // namespace content