content/browser/service_worker/service_worker_database.cc - Issue 287843002: ServiceWorker: DB functions should return status code instead of boolean (2)

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: fix 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') | content/browser/service_worker/service_worker_storage.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | 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..ba06cf42601cd8920b88d5f317792103ba28448f 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_);

@@ -292,13 +293,15 @@ ServiceWorkerStatusCode ServiceWorkerDatabase::GetOriginsWithRegistrations(

DCHECK(sequence_checker_.CalledOnValidSequencedThread());

DCHECK(origins);

- 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();

michaeln 2014/05/15 19:59:30 if origins needs to be cleared here, then it also

nhiroki 2014/05/16 02:11:11 Done.

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 +319,21 @@ 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);

- 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();

michaeln 2014/05/15 19:59:30 ditto clearing the collection

nhiroki 2014/05/16 02:11:11 Done.

- 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 +344,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 +354,34 @@ 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);

- 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();

michaeln 2014/05/15 19:59:30 ditt

nhiroki 2014/05/16 02:11:11 Done.

- 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 +391,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 +407,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 +426,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 +482,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 +500,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 +517,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 +532,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 +585,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 +600,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 +614,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(

@@ -764,7 +787,14 @@ bool ServiceWorkerDatabase::ReadResourceIds(const char* id_key_prefix,

DCHECK(id_key_prefix);

DCHECK(ids);

- 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.

+ ids->clear();

michaeln 2014/05/15 19:59:30 ditto clearing collection

nhiroki 2014/05/16 02:11:11 Done.

+ return true;

+ }

+ if (status != SERVICE_WORKER_OK)

return false;

scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));

@@ -795,7 +825,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 +844,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 +864,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) {