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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: content/browser/service_worker/service_worker_database.cc

Issue 287843002: ServiceWorker: DB functions should return status code instead of boolean (2) (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: address comments partially Created 6 years, 7 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 | « content/browser/service_worker/service_worker_database.h ('k') | content/browser/service_worker/service_worker_database_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/service_worker/service_worker_database.cc
diff --git a/content/browser/service_worker/service_worker_database.cc b/content/browser/service_worker/service_worker_database.cc
index db8f735d792b7bb88360678fcf1d7780c8931ed8..ac853fe0fd80124ddf96801ad9b0cb4e68dd0a99 100644
--- a/content/browser/service_worker/service_worker_database.cc
+++ b/content/browser/service_worker/service_worker_database.cc
@@ -260,18 +260,19 @@ ServiceWorkerStatusCode ServiceWorkerDatabase::GetNextAvailableIds(
DCHECK(next_avail_version_id);
DCHECK(next_avail_resource_id);
- if (!LazyOpen(false)) {
- if (is_disabled_)
- return SERVICE_WORKER_ERROR_FAILED;
+ ServiceWorkerStatusCode status = LazyOpen(false);
+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||
+ (status == SERVICE_WORKER_OK && !is_initialized_)) {
// Database has never been used.
*next_avail_registration_id = 0;
*next_avail_version_id = 0;
*next_avail_resource_id = 0;
return SERVICE_WORKER_OK;
}
+ if (status != SERVICE_WORKER_OK)
+ return status;
- ServiceWorkerStatusCode status =
- ReadNextAvailableId(kNextRegIdKey, &next_avail_registration_id_);
+ status = ReadNextAvailableId(kNextRegIdKey, &next_avail_registration_id_);
if (status != SERVICE_WORKER_OK)
return status;
status = ReadNextAvailableId(kNextVerIdKey, &next_avail_version_id_);
@@ -290,15 +291,16 @@ ServiceWorkerStatusCode ServiceWorkerDatabase::GetNextAvailableIds(
ServiceWorkerStatusCode ServiceWorkerDatabase::GetOriginsWithRegistrations(
std::set<GURL>* origins) {
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
- DCHECK(origins);
+ DCHECK(origins->empty());
- if (!LazyOpen(false)) {
- if (is_disabled_)
- return SERVICE_WORKER_ERROR_FAILED;
+ ServiceWorkerStatusCode status = LazyOpen(false);
+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||
+ (status == SERVICE_WORKER_OK && !is_initialized_)) {
// Database has never been used.
- origins->clear();
return SERVICE_WORKER_OK;
}
+ if (status != SERVICE_WORKER_OK)
+ return status;
scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
for (itr->Seek(kUniqueOriginKey); itr->Valid(); itr->Next()) {
@@ -316,19 +318,20 @@ ServiceWorkerStatusCode ServiceWorkerDatabase::GetOriginsWithRegistrations(
return SERVICE_WORKER_OK;
}
-bool ServiceWorkerDatabase::GetRegistrationsForOrigin(
+ServiceWorkerStatusCode ServiceWorkerDatabase::GetRegistrationsForOrigin(
const GURL& origin,
std::vector<RegistrationData>* registrations) {
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
- DCHECK(registrations);
+ DCHECK(registrations->empty());
- if (!LazyOpen(false)) {
- if (is_disabled_)
- return false;
+ ServiceWorkerStatusCode status = LazyOpen(false);
+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||
+ (status == SERVICE_WORKER_OK && !is_initialized_)) {
// Database has never been used.
- registrations->clear();
- return true;
+ return SERVICE_WORKER_OK;
}
+ if (status != SERVICE_WORKER_OK)
+ return status;
// Create a key prefix for registrations.
std::string prefix = base::StringPrintf(
@@ -339,7 +342,7 @@ bool ServiceWorkerDatabase::GetRegistrationsForOrigin(
if (!itr->status().ok()) {
HandleError(FROM_HERE, itr->status());
registrations->clear();
- return false;
+ return LevelDBStatusToServiceWorkerStatusCode(itr->status());
}
if (!RemovePrefix(itr->key().ToString(), prefix, NULL))
@@ -349,32 +352,33 @@ bool ServiceWorkerDatabase::GetRegistrationsForOrigin(
if (!ParseRegistrationData(itr->value().ToString(), &registration)) {
HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
registrations->clear();
- return false;
+ return SERVICE_WORKER_ERROR_DB_CORRUPTED;
}
registrations->push_back(registration);
}
- return true;
+ return SERVICE_WORKER_OK;
}
-bool ServiceWorkerDatabase::GetAllRegistrations(
+ServiceWorkerStatusCode ServiceWorkerDatabase::GetAllRegistrations(
std::vector<RegistrationData>* registrations) {
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
- DCHECK(registrations);
+ DCHECK(registrations->empty());
- if (!LazyOpen(false)) {
- if (is_disabled_)
- return false;
+ ServiceWorkerStatusCode status = LazyOpen(false);
+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||
+ (status == SERVICE_WORKER_OK && !is_initialized_)) {
// Database has never been used.
- registrations->clear();
- return true;
+ return SERVICE_WORKER_OK;
}
+ if (status != SERVICE_WORKER_OK)
+ return status;
scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
for (itr->Seek(kRegKeyPrefix); itr->Valid(); itr->Next()) {
if (!itr->status().ok()) {
HandleError(FROM_HERE, itr->status());
registrations->clear();
- return false;
+ return LevelDBStatusToServiceWorkerStatusCode(itr->status());
}
if (!RemovePrefix(itr->key().ToString(), kRegKeyPrefix, NULL))
@@ -384,11 +388,11 @@ bool ServiceWorkerDatabase::GetAllRegistrations(
if (!ParseRegistrationData(itr->value().ToString(), &registration)) {
HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
registrations->clear();
- return false;
+ return SERVICE_WORKER_ERROR_DB_CORRUPTED;
}
registrations->push_back(registration);
}
- return true;
+ return SERVICE_WORKER_OK;
}
bool ServiceWorkerDatabase::ReadRegistration(
@@ -400,7 +404,8 @@ bool ServiceWorkerDatabase::ReadRegistration(
DCHECK(registration);
DCHECK(resources);
- if (!LazyOpen(false) || is_disabled_)
+ ServiceWorkerStatusCode status = LazyOpen(false);
+ if (status != SERVICE_WORKER_OK || !is_initialized_)
return false;
RegistrationData value;
@@ -418,7 +423,7 @@ bool ServiceWorkerDatabase::WriteRegistration(
const RegistrationData& registration,
const std::vector<ResourceRecord>& resources) {
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
- if (!LazyOpen(true) || is_disabled_)
+ if (LazyOpen(true) != SERVICE_WORKER_OK)
return false;
leveldb::WriteBatch batch;
@@ -474,7 +479,7 @@ bool ServiceWorkerDatabase::WriteRegistration(
bool ServiceWorkerDatabase::UpdateVersionToActive(int64 registration_id,
const GURL& origin) {
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
- if (!LazyOpen(false) || is_disabled_)
+ if (LazyOpen(false) != SERVICE_WORKER_OK || !is_initialized_)
return false;
RegistrationData registration;
@@ -492,7 +497,7 @@ bool ServiceWorkerDatabase::UpdateLastCheckTime(int64 registration_id,
const GURL& origin,
const base::Time& time) {
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
- if (!LazyOpen(false) || is_disabled_)
+ if (LazyOpen(false) != SERVICE_WORKER_OK || !is_initialized_)
return false;
RegistrationData registration;
@@ -509,7 +514,13 @@ bool ServiceWorkerDatabase::UpdateLastCheckTime(int64 registration_id,
bool ServiceWorkerDatabase::DeleteRegistration(int64 registration_id,
const GURL& origin) {
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
- if (!LazyOpen(false) || is_disabled_)
+ ServiceWorkerStatusCode status = LazyOpen(false);
+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||
+ (status == SERVICE_WORKER_OK && !is_initialized_)) {
+ // Database has never been used.
+ return true;
+ }
+ if (status != SERVICE_WORKER_OK || !origin.is_valid())
return false;
leveldb::WriteBatch batch;
@@ -518,7 +529,7 @@ bool ServiceWorkerDatabase::DeleteRegistration(int64 registration_id,
// |registration_id| is the only one for |origin|.
// TODO(nhiroki): Check the uniqueness by more efficient way.
std::vector<RegistrationData> registrations;
- if (!GetRegistrationsForOrigin(origin, &registrations))
+ if (GetRegistrationsForOrigin(origin, &registrations) != SERVICE_WORKER_OK)
return false;
if (registrations.size() == 1 &&
registrations[0].registration_id == registration_id) {
@@ -571,7 +582,13 @@ bool ServiceWorkerDatabase::ClearPurgeableResourceIds(
bool ServiceWorkerDatabase::DeleteAllDataForOrigin(const GURL& origin) {
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
- if (!LazyOpen(true) || is_disabled_ || !origin.is_valid())
+ ServiceWorkerStatusCode status = LazyOpen(false);
+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||
+ (status == SERVICE_WORKER_OK && !is_initialized_)) {
+ // Database has never been used.
+ return true;
+ }
+ if (status != SERVICE_WORKER_OK || !origin.is_valid())
return false;
leveldb::WriteBatch batch;
@@ -580,7 +597,7 @@ bool ServiceWorkerDatabase::DeleteAllDataForOrigin(const GURL& origin) {
batch.Delete(CreateUniqueOriginKey(origin));
std::vector<RegistrationData> registrations;
- if (!GetRegistrationsForOrigin(origin, &registrations))
+ if (GetRegistrationsForOrigin(origin, &registrations) != SERVICE_WORKER_OK)
return false;
// Delete registrations and resource records.
@@ -594,51 +611,54 @@ bool ServiceWorkerDatabase::DeleteAllDataForOrigin(const GURL& origin) {
return WriteBatch(&batch);
}
-bool ServiceWorkerDatabase::LazyOpen(bool create_if_needed) {
+ServiceWorkerStatusCode ServiceWorkerDatabase::LazyOpen(
+ bool create_if_missing) {
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
- if (IsOpen())
- return true;
// Do not try to open a database if we tried and failed once.
if (is_disabled_)
- return false;
+ return SERVICE_WORKER_ERROR_FAILED;
+ if (IsOpen())
+ return SERVICE_WORKER_OK;
// When |path_| is empty, open a database in-memory.
bool use_in_memory_db = path_.empty();
- if (!create_if_needed) {
+ if (!create_if_missing) {
// Avoid opening a database if it does not exist at the |path_|.
if (use_in_memory_db ||
!base::PathExists(path_) ||
base::IsDirectoryEmpty(path_)) {
- return false;
+ return SERVICE_WORKER_ERROR_NOT_FOUND;
}
}
leveldb::Options options;
- options.create_if_missing = create_if_needed;
+ options.create_if_missing = create_if_missing;
if (use_in_memory_db) {
env_.reset(leveldb::NewMemEnv(leveldb::Env::Default()));
options.env = env_.get();
}
leveldb::DB* db = NULL;
- leveldb::Status status =
+ leveldb::Status db_status =
leveldb::DB::Open(options, path_.AsUTF8Unsafe(), &db);
- if (!status.ok()) {
+ if (!db_status.ok()) {
DCHECK(!db);
// TODO(nhiroki): Should we retry to open the database?
- HandleError(FROM_HERE, status);
- return false;
+ HandleError(FROM_HERE, db_status);
+ return LevelDBStatusToServiceWorkerStatusCode(db_status);
}
db_.reset(db);
int64 db_version;
- if (!ReadDatabaseVersion(&db_version))
- return false;
+ ServiceWorkerStatusCode status = ReadDatabaseVersion(&db_version);
+ if (status != SERVICE_WORKER_OK)
+ return status;
+ DCHECK_LE(0, db_version);
if (db_version > 0)
is_initialized_ = true;
- return true;
+ return SERVICE_WORKER_OK;
}
ServiceWorkerStatusCode ServiceWorkerDatabase::ReadNextAvailableId(
@@ -762,9 +782,15 @@ bool ServiceWorkerDatabase::ReadResourceIds(const char* id_key_prefix,
std::set<int64>* ids) {
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
DCHECK(id_key_prefix);
- DCHECK(ids);
+ DCHECK(ids->clear());
nhiroki 2014/05/16 02:48:20 Oops... will fix this.
- if (!LazyOpen(false) || is_disabled_)
+ ServiceWorkerStatusCode status = LazyOpen(false);
+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||
+ (status == SERVICE_WORKER_OK && !is_initialized_)) {
+ // Database has never been used.
+ return true;
+ }
+ if (status != SERVICE_WORKER_OK)
return false;
scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
@@ -795,7 +821,7 @@ bool ServiceWorkerDatabase::WriteResourceIds(const char* id_key_prefix,
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
DCHECK(id_key_prefix);
- if (!LazyOpen(true) || is_disabled_)
+ if (LazyOpen(true) != SERVICE_WORKER_OK)
return false;
if (ids.empty())
return true;
@@ -814,8 +840,15 @@ bool ServiceWorkerDatabase::DeleteResourceIds(const char* id_key_prefix,
DCHECK(sequence_checker_.CalledOnValidSequencedThread());
DCHECK(id_key_prefix);
- if (!LazyOpen(true) || is_disabled_)
+ ServiceWorkerStatusCode status = LazyOpen(false);
+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||
+ (status == SERVICE_WORKER_OK && !is_initialized_)) {
+ // Database has never been used.
+ return true;
+ }
+ if (status != SERVICE_WORKER_OK)
return false;
+
if (ids.empty())
return true;
@@ -827,34 +860,35 @@ bool ServiceWorkerDatabase::DeleteResourceIds(const char* id_key_prefix,
return WriteBatch(&batch);
}
-bool ServiceWorkerDatabase::ReadDatabaseVersion(int64* db_version) {
+ServiceWorkerStatusCode ServiceWorkerDatabase::ReadDatabaseVersion(
+ int64* db_version) {
std::string value;
leveldb::Status status =
db_->Get(leveldb::ReadOptions(), kDatabaseVersionKey, &value);
if (status.IsNotFound()) {
// The database hasn't been initialized yet.
*db_version = 0;
- return true;
+ return SERVICE_WORKER_OK;
}
if (!status.ok()) {
HandleError(FROM_HERE, status);
- return false;
+ return LevelDBStatusToServiceWorkerStatusCode(status);
}
int64 parsed;
if (!base::StringToInt64(value, &parsed)) {
HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
- return false;
+ return SERVICE_WORKER_ERROR_DB_CORRUPTED;
}
const int kFirstValidVersion = 1;
if (parsed < kFirstValidVersion || kCurrentSchemaVersion < parsed) {
HandleError(FROM_HERE, leveldb::Status::Corruption("invalid DB version"));
- return false;
+ return SERVICE_WORKER_ERROR_DB_CORRUPTED;
}
*db_version = parsed;
- return true;
+ return SERVICE_WORKER_OK;
}
bool ServiceWorkerDatabase::WriteBatch(leveldb::WriteBatch* batch) {
« no previous file with comments | « content/browser/service_worker/service_worker_database.h ('k') | content/browser/service_worker/service_worker_database_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698