Pass blob info through from the IPC to the backing store on put.

content/browser/indexed_db/indexed_db_backing_store.cc

Index: content/browser/indexed_db/indexed_db_backing_store.cc
diff --git a/content/browser/indexed_db/indexed_db_backing_store.cc b/content/browser/indexed_db/indexed_db_backing_store.cc
index 32c53bc92fe810d1cbddf112a9f75fda9eb268ce..106a367d9e5f7ba26c7604c0683f0c06251d864f 100644
--- a/content/browser/indexed_db/indexed_db_backing_store.cc
+++ b/content/browser/indexed_db/indexed_db_backing_store.cc
@@ -12,6 +12,8 @@
 #include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "content/browser/child_process_security_policy_impl.h"
+#include "content/browser/indexed_db/indexed_db_blob_info.h"
 #include "content/browser/indexed_db/indexed_db_database_error.h"
 #include "content/browser/indexed_db/indexed_db_leveldb_coding.h"
 #include "content/browser/indexed_db/indexed_db_metadata.h"
@@ -27,6 +29,7 @@
 #include "third_party/WebKit/public/platform/WebIDBTypes.h"
 #include "third_party/WebKit/public/web/WebSerializedScriptValueVersion.h"
 #include "third_party/leveldatabase/env_chromium.h"
+#include "webkit/browser/blob/blob_data_handle.h"
 #include "webkit/common/database/database_identifier.h"
 
 using base::StringPiece;
@@ -45,6 +48,12 @@ static base::FilePath ComputeFileName(const GURL& origin_url) {
       .AddExtension(FILE_PATH_LITERAL(".indexeddb.leveldb"));
 }
 
+static base::FilePath ComputeBlobPath(const GURL& origin_url) {
+  return base::FilePath()
+      .AppendASCII(webkit_database::GetIdentifierFromOrigin(origin_url))
+      .AddExtension(FILE_PATH_LITERAL(".indexeddb.blob"));

[cmumford, 2014/04/10 16:59:43]

Nit: Consider removing leading '.' as FilePath::kExtensionSeparator is possibly
(albeit extremely unlikely) different for other platforms.

[jsbell, 2014/04/11 16:19:57]

Can you give an example of where this would be problematic? We follow this
pattern elsewhere.

[ericu, 2014/04/14 23:23:38]

It need not be an extension separator as long as it's a legal filename
character.  It's legal on any operating system/filesystem combo we're likely to
run on.  I we hit a place where it's not, lots of other stuff in chrome
[including line 48] will blow up too.

+}
+
 static base::FilePath ComputeCorruptionFileName(const GURL& origin_url) {
   return ComputeFileName(origin_url)
       .Append(FILE_PATH_LITERAL("corruption_info.json"));
@@ -424,11 +433,13 @@ class DefaultLevelDBFactory : public LevelDBFactory {
 IndexedDBBackingStore::IndexedDBBackingStore(
     IndexedDBFactory* indexed_db_factory,
     const GURL& origin_url,
+    const base::FilePath& blob_path,
     scoped_ptr<LevelDBDatabase> db,
     scoped_ptr<LevelDBComparator> comparator,
     base::TaskRunner* task_runner)
     : indexed_db_factory_(indexed_db_factory),
       origin_url_(origin_url),
+      blob_path_(blob_path),
       origin_identifier_(ComputeOriginIdentifier(origin_url)),
       task_runner_(task_runner),
       db_(db.Pass()),
@@ -436,6 +447,15 @@ IndexedDBBackingStore::IndexedDBBackingStore(
       active_blob_registry_(this) {}
 
 IndexedDBBackingStore::~IndexedDBBackingStore() {
+  if (!blob_path_.empty() && !child_process_ids_granted_.empty()) {
+    ChildProcessSecurityPolicyImpl* policy =
+        ChildProcessSecurityPolicyImpl::GetInstance();
+    for (std::set<int>::iterator iter = child_process_ids_granted_.begin();

[cmumford, 2014/04/10 16:59:43]

Nit: const_iterator?

[ericu, 2014/04/14 23:23:38]

Done.

+         iter != child_process_ids_granted_.end();
+         ++iter) {
+      policy->RevokeAllPermissionsForFile(*iter, blob_path_);
+    }
+  }
   // db_'s destructor uses comparator_. The order of destruction is important.
   db_.reset();
   comparator_.reset();
@@ -654,6 +674,8 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
 
   const base::FilePath file_path =
       path_base.Append(ComputeFileName(origin_url));
+  const base::FilePath blob_path =
+      path_base.Append(ComputeBlobPath(origin_url));
 
   if (IsPathTooLong(file_path)) {
     HistogramOpenStatus(INDEXED_DB_BACKING_STORE_OPEN_ORIGIN_TOO_LONG,
@@ -749,6 +771,7 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Open(
 
   return Create(indexed_db_factory,
                 origin_url,
+                blob_path,
                 db.Pass(),
                 comparator.Pass(),
                 task_runner);
@@ -783,6 +806,7 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::OpenInMemory(
 
   return Create(NULL /* indexed_db_factory */,
                 origin_url,
+                base::FilePath(),
                 db.Pass(),
                 comparator.Pass(),
                 task_runner);
@@ -792,6 +816,7 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::OpenInMemory(
 scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Create(
     IndexedDBFactory* indexed_db_factory,
     const GURL& origin_url,
+    const base::FilePath& blob_path,
     scoped_ptr<LevelDBDatabase> db,
     scoped_ptr<LevelDBComparator> comparator,
     base::TaskRunner* task_runner) {
@@ -800,6 +825,7 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Create(
   scoped_refptr<IndexedDBBackingStore> backing_store(
       new IndexedDBBackingStore(indexed_db_factory,
                                 origin_url,
+                                blob_path,
                                 db.Pass(),
                                 comparator.Pass(),
                                 task_runner));
@@ -810,6 +836,14 @@ scoped_refptr<IndexedDBBackingStore> IndexedDBBackingStore::Create(
   return backing_store;
 }
 
+void IndexedDBBackingStore::GrantChildProcessPermissions(int child_process_id) {
+  if (!child_process_ids_granted_.count(child_process_id)) {
+    child_process_ids_granted_.insert(child_process_id);
+    ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadFile(
+        child_process_id, blob_path_);
+  }
+}
+
 std::vector<base::string16> IndexedDBBackingStore::GetDatabaseNames() {
   std::vector<base::string16> found_names;
   const std::string start_key =
@@ -1093,7 +1127,7 @@ leveldb::Status IndexedDBBackingStore::GetObjectStores(
         INTERNAL_CONSISTENCY_ERROR_UNTESTED(GET_OBJECT_STORES);
     }
 
-    it->Next();  // Is evicatble.
+    it->Next();  // Is evictable.
     if (!CheckObjectStoreAndMetaDataType(it.get(),
                                          stop_key,
                                          object_store_id,
@@ -1382,7 +1416,8 @@ leveldb::Status IndexedDBBackingStore::PutRecord(
     int64 database_id,
     int64 object_store_id,
     const IndexedDBKey& key,
-    const IndexedDBValue& value,
+    IndexedDBValue& value,
+    ScopedVector<webkit_blob::BlobDataHandle>* handles,
     RecordIdentifier* record_identifier) {
   IDB_TRACE("IndexedDBBackingStore::PutRecord");
   if (!KeyPrefix::ValidIds(database_id, object_store_id))
@@ -1404,6 +1439,12 @@ leveldb::Status IndexedDBBackingStore::PutRecord(
   v.append(value.bits);
 
   leveldb_transaction->Put(object_store_data_key, &v);
+  transaction->PutBlobInfo(database_id,
+                           object_store_id,
+                           object_store_data_key,
+                           &value.blob_info,
+                           handles);
+  DCHECK(!handles->size());
 
   const std::string exists_entry_key =
       ExistsEntryKey::Encode(database_id, object_store_id, key);
@@ -1446,6 +1487,8 @@ leveldb::Status IndexedDBBackingStore::DeleteRecord(
   const std::string object_store_data_key = ObjectStoreDataKey::Encode(
       database_id, object_store_id, record_identifier.primary_key());
   leveldb_transaction->Remove(object_store_data_key);
+  transaction->PutBlobInfo(
+      database_id, object_store_id, object_store_data_key, NULL, NULL);
 
   const std::string exists_entry_key = ExistsEntryKey::Encode(
       database_id, object_store_id, record_identifier.primary_key());
@@ -2767,9 +2810,14 @@ IndexedDBBackingStore::OpenIndexCursor(
 
 IndexedDBBackingStore::Transaction::Transaction(
     IndexedDBBackingStore* backing_store)
-    : backing_store_(backing_store) {}
+    : backing_store_(backing_store), database_id_(-1) {}
 
-IndexedDBBackingStore::Transaction::~Transaction() {}
+IndexedDBBackingStore::Transaction::~Transaction() {
+  BlobChangeMap::iterator iter = blob_change_map_.begin();

[cmumford, 2014/04/10 16:59:43]

Can you use STLDeleteContainerPairSecondPointers()?

[ericu, 2014/04/14 23:23:38]

Done.  Just barely makes it shorter, what with that long function name, but I
like the change anyway for safety.

+  for (; iter != blob_change_map_.end(); ++iter) {
+    delete iter->second;
+  }
+}
 
 void IndexedDBBackingStore::Transaction::Begin() {
   IDB_TRACE("IndexedDBBackingStore::Transaction::Begin");
@@ -2794,4 +2842,50 @@ void IndexedDBBackingStore::Transaction::Rollback() {
   transaction_ = NULL;
 }
 
+
+void IndexedDBBackingStore::Transaction::BlobChangeRecord::SetBlobInfo(
+    std::vector<IndexedDBBlobInfo>* blob_info) {
+  blob_info_.clear();
+  if (blob_info)
+    blob_info_.swap(*blob_info);

[cmumford, 2014/04/10 16:59:43]

Why swap the blob info vs. operator=()

[ericu, 2014/04/14 23:23:38]

I believe that swapping the innards of a std::vector is constant-time, whereas a
copy would be O(N).

+}
+
+void IndexedDBBackingStore::Transaction::BlobChangeRecord::SetHandles(
+    ScopedVector<webkit_blob::BlobDataHandle>* handles) {
+  handles_.clear();
+  if (handles)
+    handles_.swap(*handles);
+}
+
+// This is storing an info, even if empty, even if the previous key had no blob
+// info that we know of.  It duplicates a bunch of information stored in the
+// leveldb transaction, but only w.r.t. the user keys altered--we don't keep the
+// changes to exists or index keys here.
+void IndexedDBBackingStore::Transaction::PutBlobInfo(
+    int64 database_id,
+    int64 object_store_id,
+    const std::string& key,
+    std::vector<IndexedDBBlobInfo>* blob_info,
+    ScopedVector<webkit_blob::BlobDataHandle>* handles) {
+  DCHECK_GT(key.size(), 0UL);
+  if (database_id_ < 0)
+    database_id_ = database_id;
+  DCHECK_EQ(database_id_, database_id);
+
+  BlobChangeMap::iterator it = blob_change_map_.find(key);
+  BlobChangeRecord* record = NULL;
+  if (it == blob_change_map_.end()) {
+    record = new BlobChangeRecord();
+    blob_change_map_[key] = record;
+    record->set_key(key);
+    record->set_object_store_id(object_store_id);
+  } else {
+    record = it->second;
+  }
+  DCHECK_EQ(record->object_store_id(), object_store_id);
+  record->SetBlobInfo(blob_info);
+  record->SetHandles(handles);
+  DCHECK(!handles || !handles->size());
+}
+
 }  // namespace content