ServiceWorker: DB functions should return status code instead of boolean (2)

content/browser/service_worker/service_worker_storage.cc

 // Copyright 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/service_worker/service_worker_storage.h"
 
 #include <string>
 
 #include "base/bind_helpers.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 107 matching lines @@
   DCHECK(database);
   if (!database->DeleteRegistration(registration_id, origin)) {
     original_task_runner->PostTask(
         FROM_HERE, base::Bind(callback, false, SERVICE_WORKER_ERROR_FAILED));
     return;
   }
 
   // TODO(nhiroki): Add convenient method to ServiceWorkerDatabase to check the
   // unique origin list.
   std::vector<ServiceWorkerDatabase::RegistrationData> registrations;
-  if (!database->GetRegistrationsForOrigin(origin, &registrations)) {
+  ServiceWorkerStatusCode status =
+      database->GetRegistrationsForOrigin(origin, &registrations);
+  if (status != SERVICE_WORKER_OK) {
     original_task_runner->PostTask(
-        FROM_HERE, base::Bind(callback, false, SERVICE_WORKER_ERROR_FAILED));
+        FROM_HERE, base::Bind(callback, false, status));
     return;
   }
 
   bool deletable = registrations.empty();
   original_task_runner->PostTask(
       FROM_HERE, base::Bind(callback, deletable, SERVICE_WORKER_OK));
 }
 
 void UpdateToActiveStateInDB(
     ServiceWorkerDatabase* database,
@@ skipping 380 matching lines @@
        it != pending_tasks_.end(); ++it) {
     RunSoon(FROM_HERE, *it);
   }
   pending_tasks_.clear();
 }
 
 void ServiceWorkerStorage::DidGetRegistrationsForPattern(
     const GURL& scope,
     const FindRegistrationCallback& callback,
     RegistrationList* registrations,
-    bool success) {
+    ServiceWorkerStatusCode status) {
   DCHECK(registrations);
-  if (!success) {
+  if (status != SERVICE_WORKER_OK) {
+    // TODO(nhiroki): Handle database error (http://crbug.com/371675).
     callback.Run(SERVICE_WORKER_ERROR_FAILED,

[michaeln, 2014/05/15 19:59:30]

should |status| be passed in here?

[nhiroki, 2014/05/16 02:11:11]

If we handle DB related errors like DB_CORRUPTED in this class, I think it would
be better not to propagate such errors to the upper class. Adding DB specific
error codes that you mentioned looks good. I'll try it in this CL.

                  scoped_refptr<ServiceWorkerRegistration>());
     return;
   }
 
   // Find one with a matching scope.
   for (RegistrationList::const_iterator it = registrations->begin();
        it != registrations->end(); ++it) {
     if (scope == it->scope) {
       scoped_refptr<ServiceWorkerRegistration> registration =
           context_->GetLiveRegistration(it->registration_id);
@@ skipping 13 matching lines @@
   }
 
   callback.Run(SERVICE_WORKER_ERROR_NOT_FOUND,
                scoped_refptr<ServiceWorkerRegistration>());
 }
 
 void ServiceWorkerStorage::DidGetRegistrationsForDocument(
     const GURL& document_url,
     const FindRegistrationCallback& callback,
     RegistrationList* registrations,
-    bool success) {
+    ServiceWorkerStatusCode status) {
   DCHECK(registrations);
-  if (!success) {
+  if (status != SERVICE_WORKER_OK) {
+    // TODO(nhiroki): Handle database error (http://crbug.com/371675).
     callback.Run(SERVICE_WORKER_ERROR_FAILED,

[michaeln, 2014/05/15 19:59:30]

ditto |status|? dunno if it should just asking

                  scoped_refptr<ServiceWorkerRegistration>());
     return;
   }
 
   // Find one with a pattern match.
   for (RegistrationList::const_iterator it = registrations->begin();
        it != registrations->end(); ++it) {
     // TODO(michaeln): if there are multiple matches the one with
     // the longest scope should win.
     if (ServiceWorkerUtils::ScopeMatches(it->scope, document_url)) {
@@ skipping 45 matching lines @@
     return;
   }
 
   callback.Run(status, scoped_refptr<ServiceWorkerRegistration>());
   return;
 }
 
 void ServiceWorkerStorage::DidGetAllRegistrations(
     const GetAllRegistrationInfosCallback& callback,
     RegistrationList* registrations,
-    bool success) {
+    ServiceWorkerStatusCode status) {
   DCHECK(registrations);
-  if (!success) {
+  if (status != SERVICE_WORKER_OK) {
+    // TODO(nhiroki): Handle database error (http://crbug.com/371675).
     callback.Run(std::vector<ServiceWorkerRegistrationInfo>());
     return;
   }
 
   // Add all stored registrations.
   std::set<int64> pushed_registrations;
   std::vector<ServiceWorkerRegistrationInfo> infos;
   for (RegistrationList::const_iterator it = registrations->begin();
        it != registrations->end(); ++it) {
     DCHECK(pushed_registrations.insert(it->registration_id).second);
@@ skipping 120 matching lines @@
   // TODO(michaeln): Store data on disk and do error checking.
   disk_cache_.reset(new ServiceWorkerDiskCache);
   int rv = disk_cache_->InitWithMemBackend(
       kMaxMemDiskCacheSize,
       base::Bind(&EmptyCompletionCallback));
   DCHECK_EQ(net::OK, rv);
   return disk_cache_.get();
 }
 
 }  // namespace content