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

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

Issue 284123003: ServiceWorker: DB functions should return status code instead of boolean (3) (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: 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.h » ('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 ba06cf42601cd8920b88d5f317792103ba28448f..fb47878328ac65c604dfb9ec560925faf30948c3 100644

--- a/content/browser/service_worker/service_worker_database.cc

+++ b/content/browser/service_worker/service_worker_database.cc

@@ -398,7 +398,7 @@ ServiceWorkerStatusCode ServiceWorkerDatabase::GetAllRegistrations(

return SERVICE_WORKER_OK;

}

-bool ServiceWorkerDatabase::ReadRegistration(

+ServiceWorkerStatusCode ServiceWorkerDatabase::ReadRegistration(

int64 registration_id,

const GURL& origin,

RegistrationData* registration,

@@ -408,26 +408,33 @@ bool ServiceWorkerDatabase::ReadRegistration(

DCHECK(resources);

ServiceWorkerStatusCode status = LazyOpen(false);

- if (status != SERVICE_WORKER_OK || !is_initialized_)

- return false;

+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||

+ (status == SERVICE_WORKER_OK && !is_initialized_)) {

+ return SERVICE_WORKER_ERROR_NOT_FOUND;

+ }

+ if (status != SERVICE_WORKER_OK)

+ return status;

RegistrationData value;

- if (!ReadRegistrationData(registration_id, origin, &value))

- return false;

+ status = ReadRegistrationData(registration_id, origin, &value);

+ if (status != SERVICE_WORKER_OK)

+ return status;

- if (!ReadResourceRecords(value.version_id, resources))

- return false;

+ status = ReadResourceRecords(value.version_id, resources);

+ if (status != SERVICE_WORKER_OK)

+ return status;

*registration = value;

- return true;

+ return SERVICE_WORKER_OK;

}

-bool ServiceWorkerDatabase::WriteRegistration(

+ServiceWorkerStatusCode ServiceWorkerDatabase::WriteRegistration(

const RegistrationData& registration,

const std::vector<ResourceRecord>& resources) {

DCHECK(sequence_checker_.CalledOnValidSequencedThread());

- if (LazyOpen(true) != SERVICE_WORKER_OK)

- return false;

+ ServiceWorkerStatusCode status = LazyOpen(true);

+ if (status != SERVICE_WORKER_OK)

+ return status;

leveldb::WriteBatch batch;

BumpNextRegistrationIdIfNeeded(registration.registration_id, &batch);

@@ -441,20 +448,21 @@ bool ServiceWorkerDatabase::WriteRegistration(

// Retrieve a previous version to sweep purgeable resources.

RegistrationData old_registration;

- if (!ReadRegistrationData(registration.registration_id,

- registration.scope.GetOrigin(),

- &old_registration)) {

- if (is_disabled_)

- return false;

- // Just not found.

+ status = ReadRegistrationData(registration.registration_id,

+ registration.scope.GetOrigin(),

+ &old_registration);

+ if (status != SERVICE_WORKER_OK) {

+ if (status != SERVICE_WORKER_ERROR_NOT_FOUND)

michaeln 2014/05/15 20:44:58 nit: could be restructured to only utilize early r

nhiroki 2014/05/19 04:58:04 Done.

+ return status;

} else {

DCHECK_LT(old_registration.version_id, registration.version_id);

// Currently resource sharing across versions and registrations is not

// suppported, so resource ids should not be overlapped between

// |registration| and |old_registration|.

// TODO(nhiroki): Add DCHECK to make sure the overlap does not exist.

- if (!DeleteResourceRecords(old_registration.version_id, &batch))

- return false;

+ status = DeleteResourceRecords(old_registration.version_id, &batch);

+ if (status != SERVICE_WORKER_OK)

+ return status;

}

// Used for avoiding multiple writes for the same resource id or url.

@@ -463,7 +471,7 @@ bool ServiceWorkerDatabase::WriteRegistration(

for (std::vector<ResourceRecord>::const_iterator itr = resources.begin();

itr != resources.end(); ++itr) {

if (!itr->url.is_valid())

- return false;

+ return SERVICE_WORKER_ERROR_FAILED;

// Duplicated resource id or url should not exist.

DCHECK(pushed_resources.insert(itr->resource_id).second);

@@ -479,15 +487,25 @@ bool ServiceWorkerDatabase::WriteRegistration(

return WriteBatch(&batch);

}

-bool ServiceWorkerDatabase::UpdateVersionToActive(int64 registration_id,

- const GURL& origin) {

+ServiceWorkerStatusCode ServiceWorkerDatabase::UpdateVersionToActive(

+ int64 registration_id,

+ const GURL& origin) {

DCHECK(sequence_checker_.CalledOnValidSequencedThread());

- if (LazyOpen(false) != SERVICE_WORKER_OK || !is_initialized_)

- return false;

+ ServiceWorkerStatusCode status = LazyOpen(false);

+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||

+ (status == SERVICE_WORKER_OK && !is_initialized_)) {

+ // Database has never been used.

+ return SERVICE_WORKER_ERROR_NOT_FOUND;

+ }

+ if (status != SERVICE_WORKER_OK)

+ return status;

+ if (!origin.is_valid())

+ return SERVICE_WORKER_ERROR_FAILED;

RegistrationData registration;

- if (!ReadRegistrationData(registration_id, origin, &registration))

- return false;

+ status = ReadRegistrationData(registration_id, origin, &registration);

+ if (status != SERVICE_WORKER_OK)

+ return status;

registration.is_active = true;

@@ -496,16 +514,26 @@ bool ServiceWorkerDatabase::UpdateVersionToActive(int64 registration_id,

return WriteBatch(&batch);

}

-bool ServiceWorkerDatabase::UpdateLastCheckTime(int64 registration_id,

- const GURL& origin,

- const base::Time& time) {

+ServiceWorkerStatusCode ServiceWorkerDatabase::UpdateLastCheckTime(

+ int64 registration_id,

+ const GURL& origin,

+ const base::Time& time) {

DCHECK(sequence_checker_.CalledOnValidSequencedThread());

- if (LazyOpen(false) != SERVICE_WORKER_OK || !is_initialized_)

- return false;

+ ServiceWorkerStatusCode status = LazyOpen(false);

+ if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||

+ (status == SERVICE_WORKER_OK && !is_initialized_)) {

michaeln 2014/05/15 20:44:58 this test is a recurring theme, stuffing it in a h

nhiroki 2014/05/19 04:58:04 Done.

+ // Database has never been used.

+ return SERVICE_WORKER_ERROR_NOT_FOUND;

+ }

+ if (status != SERVICE_WORKER_OK)

+ return status;

+ if (!origin.is_valid())

+ return SERVICE_WORKER_ERROR_FAILED;

RegistrationData registration;

- if (!ReadRegistrationData(registration_id, origin, &registration))

- return false;

+ status = ReadRegistrationData(registration_id, origin, &registration);

+ if (status != SERVICE_WORKER_OK)

+ return status;

registration.last_update_check = time;

@@ -514,17 +542,20 @@ bool ServiceWorkerDatabase::UpdateLastCheckTime(int64 registration_id,

return WriteBatch(&batch);

}

-bool ServiceWorkerDatabase::DeleteRegistration(int64 registration_id,

- const GURL& origin) {

+ServiceWorkerStatusCode ServiceWorkerDatabase::DeleteRegistration(

+ int64 registration_id,

+ const GURL& origin) {

DCHECK(sequence_checker_.CalledOnValidSequencedThread());

ServiceWorkerStatusCode status = LazyOpen(false);

if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||

(status == SERVICE_WORKER_OK && !is_initialized_)) {

// Database has never been used.

- return true;

+ return SERVICE_WORKER_ERROR_NOT_FOUND;

}

- if (status != SERVICE_WORKER_OK || !origin.is_valid())

- return false;

+ if (status != SERVICE_WORKER_OK)

+ return status;

+ if (!origin.is_valid())

+ return SERVICE_WORKER_ERROR_FAILED;

leveldb::WriteBatch batch;

@@ -532,8 +563,9 @@ 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) != SERVICE_WORKER_OK)

- return false;

+ status = GetRegistrationsForOrigin(origin, &registrations);

+ if (status != SERVICE_WORKER_OK)

+ return status;

if (registrations.size() == 1 &&

registrations[0].registration_id == registration_id) {

batch.Delete(CreateUniqueOriginKey(origin));

@@ -546,8 +578,9 @@ bool ServiceWorkerDatabase::DeleteRegistration(int64 registration_id,

for (std::vector<RegistrationData>::const_iterator itr =

registrations.begin(); itr != registrations.end(); ++itr) {

if (itr->registration_id == registration_id) {

- if (!DeleteResourceRecords(itr->version_id, &batch))

- return false;

+ status = DeleteResourceRecords(itr->version_id, &batch);

+ if (status != SERVICE_WORKER_OK)

+ return status;

break;

}

@@ -583,16 +616,19 @@ bool ServiceWorkerDatabase::ClearPurgeableResourceIds(

return DeleteResourceIds(kPurgeableResIdKeyPrefix, ids);

}

-bool ServiceWorkerDatabase::DeleteAllDataForOrigin(const GURL& origin) {

+ServiceWorkerStatusCode ServiceWorkerDatabase::DeleteAllDataForOrigin(

+ const GURL& origin) {

DCHECK(sequence_checker_.CalledOnValidSequencedThread());

ServiceWorkerStatusCode status = LazyOpen(false);

if (status == SERVICE_WORKER_ERROR_NOT_FOUND ||

(status == SERVICE_WORKER_OK && !is_initialized_)) {

// Database has never been used.

- return true;

+ return SERVICE_WORKER_ERROR_NOT_FOUND;

}

- if (status != SERVICE_WORKER_OK || !origin.is_valid())

- return false;

+ if (status != SERVICE_WORKER_OK)

+ return status;

+ if (!origin.is_valid())

+ return SERVICE_WORKER_ERROR_FAILED;

leveldb::WriteBatch batch;

@@ -600,15 +636,17 @@ bool ServiceWorkerDatabase::DeleteAllDataForOrigin(const GURL& origin) {

batch.Delete(CreateUniqueOriginKey(origin));

std::vector<RegistrationData> registrations;

- if (GetRegistrationsForOrigin(origin, &registrations) != SERVICE_WORKER_OK)

- return false;

+ status = GetRegistrationsForOrigin(origin, &registrations);

+ if (status != SERVICE_WORKER_OK)

+ return status;

// Delete registrations and resource records.

for (std::vector<RegistrationData>::const_iterator itr =

registrations.begin(); itr != registrations.end(); ++itr) {

batch.Delete(CreateRegistrationKey(itr->registration_id, origin));

- if (!DeleteResourceRecords(itr->version_id, &batch))

- return false;

+ status = DeleteResourceRecords(itr->version_id, &batch);

+ if (status != SERVICE_WORKER_OK)

+ return status;

}

return WriteBatch(&batch);

@@ -693,7 +731,7 @@ ServiceWorkerStatusCode ServiceWorkerDatabase::ReadNextAvailableId(

return SERVICE_WORKER_OK;

}

-bool ServiceWorkerDatabase::ReadRegistrationData(

+ServiceWorkerStatusCode ServiceWorkerDatabase::ReadRegistrationData(

int64 registration_id,

const GURL& origin,

RegistrationData* registration) {

@@ -706,20 +744,20 @@ bool ServiceWorkerDatabase::ReadRegistrationData(

if (!status.ok()) {

if (!status.IsNotFound())

HandleError(FROM_HERE, status);

- return false;

+ return LevelDBStatusToServiceWorkerStatusCode(status);

}

RegistrationData parsed;

if (!ParseRegistrationData(value, &parsed)) {

HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));

- return false;

+ return SERVICE_WORKER_ERROR_DB_CORRUPTED;

}

*registration = parsed;

- return true;

+ return SERVICE_WORKER_OK;

}

-bool ServiceWorkerDatabase::ReadResourceRecords(

+ServiceWorkerStatusCode ServiceWorkerDatabase::ReadResourceRecords(

int64 version_id,

std::vector<ResourceRecord>* resources) {

DCHECK(resources);

@@ -730,7 +768,7 @@ bool ServiceWorkerDatabase::ReadResourceRecords(

if (!itr->status().ok()) {

HandleError(FROM_HERE, itr->status());

resources->clear();

- return false;

+ return LevelDBStatusToServiceWorkerStatusCode(itr->status());

}

if (!RemovePrefix(itr->key().ToString(), prefix, NULL))

@@ -740,14 +778,14 @@ bool ServiceWorkerDatabase::ReadResourceRecords(

if (!ParseResourceRecord(itr->value().ToString(), &resource)) {

HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));

resources->clear();

- return false;

+ return SERVICE_WORKER_ERROR_DB_CORRUPTED;

}

resources->push_back(resource);

}

- return true;

+ return SERVICE_WORKER_OK;

}

-bool ServiceWorkerDatabase::DeleteResourceRecords(

+ServiceWorkerStatusCode ServiceWorkerDatabase::DeleteResourceRecords(

int64 version_id,

leveldb::WriteBatch* batch) {

DCHECK(batch);

@@ -757,7 +795,7 @@ bool ServiceWorkerDatabase::DeleteResourceRecords(

for (itr->Seek(prefix); itr->Valid(); itr->Next()) {

if (!itr->status().ok()) {

HandleError(FROM_HERE, itr->status());

- return false;

+ return LevelDBStatusToServiceWorkerStatusCode(itr->status());

}

std::string key = itr->key().ToString();

@@ -768,7 +806,7 @@ bool ServiceWorkerDatabase::DeleteResourceRecords(

int64 resource_id;

if (!base::StringToInt64(unprefixed, &resource_id)) {

HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));

- return false;

+ return SERVICE_WORKER_ERROR_DB_CORRUPTED;

}

// Remove a resource record.

@@ -778,7 +816,7 @@ bool ServiceWorkerDatabase::DeleteResourceRecords(

// supported, so we can purge this without caring about it.

PutPurgeableResourceIdToBatch(resource_id, batch);

}

- return true;

+ return SERVICE_WORKER_OK;

}

bool ServiceWorkerDatabase::ReadResourceIds(const char* id_key_prefix,

@@ -836,7 +874,10 @@ bool ServiceWorkerDatabase::WriteResourceIds(const char* id_key_prefix,

// Value should be empty.

batch.Put(CreateResourceIdKey(id_key_prefix, *itr), "");

}

- return WriteBatch(&batch);

+ if (WriteBatch(&batch) != SERVICE_WORKER_OK)

+ return false;

+ return true;

michaeln 2014/05/15 20:44:58 nit: return WriteBatch() == OK;

nhiroki 2014/05/19 04:58:04 Removed this branch.

}

bool ServiceWorkerDatabase::DeleteResourceIds(const char* id_key_prefix,

@@ -861,7 +902,10 @@ bool ServiceWorkerDatabase::DeleteResourceIds(const char* id_key_prefix,

itr != ids.end(); ++itr) {

batch.Delete(CreateResourceIdKey(id_key_prefix, *itr));

}

- return WriteBatch(&batch);

+ if (WriteBatch(&batch) != SERVICE_WORKER_OK)

+ return false;

+ return true;

michaeln 2014/05/15 20:44:58 ditto

nhiroki 2014/05/19 04:58:04 Done.

}

ServiceWorkerStatusCode ServiceWorkerDatabase::ReadDatabaseVersion(

@@ -895,7 +939,8 @@ ServiceWorkerStatusCode ServiceWorkerDatabase::ReadDatabaseVersion(

return SERVICE_WORKER_OK;

}

-bool ServiceWorkerDatabase::WriteBatch(leveldb::WriteBatch* batch) {

+ServiceWorkerStatusCode ServiceWorkerDatabase::WriteBatch(

+ leveldb::WriteBatch* batch) {

DCHECK(batch);

DCHECK(!is_disabled_);

@@ -906,11 +951,9 @@ bool ServiceWorkerDatabase::WriteBatch(leveldb::WriteBatch* batch) {

}

leveldb::Status status = db_->Write(leveldb::WriteOptions(), batch);

- if (!status.ok()) {

+ if (!status.ok())

HandleError(FROM_HERE, status);

- return false;

- }

- return true;

+ return LevelDBStatusToServiceWorkerStatusCode(status);

}

void ServiceWorkerDatabase::BumpNextRegistrationIdIfNeeded(