ServiceWorker: Store registration data in ServiceWorkerDatabase

content/browser/service_worker/service_worker_database.cc

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 77e33473f439b968c3b8be3c9d0900cab5e45e01..bb909dee9f67669bb8a459aaee751219486c4602 100644
--- a/content/browser/service_worker/service_worker_database.cc
+++ b/content/browser/service_worker/service_worker_database.cc
@@ -7,8 +7,12 @@
 #include <string>
 
 #include "base/file_util.h"
+#include "base/location.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
+#include "content/browser/service_worker/service_worker_database.pb.h"

[michaeln, 2014/04/24 00:35:04]

so a build step on the .proto file produces c structs here... is that right?

[nhiroki, 2014/04/24 05:57:30]

Yes, you're right.

 #include "third_party/leveldatabase/src/helpers/memenv/memenv.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "third_party/leveldatabase/src/include/leveldb/env.h"
@@ -17,18 +21,37 @@
 // LevelDB database schema
 // =======================
 //
+// NOTE
+// - int64 value is serialized as a string by base::Int64ToString().
+//
 // Version 1 (in sorted order)
 //   key: "DB_VERSION"
-//   value: <int64 serialized as a string>
+//   value: "1"
 //
 //   key: "NEXT_REGISTRATION_ID"
-//   value: <int64 serialized as a string>
+//   value: <int64 'next_available_registration_id'>
 //
 //   key: "NEXT_RESOURCE_ID"
-//   value: <int64 serialized as a string>
+//   value: <int64 'next_available_resource_id'>
 //
 //   key: "NEXT_VERSION_ID"
-//   value: <int64 serialized as a string>
+//   value: <int64 'next_available_version_id'>
+//
+//   key: "PRES:" + <int64 'purgeable_resource_id'>
+//   value: <empty>
+//
+//   key: "REG:" + (1) + '\x00' + (2)
+//     (1) <GURL 'origin_url' serialized by GURL::spec()>
+//     (2) <int64 'registration_id'>
+//     (ex. "REG:http://example.com\x00123456")
+//   value: <ServiceWorkerRegistrationData serialized as a string>
+//
+//   key: "RES:" + <int64 'version_id'> + '\x00' + <int64 'resource_id'>
+//     (ex. "RES:123456\x00654321")
+//   value: <ServiceWorkerResourceRecord serialized as a string>
+//
+//   key: "URES:" + <int64 'uncommitted_resource_id'>
+//   value: <empty>
 
 namespace content {
 
@@ -39,20 +62,119 @@ const char kNextRegIdKey[] = "NEXT_REGISTRATION_ID";
 const char kNextResIdKey[] = "NEXT_RESOURCE_ID";
 const char kNextVerIdKey[] = "NEXT_VERSION_ID";
 
+const char kRegKeyPrefix[] = "REG:";
+const char kResKeyPrefix[] = "RES:";
+const char kUncommittedResKeyPrefix[] = "URES:";
+const char kPurgeableResKeyPrefix[] = "PRES:";
+const char kKeySeparator = '\x00';
+
 const int64 kCurrentSchemaVersion = 1;
 
-}  // namespace
+std::string RemovePrefix(const std::string& str,

[jsbell, 2014/04/23 22:55:54]

This is a little weird since you can't tell if it removed a prefix or not. All
the current callers of RemovePrefix() do a StartsWithASCII() check first - maybe
change the signature so it returns bool and the string is an out param? Or just
DCHECK(StartsWithASCII(...)) ?

[nhiroki, 2014/04/24 12:12:38]

Done.

+                         const std::string& prefix) {
+  if (StartsWithASCII(str, prefix, true))
+    return str.substr(prefix.size());
+  return str;
+}
+
+std::string CreateRegistrationKey(const ServiceWorkerRegistrationData& data) {
+  GURL origin = GURL(data.scope_url()).GetOrigin();
+  std::ostringstream out;

[jsbell, 2014/04/23 22:55:54]

#include <sstream> for ostringstream?

[michaeln, 2014/04/24 00:35:04]

should probably use #include "base/strings/stringprintf.h" for this instead,
lots of usage of that in the project (vs 2 for oss in content/)

[nhiroki, 2014/04/24 12:12:38]

Replaced them with StringPrintf.

+  out << kRegKeyPrefix << origin.spec() << kKeySeparator
+      << base::Int64ToString(data.registration_id());
+  return out.str();
+}
+
+std::string CreateResourceKey(
+    const ServiceWorkerResourceRecord& resource,
+    int64 version_id) {
+  std::ostringstream out;
+  out << kResKeyPrefix << base::Int64ToString(version_id)
+      << kKeySeparator << base::Int64ToString(resource.resource_id());
+  return out.str();
+}
+
+std::string CreateUncommittedResourceKey(int64 resource_id) {
+  std::ostringstream out;
+  out << kUncommittedResKeyPrefix << base::Int64ToString(resource_id);
+  return out.str();
+}
 
-ServiceWorkerDatabase::RegistrationData::RegistrationData()
-    : registration_id(-1),
-      version_id(-1),
-      is_active(false),
-      has_fetch_handler(false) {
+std::string CreatePurgeableResourceKey(int64 resource_id) {
+  std::ostringstream out;
+  out << kPurgeableResKeyPrefix << base::Int64ToString(resource_id);
+  return out.str();
 }
 
-ServiceWorkerDatabase::RegistrationData::~RegistrationData() {
+void PutRegistrationDataToBatch(const ServiceWorkerRegistrationData& data,
+                                leveldb::WriteBatch* batch) {
+  DCHECK(batch);
+  std::string value;
+  bool success = data.SerializeToString(&value);
+  DCHECK(success);
+  batch->Put(CreateRegistrationKey(data), value);
+}
+
+void PutResourceRecordToBatch(const ServiceWorkerResourceRecord& resource,
+                              int64 version_id,
+                              leveldb::WriteBatch* batch) {
+  DCHECK(batch);
+  std::string value;
+  bool success = resource.SerializeToString(&value);
+  DCHECK(success);
+  batch->Put(CreateResourceKey(resource, version_id), value);
+}
+
+bool ParseRegistrationKey(const std::string& key,
+                          GURL* origin_out,
+                          int64* registration_id_out) {
+  std::string unprefixed = RemovePrefix(key, kRegKeyPrefix);
+  std::vector<std::string> tokens;
+  base::SplitString(unprefixed, kKeySeparator, &tokens);
+  if (tokens.size() != 2)
+    return false;
+
+  GURL origin(tokens[0]);
+  if (!origin.is_valid())
+    return false;
+
+  int64 registration_id;
+  if (!base::StringToInt64(tokens[1], &registration_id))
+    return false;
+
+  if (origin_out)
+    *origin_out = origin;
+  if (registration_id_out)
+    *registration_id_out = registration_id;
+  return true;
 }
 
+bool ParseResourceKey(const std::string& key,
+                      int64* version_id_out,
+                      int64* resource_id_out) {
+  std::string unprefixed = RemovePrefix(key, kResKeyPrefix);
+  std::vector<std::string> tokens;
+  base::SplitString(unprefixed, kKeySeparator, &tokens);
+  if (tokens.size() != 2)
+    return false;
+
+  int64 version_id;
+  if (!base::StringToInt64(tokens[0], &version_id))
+    return false;
+
+  int64 resource_id;
+  if (!base::StringToInt64(tokens[1], &resource_id))
+    return false;
+
+  if (version_id_out)
+    *version_id_out = version_id;
+  if (resource_id_out)
+    *resource_id_out = resource_id;
+  return true;
+}
+
+}  // namespace
+
 ServiceWorkerDatabase::ServiceWorkerDatabase(const base::FilePath& path)
     : path_(path),
       is_disabled_(false),
@@ -80,9 +202,9 @@ bool ServiceWorkerDatabase::GetNextAvailableIds(
   int64 ver_id = -1;
   int64 res_id = -1;
 
-  if (!ReadInt64(kNextRegIdKey, &reg_id) ||
-      !ReadInt64(kNextVerIdKey, &ver_id) ||
-      !ReadInt64(kNextResIdKey, &res_id))
+  if (!ReadNextAvailableId(kNextRegIdKey, &reg_id) ||
+      !ReadNextAvailableId(kNextVerIdKey, &ver_id) ||
+      !ReadNextAvailableId(kNextResIdKey, &res_id))
     return false;
 
   *next_avail_registration_id = reg_id;
@@ -91,6 +213,304 @@ bool ServiceWorkerDatabase::GetNextAvailableIds(
   return true;
 }
 
+bool ServiceWorkerDatabase::GetOriginsWithRegistrations(

[michaeln, 2014/04/24 00:35:04]

This method and GetNextAvailableIds will be called very early on in browser
startup (prior to loading the initial page shown by the browser). The full table
scan in this method gives me pause and makes me wonder if we should store unique
origins separately in the schema for faster retrieval.

Is there a difference in io required between iterating over keys with zerolen
values vs iterating keys with larger values?

Also, is there benefit to having all of the key values we'll be accessing early
on prefixed similarly... ala "init-"?

Was just talking to jsbell about that and it sounds like "yes" to both, so in
light of that, i think we should tweek the schema:

* add a new 'table' for unique origins
* prefix schemaversion, nextids, and uniqueorigins with "initdata-" (or
something). 

[nhiroki, 2014/04/24 05:57:30]

I looked over some docs and sourcecode for leveldb:
According to [1], key and value are adjacent in a block, so I think the former
can be scanned more quickly than the latter.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/leveld...
I think this is true. "Key Layout" section in [2] says "Adjacent keys (according
to the database sort order) will usually be placed in the same block. Therefore
the application can improve its performance by placing keys that are accessed
together near each other and ..."

[2] http://leveldb.googlecode.com/svn/trunk/doc/index.html
As mentioned above, I agree with both of you. I'll tweak the schema as per you
described.

[nhiroki, 2014/04/24 12:12:38]

Done.

+    std::set<GURL>* origins) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  DCHECK(origins);
+
+  if (!LazyOpen(false) || is_disabled_)
+    return false;
+
+  GURL origin;
+  scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
+  for (itr->Seek(kRegKeyPrefix); itr->Valid(); itr->Next()) {
+    std::string key = itr->key().ToString();
+    if (!StartsWithASCII(key, kRegKeyPrefix, true))
+      break;
+
+    if (!ParseRegistrationKey(key, &origin, NULL /* registration_id */)) {
+      HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
+      origins->clear();
+      return false;
+    }
+    origins->insert(origin);
+  }
+  return true;

[jsbell, 2014/04/23 22:55:54]

If the iterator fails (!itr->status().ok()) should this HandleError?

(ditto for all the other iteration use)

[nhiroki, 2014/04/24 12:12:38]

Done.

+}
+
+bool ServiceWorkerDatabase::GetRegistrationsForOrigin(
+    const GURL& origin,
+    std::vector<ServiceWorkerRegistrationData>* registrations) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  DCHECK(registrations);
+
+  if (!LazyOpen(false) || is_disabled_)
+    return false;
+
+  // Create a key prefix for registrations.
+  std::ostringstream out;
+  out << kRegKeyPrefix << origin.spec() << kKeySeparator;
+  std::string prefix = out.str();
+
+  scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
+  for (itr->Seek(prefix); itr->Valid(); itr->Next()) {
+    std::string key = itr->key().ToString();
+    if (!StartsWithASCII(key, prefix, true))
+      break;
+
+    ServiceWorkerRegistrationData registration;
+    if (!registration.ParseFromString(itr->value().ToString())) {

[jsbell, 2014/04/23 22:55:54]

Is there any other post-parsing sanity checking that we want to do?

It's probably overly paranoid, but we could e.g. verify that the origins of the
script/scope URLs match.

[nhiroki, 2014/04/24 12:12:38]

For now, we seems to be able to check only script/scope URLs match. Added the
check.

+      HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
+      registrations->clear();
+      return false;
+    }
+
+    registrations->push_back(registration);
+  }
+  return true;
+}
+
+bool ServiceWorkerDatabase::ReadRegistration(
+    int64 registration_id,
+    ServiceWorkerRegistrationData* registration,
+    std::vector<ServiceWorkerResourceRecord>* resources) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  DCHECK(registration);
+  DCHECK(resources);
+
+  if (!LazyOpen(false) || is_disabled_)
+    return false;
+
+  ServiceWorkerRegistrationData value;
+  if (!ReadRegistrationData(registration_id, &value))
+    return false;
+
+  if (!ReadResourceRecords(value.version_id(), resources))
+    return false;
+
+  *registration = value;
+  return true;
+}
+
+bool ServiceWorkerDatabase::WriteRegistration(
+    const ServiceWorkerRegistrationData& registration,
+    const std::vector<ServiceWorkerResourceRecord>& resources) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  if (!LazyOpen(true) || is_disabled_)
+    return false;
+
+  leveldb::WriteBatch batch;
+  if (!BumpNextAvailableIdIfNeeded(
+          kNextRegIdKey, registration.registration_id(), &batch) ||
+      !BumpNextAvailableIdIfNeeded(
+          kNextVerIdKey, registration.version_id(), &batch)) {
+    return false;
+  }
+
+  PutRegistrationDataToBatch(registration, &batch);
+
+  std::vector<ServiceWorkerResourceRecord>::const_iterator itr;
+  for (itr = resources.begin(); itr != resources.end(); ++itr)
+    PutResourceRecordToBatch(*itr, registration.version_id(), &batch);

[michaeln, 2014/04/24 00:35:04]

If it helps, we could defer dealing with ResourceRecords for a while until we're
happy with the reading/writing the RegistrationData.

[nhiroki, 2014/04/24 12:12:38]

Okay, dropped them.

+
+  return WriteBatch(&batch);
+}
+
+bool ServiceWorkerDatabase::UpdateVersionToActive(int64 registration_id) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  if (!LazyOpen(false) || is_disabled_)
+    return false;
+
+  ServiceWorkerRegistrationData data;
+  if (!ReadRegistrationData(registration_id, &data))
+    return false;
+
+  data.set_is_active(true);
+
+  leveldb::WriteBatch batch;
+  PutRegistrationDataToBatch(data, &batch);
+  return WriteBatch(&batch);
+}
+
+bool ServiceWorkerDatabase::UpdateLastCheckTime(
+    int64 registration_id,
+    const base::Time& time) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  if (!LazyOpen(false) || is_disabled_)
+    return false;
+
+  ServiceWorkerRegistrationData data;
+  if (!ReadRegistrationData(registration_id, &data))
+    return false;
+
+  data.set_last_update_check_time(time.ToInternalValue());
+
+  leveldb::WriteBatch batch;
+  PutRegistrationDataToBatch(data, &batch);
+  return WriteBatch(&batch);
+}
+
+bool ServiceWorkerDatabase::DeleteRegistration(
+    int64 registration_id) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  if (!LazyOpen(false) || is_disabled_)
+    return false;
+
+  leveldb::WriteBatch batch;
+  ServiceWorkerRegistrationData data;
+  if (!ReadRegistrationData(registration_id, &data)) {
+    if (is_disabled_)
+      return false;
+    // Just not found.
+    return true;
+  }
+  batch.Delete(CreateRegistrationKey(data));
+
+  // Create a key prefix for resource records.
+  std::ostringstream out;
+  out << kResKeyPrefix << data.version_id() << kKeySeparator;
+  std::string prefix = out.str();
+
+  scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
+  for (itr->Seek(prefix); itr->Valid(); itr->Next()) {
+    std::string key = itr->key().ToString();
+    if (!StartsWithASCII(key, prefix, true))
+      break;
+    batch.Delete(key);
+  }
+
+  return WriteBatch(&batch);
+}
+
+bool ServiceWorkerDatabase::GetUncommittedResourceIds(std::set<int64>* ids) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  DCHECK(ids);
+
+  if (!LazyOpen(false) || is_disabled_)
+    return false;
+
+  scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
+  for (itr->Seek(kUncommittedResKeyPrefix); itr->Valid(); itr->Next()) {
+    std::string key = itr->key().ToString();
+    if (!StartsWithASCII(key, kUncommittedResKeyPrefix, true))
+      break;
+
+    std::string unprefixed = RemovePrefix(key, kUncommittedResKeyPrefix);
+    int64 resource_id;
+    if (!base::StringToInt64(unprefixed, &resource_id)) {
+      HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
+      ids->clear();
+      return false;
+    }
+    ids->insert(resource_id);
+  }
+  return true;
+}
+
+bool ServiceWorkerDatabase::WriteUncommittedResourceIds(
+    const std::set<int64>& ids) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  if (!LazyOpen(true) || is_disabled_)
+    return false;
+  if (ids.empty())
+    return true;
+
+  leveldb::WriteBatch batch;
+  for (std::set<int64>::const_iterator itr = ids.begin();
+       itr != ids.end(); ++itr) {
+    // Value should be empty.
+    batch.Put(CreateUncommittedResourceKey(*itr), "");
+  }
+  return WriteBatch(&batch);
+}
+
+bool ServiceWorkerDatabase::ClearUncommittedResourceIds(
+    const std::set<int64>& ids) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  if (!LazyOpen(true) || is_disabled_)
+    return false;
+  if (ids.empty())
+    return true;
+
+  leveldb::WriteBatch batch;
+  for (std::set<int64>::const_iterator itr = ids.begin();
+       itr != ids.end(); ++itr) {
+    batch.Delete(CreateUncommittedResourceKey(*itr));
+  }
+  return WriteBatch(&batch);
+}
+
+bool ServiceWorkerDatabase::GetPurgeableResourceIds(std::set<int64>* ids) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  DCHECK(ids);
+
+  if (!LazyOpen(false) || is_disabled_)
+    return false;
+
+  scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
+  for (itr->Seek(kPurgeableResKeyPrefix); itr->Valid(); itr->Next()) {
+    std::string key = itr->key().ToString();
+    if (!StartsWithASCII(key, kPurgeableResKeyPrefix, true))
+      break;
+
+    std::string unprefixed = RemovePrefix(key, kPurgeableResKeyPrefix);
+    int64 resource_id;
+    if (!base::StringToInt64(unprefixed, &resource_id)) {
+      HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
+      ids->clear();
+      return false;
+    }
+    ids->insert(resource_id);
+  }
+  return true;
+}
+
+bool ServiceWorkerDatabase::WritePurgeableResourceIds(
+    const std::set<int64>& ids) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  if (!LazyOpen(true) || is_disabled_)
+    return false;
+  if (ids.empty())
+    return true;
+
+  leveldb::WriteBatch batch;
+  for (std::set<int64>::const_iterator itr = ids.begin();
+       itr != ids.end(); ++itr) {
+    // Value should be empty.
+    batch.Put(CreatePurgeableResourceKey(*itr), "");
+  }
+  return WriteBatch(&batch);
+}
+
+bool ServiceWorkerDatabase::ClearPurgeableResourceIds(
+    const std::set<int64>& ids) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  if (!LazyOpen(true) || is_disabled_)
+    return false;
+  if (ids.empty())
+    return true;
+
+  leveldb::WriteBatch batch;
+  for (std::set<int64>::const_iterator itr = ids.begin();
+       itr != ids.end(); ++itr) {
+    batch.Delete(CreatePurgeableResourceKey(*itr));
+  }
+  return WriteBatch(&batch);
+}
+
+bool ServiceWorkerDatabase::DeleteAllDataForOrigin(
+    const GURL& origin) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  NOTIMPLEMENTED();
+  return false;
+}
+
+bool ServiceWorkerDatabase::DeleteAllData() {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+  NOTIMPLEMENTED();
+  return false;
+}
+
 bool ServiceWorkerDatabase::LazyOpen(bool create_if_needed) {
   DCHECK(sequence_checker_.CalledOnValidSequencedThread());
   if (IsOpen())
@@ -125,15 +545,13 @@ bool ServiceWorkerDatabase::LazyOpen(bool create_if_needed) {
   if (!status.ok()) {
     DCHECK(!db);
     // TODO(nhiroki): Should we retry to open the database?
-    DLOG(ERROR) << "Failed to open LevelDB database: " << status.ToString();
-    is_disabled_ = true;
+    HandleError(FROM_HERE, status);
     return false;
   }
   db_.reset(db);
 
   if (IsEmpty() && !PopulateInitialData()) {
     DLOG(ERROR) << "Failed to populate the database.";
-    is_disabled_ = true;
     db_.reset();
     return false;
   }
@@ -141,58 +559,122 @@ bool ServiceWorkerDatabase::LazyOpen(bool create_if_needed) {
 }
 
 bool ServiceWorkerDatabase::PopulateInitialData() {
-  scoped_ptr<leveldb::WriteBatch> batch(new leveldb::WriteBatch);
-  batch->Put(kDatabaseVersionKey, base::Int64ToString(kCurrentSchemaVersion));
-  batch->Put(kNextRegIdKey, "0");
-  batch->Put(kNextResIdKey, "0");
-  batch->Put(kNextVerIdKey, "0");
-  return WriteBatch(batch.Pass());
+  leveldb::WriteBatch batch;
+  batch.Put(kDatabaseVersionKey, base::Int64ToString(kCurrentSchemaVersion));
+  batch.Put(kNextRegIdKey, "0");
+  batch.Put(kNextResIdKey, "0");
+  batch.Put(kNextVerIdKey, "0");
+  return WriteBatch(&batch);
 }
 
-bool ServiceWorkerDatabase::ReadInt64(
-    const leveldb::Slice& key,
-    int64* value_out) {
-  DCHECK(value_out);
+bool ServiceWorkerDatabase::ReadNextAvailableId(
+    const char* id_key, int64* next_avail_id) {
+  DCHECK(id_key);
+  DCHECK(next_avail_id);
 
   std::string value;
-  leveldb::Status status = db_->Get(leveldb::ReadOptions(), key, &value);
+  leveldb::Status status = db_->Get(leveldb::ReadOptions(), id_key, &value);
   if (!status.ok()) {
-    DLOG(ERROR) << "Failed to read data keyed by "
-                << key.ToString() << ": " << status.ToString();
-    is_disabled_ = true;
-    if (status.IsCorruption())
-      was_corruption_detected_ = true;
+    HandleError(FROM_HERE, status);
     return false;
   }
 
-  int64 parsed = -1;
-  if (!base::StringToInt64(value, &parsed)) {
-    DLOG(ERROR) << "Database might be corrupted: "
-                << key.ToString() << ", " << value;
-    is_disabled_ = true;
-    was_corruption_detected_ = true;
+  int64 parsed_id;
+  if (!base::StringToInt64(value, &parsed_id)) {
+    HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
     return false;
   }
 
-  *value_out = parsed;
+  *next_avail_id = parsed_id;
   return true;
 }
 
-bool ServiceWorkerDatabase::WriteBatch(scoped_ptr<leveldb::WriteBatch> batch) {
-  if (!batch)
-    return true;
+bool ServiceWorkerDatabase::ReadRegistrationData(
+    int64 registration_id,
+    ServiceWorkerRegistrationData* data_out) {
+  int64 parsed_id;
+  scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
+  for (itr->Seek(kRegKeyPrefix); itr->Valid(); itr->Next()) {

[michaeln, 2014/04/24 00:35:04]

Oh no... linear scans. Let's massage things so this can seek directly to the
right entry. Looks like we have two options:

a. tweek the inputs to this function to include the 'origin' as well
b. tweek the schema such that there's also an index by id

I'm looking at the ServiceWorkerStorage class and requiring it to know origin +
id in order to do anything is fine, so my vote is for a.

[nhiroki, 2014/04/24 05:57:30]

That sounds reasonable and I thought I'd like to change like (a). I'll do so.

+    std::string key = itr->key().ToString();
+    if (!StartsWithASCII(key, kRegKeyPrefix, true))
+      return false;
+
+    if (!ParseRegistrationKey(key, NULL /* origin */, &parsed_id)) {
+      HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
+      return false;
+    }
+    if (registration_id != parsed_id)
+      continue;
+
+    ServiceWorkerRegistrationData data;
+    if (!data.ParseFromString(itr->value().ToString())) {
+      HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
+      return false;
+    }
 
-  leveldb::Status status = db_->Write(leveldb::WriteOptions(), batch.get());
-  if (status.ok())
+    *data_out = data;
     return true;
+  }
 
-  DLOG(ERROR) << "Failed to write the batch: " << status.ToString();
-  is_disabled_ = true;
-  if (status.IsCorruption())
-    was_corruption_detected_ = true;
   return false;
 }
 
+bool ServiceWorkerDatabase::ReadResourceRecords(
+    int64 version_id,
+    std::vector<ServiceWorkerResourceRecord>* resources) {
+  DCHECK(resources);
+
+  // Create a key prefix for resource records.
+  std::ostringstream out;
+  out << kResKeyPrefix << version_id << kKeySeparator;
+  std::string prefix = out.str();
+
+  int64 resource_id;
+  scoped_ptr<leveldb::Iterator> itr(db_->NewIterator(leveldb::ReadOptions()));
+  for (itr->Seek(prefix); itr->Valid(); itr->Next()) {
+    std::string key = itr->key().ToString();
+    if (!StartsWithASCII(key, prefix, true))
+      break;
+
+    if (!ParseResourceKey(key, NULL /* version_id */, &resource_id)) {
+      HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
+      resources->clear();
+      return false;
+    }
+
+    ServiceWorkerResourceRecord resource;
+    if (!resource.ParseFromString(itr->value().ToString())) {
+      HandleError(FROM_HERE, leveldb::Status::Corruption("failed to parse"));
+      resources->clear();
+      return false;
+    }
+
+    resources->push_back(resource);
+  }
+  return true;
+}
+
+bool ServiceWorkerDatabase::WriteBatch(leveldb::WriteBatch* batch) {
+  DCHECK(batch);
+  leveldb::Status status = db_->Write(leveldb::WriteOptions(), batch);
+  if (!status.ok()) {
+    HandleError(FROM_HERE, status);
+    return false;
+  }
+  return true;
+}
+
+bool ServiceWorkerDatabase::BumpNextAvailableIdIfNeeded(
+    const char* id_key, int64 used_id, leveldb::WriteBatch* batch) {
+  DCHECK(batch);
+  int64 next_available_id;
+  if (!ReadNextAvailableId(id_key, &next_available_id))
+    return false;
+  if (next_available_id <= used_id)
+    batch->Put(id_key, base::Int64ToString(used_id + 1));
+  return true;
+}
+
 bool ServiceWorkerDatabase::IsOpen() {
   return db_.get() != NULL;
 }
@@ -203,4 +685,14 @@ bool ServiceWorkerDatabase::IsEmpty() {
   return !itr->Valid();
 }
 
+void ServiceWorkerDatabase::HandleError(

[jsbell, 2014/04/23 22:55:54]

Can you add a TODO: for adding an UMA histogram?

[nhiroki, 2014/04/24 12:12:38]

Done.

+    const tracked_objects::Location& from_here,
+    const leveldb::Status& status) {
+  DLOG(ERROR) << "Failed at: " << from_here.ToString()
+              << " with error: " << status.ToString();
+  is_disabled_ = true;
+  if (status.IsCorruption())
+    was_corruption_detected_ = true;
+}
+
 }  // namespace content