Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(88)

Unified Diff: content/browser/indexed_db/indexed_db_backing_store.cc

Issue 25541006: Extract two methods by refactoring IndexedDBBackingStore::Open. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 7 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/indexed_db/indexed_db_backing_store.cc
diff --git a/content/browser/indexed_db/indexed_db_backing_store.cc b/content/browser/indexed_db/indexed_db_backing_store.cc
index ee3ba135b16e050c5b7a4f393857413bdcd6c4d6..c797236d57132b7121121ed5740a4de9acf941be 100644
--- a/content/browser/indexed_db/indexed_db_backing_store.cc
+++ b/content/browser/indexed_db/indexed_db_backing_store.cc
@@ -384,22 +384,22 @@ IndexedDBBackingStore::RecordIdentifier::~RecordIdentifier() {}
IndexedDBBackingStore::Cursor::CursorOptions::CursorOptions() {}
IndexedDBBackingStore::Cursor::CursorOptions::~CursorOptions() {}
-enum IndexedDBLevelDBBackingStoreOpenResult {
- INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_MEMORY_SUCCESS,
- INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_SUCCESS,
- INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_FAILED_DIRECTORY,
- INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_FAILED_UNKNOWN_SCHEMA,
- INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_CLEANUP_DESTROY_FAILED,
- INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_CLEANUP_REOPEN_FAILED,
- INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_CLEANUP_REOPEN_SUCCESS,
- INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_FAILED_IO_ERROR_CHECKING_SCHEMA,
- INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_FAILED_UNKNOWN_ERR,
- INDEXED_DB_LEVEL_DB_BACKING_STORE_OPEN_MEMORY_FAILED,
- 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,
+enum IndexedDBBackingStoreOpenResult {
+ INDEXED_DB_BACKING_STORE_OPEN_MEMORY_SUCCESS,
+ 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_ORIGIN_TOO_LONG,
+ INDEXED_DB_BACKING_STORE_OPEN_NO_RECOVERY,
+ INDEXED_DB_BACKING_STORE_OPEN_MAX,
};
// TODO(dgrogan): Move to leveldb_env.
@@ -454,6 +454,44 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
&leveldb_factory);
}
+static void HistogramOpenStatus(IndexedDBBackingStoreOpenResult result) {
+ UMA_HISTOGRAM_ENUMERATION("WebCore.IndexedDB.BackingStore.OpenStatus",
+ result,
+ INDEXED_DB_BACKING_STORE_OPEN_MAX);
+}
+
+// TODO(dgrogan): This only checks the lengths of the path components. It should
+// also check the total path length against MAX_PATH.
jsbell 2013/10/01 23:56:46 I believe this is done implicitly by the call to G
dgrogan 2013/10/02 00:54:24 True. Comment removed.
+static bool IsPathTooLong(const base::FilePath& leveldb_dir) {
+ int limit = file_util::GetMaximumPathComponentLength(leveldb_dir);
jsbell 2013/10/01 23:56:46 Based on the windows impl of GetMaximumPathCompone
dgrogan 2013/10/02 00:54:24 Good catch. Changed back to only using the base di
+ if (limit == -1) {
+ DLOG(WARNING) << "GetMaximumPathComponentLength returned -1";
+ // In limited testing, ChromeOS returns 143, other OSes 255.
jsbell 2013/10/01 23:56:46 Bonus points if you can make clang-format not want
+#if defined(OS_CHROMEOS)
+ limit = 143;
+#else
+ limit = 255;
+#endif
+ }
+ size_t component_length = leveldb_dir.BaseName().value().length();
+ if (component_length > static_cast<uint32_t>(limit)) {
+ DLOG(WARNING) << "Path component length (" << component_length
+ << ") exceeds maximum (" << limit
+ << ") allowed by this filesystem.";
+ const int min = 140;
+ const int max = 300;
+ const int num_buckets = 12;
+ UMA_HISTOGRAM_CUSTOM_COUNTS(
+ "WebCore.IndexedDB.BackingStore.OverlyLargeOriginLength",
+ component_length,
+ min,
+ max,
+ num_buckets);
+ return true;
+ }
+ return false;
+}
+
scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
const std::string& origin_identifier,
const base::FilePath& path_base,
@@ -469,67 +507,24 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
scoped_ptr<LevelDBComparator> comparator(new Comparator());
if (!IsStringASCII(path_base.AsUTF8Unsafe())) {
- 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_ATTEMPT_NON_ASCII);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_ATTEMPT_NON_ASCII);
}
if (!file_util::CreateDirectory(path_base)) {
LOG(ERROR) << "Unable to create IndexedDB database path "
<< path_base.AsUTF8Unsafe();
- 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_FAILED_DIRECTORY);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_FAILED_DIRECTORY);
return scoped_refptr<IndexedDBBackingStore>();
}
- base::FilePath identifier_path =
- base::FilePath().AppendASCII(origin_identifier)
+ base::FilePath file_path =
+ path_base.AppendASCII(origin_identifier)
.AddExtension(FILE_PATH_LITERAL(".indexeddb.leveldb"));
- int limit = file_util::GetMaximumPathComponentLength(path_base);
- if (limit == -1) {
- DLOG(WARNING) << "GetMaximumPathComponentLength returned -1";
- // In limited testing, ChromeOS returns 143, other OSes 255.
-#if defined(OS_CHROMEOS)
- limit = 143;
-#else
- limit = 255;
-#endif
- }
- if (identifier_path.value().length() > static_cast<uint32_t>(limit)) {
- DLOG(WARNING) << "Path component length ("
- << identifier_path.value().length() << ") exceeds maximum ("
- << limit << ") allowed by this filesystem.";
- const int min = 140;
- const int max = 300;
- const int num_buckets = 12;
- // TODO(dgrogan): Remove WebCore from these histogram names.
- UMA_HISTOGRAM_CUSTOM_COUNTS(
- "WebCore.IndexedDB.BackingStore.OverlyLargeOriginLength",
- identifier_path.value().length(),
- min,
- max,
- num_buckets);
- // TODO(dgrogan): Translate the FactoryGet calls to
- // UMA_HISTOGRAM_ENUMERATION. FactoryGet was the most direct translation
- // from the WebCore code.
- 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_ORIGIN_TOO_LONG);
+ if (IsPathTooLong(file_path)) {
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_ORIGIN_TOO_LONG);
return scoped_refptr<IndexedDBBackingStore>();
}
- base::FilePath file_path = path_base.Append(identifier_path);
-
scoped_ptr<LevelDBDatabase> db;
leveldb::Status status = leveldb_factory->OpenLevelDB(
file_path, comparator.get(), &db, is_disk_full);
@@ -545,53 +540,27 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
if (!ok) {
LOG(ERROR) << "IndexedDB had IO error checking schema, treating it as "
"failure to open";
- 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_FAILED_IO_ERROR_CHECKING_SCHEMA);
+ HistogramOpenStatus(
+ INDEXED_DB_BACKING_STORE_OPEN_FAILED_IO_ERROR_CHECKING_SCHEMA);
db.reset();
} else if (!known) {
LOG(ERROR) << "IndexedDB backing store had unknown schema, treating it "
"as failure to open";
- 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_FAILED_UNKNOWN_SCHEMA);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_FAILED_UNKNOWN_SCHEMA);
db.reset();
}
}
if (db) {
- 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_SUCCESS);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_SUCCESS);
} 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);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_NO_RECOVERY);
return scoped_refptr<IndexedDBBackingStore>();
} else if (*is_disk_full) {
LOG(ERROR) << "Unable to open backing store - disk is full.";
- 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_DISK_FULL);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_DISK_FULL);
return scoped_refptr<IndexedDBBackingStore>();
} else {
LOG(ERROR) << "IndexedDB backing store open failed, attempting cleanup";
@@ -599,13 +568,7 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
bool success = leveldb_factory->DestroyLevelDB(file_path);
if (!success) {
LOG(ERROR) << "IndexedDB backing store cleanup failed";
- 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_CLEANUP_DESTROY_FAILED);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_CLEANUP_DESTROY_FAILED);
return scoped_refptr<IndexedDBBackingStore>();
}
@@ -613,31 +576,15 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
leveldb_factory->OpenLevelDB(file_path, comparator.get(), &db, NULL);
if (!db) {
LOG(ERROR) << "IndexedDB backing store reopen after recovery failed";
- 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_CLEANUP_REOPEN_FAILED);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_CLEANUP_REOPEN_FAILED);
return scoped_refptr<IndexedDBBackingStore>();
}
- 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_CLEANUP_REOPEN_SUCCESS);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_CLEANUP_REOPEN_SUCCESS);
}
if (!db) {
NOTREACHED();
- 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_FAILED_UNKNOWN_ERR);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_FAILED_UNKNOWN_ERR);
return scoped_refptr<IndexedDBBackingStore>();
}
@@ -660,20 +607,10 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::OpenInMemory(
LevelDBDatabase::OpenInMemory(comparator.get());
if (!db) {
LOG(ERROR) << "LevelDBDatabase::OpenInMemory failed.";
- 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_MEMORY_FAILED);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_MEMORY_FAILED);
return scoped_refptr<IndexedDBBackingStore>();
}
- 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_MEMORY_SUCCESS);
+ HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_MEMORY_SUCCESS);
return Create(file_identifier, db.Pass(), comparator.Pass());
}
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698