Blob support for IDB [Chromium]

content/browser/indexed_db/indexed_db_database.cc

Index: content/browser/indexed_db/indexed_db_database.cc
diff --git a/content/browser/indexed_db/indexed_db_database.cc b/content/browser/indexed_db/indexed_db_database.cc
index 8d6329326870f26042825f07609a68d5320c04d3..f58c1848c1956ae5b46da5213536505284f0efbb 100644
--- a/content/browser/indexed_db/indexed_db_database.cc
+++ b/content/browser/indexed_db/indexed_db_database.cc
@@ -6,22 +6,27 @@
 
 #include <math.h>
 #include <set>
+#include <vector>
 
 #include "base/auto_reset.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
+#include "content/browser/indexed_db/indexed_db_blob_info.h"
 #include "content/browser/indexed_db/indexed_db_connection.h"
 #include "content/browser/indexed_db/indexed_db_cursor.h"
 #include "content/browser/indexed_db/indexed_db_factory.h"
 #include "content/browser/indexed_db/indexed_db_index_writer.h"
 #include "content/browser/indexed_db/indexed_db_tracing.h"
 #include "content/browser/indexed_db/indexed_db_transaction.h"
+#include "content/browser/indexed_db/indexed_db_value.h"
 #include "content/common/indexed_db/indexed_db_key_path.h"
 #include "content/common/indexed_db/indexed_db_key_range.h"
 #include "content/public/browser/browser_thread.h"
 #include "third_party/WebKit/public/platform/WebIDBDatabaseException.h"
+#include "webkit/browser/blob/blob_data_handle.h"
 
 using base::Int64ToString16;
 using blink::WebIDBKeyTypeNumber;
@@ -34,22 +39,26 @@ class IndexedDBDatabase::PendingOpenCall {
  public:
   PendingOpenCall(scoped_refptr<IndexedDBCallbacks> callbacks,
                   scoped_refptr<IndexedDBDatabaseCallbacks> database_callbacks,
+                  int child_process_id,
                   int64 transaction_id,
                   int64 version)
       : callbacks_(callbacks),

[jsbell, 2013/12/18 23:04:40]

We could potentially introduce a IndexedDBPendingConnection class that
encapsulates { callbacks, database_callbacks, version, transaction_id,
child_process_id } rather than schlepping these all around in parallel. That
could land early (without child_process_id) to minimize the later diff.

(There's a TODO item to introduce an actual queue of pending connections - 
crbug.com/225850 - so this would be generally helpful.)

[ericu, 2013/12/19 05:19:11]

I put a TODO in my notes; I'll look into it.

[ericu, 2014/03/05 23:30:40]

I just put this change in.

         database_callbacks_(database_callbacks),
+        child_process_id_(child_process_id),
         version_(version),
         transaction_id_(transaction_id) {}
   scoped_refptr<IndexedDBCallbacks> Callbacks() { return callbacks_; }
   scoped_refptr<IndexedDBDatabaseCallbacks> DatabaseCallbacks() {
     return database_callbacks_;
   }
+  int ChildProcessId() { return child_process_id_; }

[jsbell, 2013/12/18 23:04:40]

FYI, I think I have a patch in flight somewhere that makes all of these conform
to coding conventions, i.e. simple const accessors are named child_process_id().
But you'll probably notice during a rebase.

[ericu, 2013/12/19 05:19:11]

Yep--that'll be fun.

   int64 Version() { return version_; }
   int64 TransactionId() const { return transaction_id_; }
 
  private:
   scoped_refptr<IndexedDBCallbacks> callbacks_;
   scoped_refptr<IndexedDBDatabaseCallbacks> database_callbacks_;
+  int child_process_id_;
   int64 version_;
   const int64 transaction_id_;
 };
@@ -574,7 +583,7 @@ void IndexedDBDatabase::GetOperation(
   bool ok;
   if (index_id == IndexedDBIndexMetadata::kInvalidId) {
     // Object Store Retrieval Operation
-    std::string value;
+    IndexedDBValue value;
     ok = backing_store_->GetRecord(transaction->BackingStoreTransaction(),
                                    id(),
                                    object_store_id,
@@ -627,7 +636,7 @@ void IndexedDBDatabase::GetOperation(
   }
 
   // Index Referenced Value Retrieval Operation
-  std::string value;
+  IndexedDBValue value;
   ok = backing_store_->GetRecord(transaction->BackingStoreTransaction(),
                                  id(),
                                  object_store_id,
@@ -694,7 +703,8 @@ static bool UpdateKeyGenerator(
 struct IndexedDBDatabase::PutOperationParams {
   PutOperationParams() {}
   int64 object_store_id;
-  std::string value;
+  IndexedDBValue value;
+  ScopedVector<webkit_blob::BlobDataHandle> handles;

[jsbell, 2013/12/18 23:04:40]

Would it make sense to add the vector to IndexedDBValue, to avoid passing things
in parallel?

[ericu, 2013/12/19 05:19:11]

It's not a clear win to me.  We'd use it here, but when doing gets we'd have an
IndexedDBValue coming back that had no BlobDataHandles, even if it had blob
info.  Would it complicate managing the lifetime of the BlobDataHandles?  I'm
not sure.  Is it worth it?  I suspect not.

   scoped_ptr<IndexedDBKey> key;
   IndexedDBDatabase::PutMode put_mode;
   scoped_refptr<IndexedDBCallbacks> callbacks;
@@ -706,7 +716,8 @@ struct IndexedDBDatabase::PutOperationParams {
 
 void IndexedDBDatabase::Put(int64 transaction_id,
                             int64 object_store_id,
-                            std::string* value,
+                            IndexedDBValue* value,
+                            ScopedVector<webkit_blob::BlobDataHandle>* handles,
                             scoped_ptr<IndexedDBKey> key,
                             PutMode put_mode,
                             scoped_refptr<IndexedDBCallbacks> callbacks,
@@ -722,9 +733,11 @@ void IndexedDBDatabase::Put(int64 transaction_id,
     return;
 
   DCHECK(key);
+  DCHECK(value);
   scoped_ptr<PutOperationParams> params(new PutOperationParams());
   params->object_store_id = object_store_id;
   params->value.swap(*value);
+  params->handles.swap(*handles);
   params->key = key.Pass();
   params->put_mode = put_mode;
   params->callbacks = callbacks;
@@ -824,6 +837,7 @@ void IndexedDBDatabase::PutOperation(scoped_ptr<PutOperationParams> params,
                                 params->object_store_id,
                                 *key,
                                 params->value,
+                                &params->handles,
                                 &record_identifier);
   if (!backing_store_success) {
     params->callbacks->OnError(IndexedDBDatabaseError(
@@ -1055,7 +1069,7 @@ void IndexedDBDatabase::OpenCursorOperation(
   }
 
   if (!backing_store_cursor) {
-    params->callbacks->OnSuccess(static_cast<std::string*>(NULL));
+    params->callbacks->OnSuccess(static_cast<IndexedDBValue*>(NULL));
     return;
   }
 
@@ -1154,28 +1168,13 @@ void IndexedDBDatabase::DeleteRangeOperation(
     scoped_refptr<IndexedDBCallbacks> callbacks,
     IndexedDBTransaction* transaction) {
   IDB_TRACE("IndexedDBDatabase::DeleteRangeOperation");
-  scoped_ptr<IndexedDBBackingStore::Cursor> backing_store_cursor =
-      backing_store_->OpenObjectStoreCursor(
-          transaction->BackingStoreTransaction(),
-          id(),
-          object_store_id,
-          *key_range,
-          indexed_db::CURSOR_NEXT);
-  if (backing_store_cursor) {
-    do {
-      if (!backing_store_->DeleteRecord(
-              transaction->BackingStoreTransaction(),
-              id(),
-              object_store_id,
-              backing_store_cursor->record_identifier())) {
-        callbacks->OnError(
-            IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
-                                   "Internal error deleting data in range"));
-        return;
-      }
-    } while (backing_store_cursor->Continue());
+  if (!backing_store_->DeleteRange(transaction->BackingStoreTransaction(),

[jsbell, 2013/12/18 23:04:40]

This could be landed early, along with IndexedDBBackingStore::DeleteRange

[ericu, 2013/12/19 05:19:11]

...or just dropped for now, as it doesn't gain any efficiency yet.

+      id(), object_store_id, *key_range)) {
+    callbacks->OnError(
+        IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
+                               "Internal error deleting data in range"));
+    return;
   }
-
   callbacks->OnSuccess();
 }
 
@@ -1378,6 +1377,7 @@ void IndexedDBDatabase::ProcessPendingCalls() {
       pending_open_calls.pop_front();
       OpenConnection(pending_open_call->Callbacks(),
                      pending_open_call->DatabaseCallbacks(),
+                     pending_open_call->ChildProcessId(),
                      pending_open_call->TransactionId(),
                      pending_open_call->Version());
     }
@@ -1413,17 +1413,20 @@ bool IndexedDBDatabase::IsOpenConnectionBlocked() const {
 void IndexedDBDatabase::OpenConnection(
     scoped_refptr<IndexedDBCallbacks> callbacks,
     scoped_refptr<IndexedDBDatabaseCallbacks> database_callbacks,
+    int child_process_id,
     int64 transaction_id,
     int64 version) {
   const blink::WebIDBDataLoss kDataLoss =
       blink::WebIDBDataLossNone;
   OpenConnection(
-      callbacks, database_callbacks, transaction_id, version, kDataLoss, "");
+      callbacks, database_callbacks, child_process_id, transaction_id, version,
+      kDataLoss, "");
 }
 
 void IndexedDBDatabase::OpenConnection(
     scoped_refptr<IndexedDBCallbacks> callbacks,
     scoped_refptr<IndexedDBDatabaseCallbacks> database_callbacks,
+    int child_process_id,
     int64 transaction_id,
     int64 version,
     blink::WebIDBDataLoss data_loss,
@@ -1438,7 +1441,8 @@ void IndexedDBDatabase::OpenConnection(
     // so there'd better not be any.
     DCHECK_NE(blink::WebIDBDataLossTotal, data_loss);
     pending_open_calls_.push_back(new PendingOpenCall(
-        callbacks, database_callbacks, transaction_id, version));
+        callbacks, database_callbacks, child_process_id, transaction_id,
+        version));
     return;
   }
 
@@ -1478,6 +1482,7 @@ void IndexedDBDatabase::OpenConnection(
     // TODO(jsbell): DCHECK that not in unit tests.
     DCHECK(is_new_database);
     connections_.insert(connection.get());
+    backing_store_->GrantChildProcessPermissions(child_process_id);

[jsbell, 2013/12/18 23:04:40]

DRY; can we wrap the connection insertion and granting permissions (and even
connection object creation?) into a helper?

[ericu, 2013/12/19 05:19:11]

Done.

     callbacks->OnSuccess(connection.Pass(), this->metadata());
     return;
   }
@@ -1485,6 +1490,7 @@ void IndexedDBDatabase::OpenConnection(
   if (version == IndexedDBDatabaseMetadata::NO_INT_VERSION) {
     if (!is_new_database) {
       connections_.insert(connection.get());
+      backing_store_->GrantChildProcessPermissions(child_process_id);
       callbacks->OnSuccess(connection.Pass(), this->metadata());
       return;
     }
@@ -1495,6 +1501,7 @@ void IndexedDBDatabase::OpenConnection(
 
   if (version > metadata_.int_version) {
     connections_.insert(connection.get());
+    backing_store_->GrantChildProcessPermissions(child_process_id);
     RunVersionChangeTransaction(callbacks,
                                 connection.Pass(),
                                 transaction_id,
@@ -1513,6 +1520,7 @@ void IndexedDBDatabase::OpenConnection(
   }
   DCHECK_EQ(version, metadata_.int_version);
   connections_.insert(connection.get());
+  backing_store_->GrantChildProcessPermissions(child_process_id);
   callbacks->OnSuccess(connection.Pass(), this->metadata());
 }