IndexedDB: Change Mojo IDBValue.bits type from string to array<uint8>

TL;DR IDBValue.bits was type string which worked until we added a
Mojo Blink variant that introduced a new conversion requirement.
IDBValue.bits is actually a sequence of octets, so use array<uint8>,
instead.

More details:

Chrome's IndexedDB implementation uses a Mojo definition that
specifies the IDBValue struct contains a field |bits|.  The field is
used to represent the data that's written to and read off of disk by
IndexedDB's LevelDB instance.

Previously, the |bits| field was of type string and there were only
content variants of the Mojo C++ bindings for IndexedDB.  This led to
the field only ever being translated to/from std::string.
std::string can contain these raw byte sequences since there are no
constraints placed on the string holding data following any
particular format or validation.

As part of bug 717812 and https://crrev.com/1265900, a new Blink
variant for the IndexedDB Mojo interfaces was added.  The Blink
variant relies on WTF::String instead of std::string, and so the
IDBValue.bits field was translated for that variant to/from
WTF::String.

Mojo's WTF::String traits uses WTF::String::FromUTF8().  As part of
FromUTF8()'s implementation, if the given set of characters contains
invalid UTF-8 data, FromUTF8() will return a null String instance.
The IDBValue.bits field is not restricted just to UTF-8 characters,
and so it always ended up being set to the null String in the traits.

The net result of this bug was that our carriage format changed:

- Chrome with https://crrev.com/1265900 could not read Chrome
  stable's IDB instances

- Chrome without https://crrev.com/1265900 could not read Chrome with
  http://crrev.com/1265900's IDB instances

More about the impact of that bug can be read at
https://crbug.com/899446.

We must observe FromUTF8()'s requirement that we pass only UTF-8
valid data in string types.  We also need to observe that we are in
fact passing raw byte data in the IDBValue.bits field instead of what
both variants would consider as valid strings.

The fix is to change IDBValue.bits from a string type to a type of
array<uint8>.  LevelDB's API requires strings, so in content to/from
the LevelDB instance we must pass in and receive strings.  In Blink,
the data is represented as a WebData instance, which also takes raw
character data.  To support both sides, I've added in-place
conversions to/from std::string and to/from WebData.

Bug: 717812
Change-Id: Iadfbd21be52fc50abc60a2695b95691bc07028e4
Reviewed-on: https://chromium-review.googlesource.com/c/1316444
Commit-Queue: Chase Phillips <cmp@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605918}

content/browser/indexed_db/database_impl.cc

@@ -683,7 +683,10 @@ void DatabaseImpl::IDBSequenceHelper::Put(
 
   uint64_t commit_size = mojo_value->bits.size() + key.size_estimate();
   IndexedDBValue value;
-  swap(value.bits, mojo_value->bits);
+  // TODO(crbug.com/902498): Use mojom traits to map directly to std::string.
+  value.bits = std::string(mojo_value->bits.begin(), mojo_value->bits.end());
+  // Release mojo_value->bits std::vector.
+  mojo_value->bits.clear();
   swap(value.blob_info, blob_info);
   connection_->database()->Put(transaction, object_store_id, &value,
                                std::make_unique<IndexedDBKey>(key), mode,

content/browser/indexed_db/indexed_db_callbacks.cc

@@ -123,8 +123,15 @@ blink::mojom::IDBReturnValuePtr ConvertReturnValue(
     mojo_value->primary_key = value->primary_key;
     mojo_value->key_path = value->key_path;
   }
-  if (!value->empty())
-    swap(mojo_value->value->bits, value->bits);
+  if (!value->empty()) {
+    // TODO(crbug.com/902498): Use mojom traits to map directly from
+    //                         std::string.
+    const char* value_data = value->bits.data();
+    mojo_value->value->bits =
+        std::vector<uint8_t>(value_data, value_data + value->bits.length());
+    // Release value->bits std::string.
+    value->bits.clear();
+  }
   ConvertBlobInfo(value->blob_info, &mojo_value->value->blob_or_file_info);
   return mojo_value;
 }
@@ -196,8 +203,15 @@ class IndexedDBCallbacks::IOThreadHelper {
 blink::mojom::IDBValuePtr IndexedDBCallbacks::ConvertAndEraseValue(
     IndexedDBValue* value) {
   auto mojo_value = blink::mojom::IDBValue::New();
-  if (!value->empty())
-    swap(mojo_value->bits, value->bits);
+  if (!value->empty()) {
+    // TODO(crbug.com/902498): Use mojom traits to map directly from
+    //                         std::string.
+    const char* value_data = value->bits.data();
+    mojo_value->bits =
+        std::vector<uint8_t>(value_data, value_data + value->bits.length());
+    // Release value->bits std::string.
+    value->bits.clear();
+  }
   ConvertBlobInfo(value->blob_info, &mojo_value->blob_or_file_info);
   return mojo_value;
 }

content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc

@@ -433,12 +433,20 @@ TEST_F(IndexedDBDispatcherHostTest, PutWithInvalidBlob) {
     mojo::MakeRequest(&blob);
     blobs.push_back(blink::mojom::IDBBlobInfo::New(
         std::move(blob), "fakeUUID", base::string16(), 100, nullptr));
-    connection.database->Put(kTransactionId, kObjectStoreId,
-                             IDBValue::New("hello", std::move(blobs)),
-                             IndexedDBKey(base::UTF8ToUTF16("hello")),
-                             blink::kWebIDBPutModeAddOnly,
-                             std::vector<IndexedDBIndexKeys>(),
-                             put_callbacks->CreateInterfacePtrAndBind());
+
+    std::string value = "hello";
+    const char* value_data = value.data();
+    std::vector<uint8_t> value_vector(value_data, value_data + value.length());
+
+    auto new_value = blink::mojom::IDBValue::New();
+    new_value->bits = std::move(value_vector);
+    new_value->blob_or_file_info = std::move(blobs);
+
+    connection.database->Put(
+        kTransactionId, kObjectStoreId, std::move(new_value),
+        IndexedDBKey(base::UTF8ToUTF16("hello")), blink::kWebIDBPutModeAddOnly,
+        std::vector<IndexedDBIndexKeys>(),
+        put_callbacks->CreateInterfacePtrAndBind());
     connection.database->Commit(kTransactionId);
     loop.Run();
   }
@@ -1058,10 +1066,17 @@ TEST_F(IndexedDBDispatcherHostTest, NotifyIndexedDBContentChanged) {
     connection1.database->CreateObjectStore(kTransactionId1, kObjectStoreId,
                                             base::UTF8ToUTF16(kObjectStoreName),
                                             blink::IndexedDBKeyPath(), false);
+
+    std::string value = "value";
+    const char* value_data = value.data();
+    std::vector<uint8_t> value_vector(value_data, value_data + value.length());
+
+    auto new_value = blink::mojom::IDBValue::New();
+    new_value->bits = std::move(value_vector);
+    new_value->blob_or_file_info = std::vector<blink::mojom::IDBBlobInfoPtr>();
+
     connection1.database->Put(
-        kTransactionId1, kObjectStoreId,
-        blink::mojom::IDBValue::New(
-            "value", std::vector<blink::mojom::IDBBlobInfoPtr>()),
+        kTransactionId1, kObjectStoreId, std::move(new_value),
         IndexedDBKey(base::UTF8ToUTF16("key")), blink::kWebIDBPutModeAddOnly,
         std::vector<IndexedDBIndexKeys>(),
         put_callbacks->CreateInterfacePtrAndBind());

content/renderer/indexed_db/indexed_db_callbacks_impl.cc

@@ -109,8 +109,12 @@ WebIDBValue IndexedDBCallbacksImpl::ConvertValue(
     }
   }
 
-  return WebIDBValue(WebData(&*value->bits.begin(), value->bits.size()),
-                     std::move(local_blob_info));
+  // TODO(crbug.com/902498): Use mojom traits to map directly to WebData.
+  WebData web_data(reinterpret_cast<const char*>(value->bits.data()),
+                   value->bits.size());
+  // Release value->bits std::vector.
+  value->bits.clear();
+  return WebIDBValue(std::move(web_data), std::move(local_blob_info));
 }
 
 IndexedDBCallbacksImpl::IndexedDBCallbacksImpl(

content/renderer/indexed_db/webidbdatabase_impl.cc

@@ -188,13 +188,17 @@ void WebIDBDatabaseImpl::Put(
       transaction_id, nullptr);
 
   auto mojo_value = blink::mojom::IDBValue::New();
-  DCHECK(mojo_value->bits.empty());
-  mojo_value->bits.reserve(value.size());
+
+  // mojo_value->bits initialization.
   value.ForEachSegment([&mojo_value](const char* segment, size_t segment_size,
                                      size_t segment_offset) {
-    mojo_value->bits.append(segment, segment_size);
+    const auto& segment_span = base::make_span(segment, segment + segment_size);
+    mojo_value->bits.insert(mojo_value->bits.end(), segment_span.begin(),
+                            segment_span.end());
     return true;
   });
+
+  // mojo_value->blob_or_file_info initialization.
   mojo_value->blob_or_file_info.reserve(web_blob_info.size());
   for (const WebBlobInfo& info : web_blob_info) {
     auto blob_info = blink::mojom::IDBBlobInfo::New();

third_party/blink/public/mojom/indexeddb/indexeddb.mojom

@@ -156,7 +156,7 @@ struct IDBBlobInfo {
 };
 
 struct IDBValue {
-  string bits;
+  array<uint8> bits;
   array<IDBBlobInfo> blob_or_file_info;
 };
 

third_party/blink/renderer/bindings/modules/v8/v8_binding_for_modules.cc

@@ -478,7 +478,7 @@ static v8::Local<v8::Value> DeserializeIDBValueArray(
 // the conceptual description in the spec states that the key produced by the
 // key generator is injected into the value before it is written to IndexedDB.
 //
-// We cannot implementing the spec's conceptual description. We need to assign
+// We cannot implement the spec's conceptual description. We need to assign
 // primary keys in the browser process, to ensure that multiple renderer
 // processes talking to the same database receive sequential keys. At the same
 // time, we want the value serialization code to live in the renderer process,

third_party/blink/renderer/modules/indexeddb/idb_value_wrapping.h

@@ -40,7 +40,7 @@ const base::Feature kIndexedDBLargeValueWrapping{
 //    This may be necessary when extracting the primary key and/or index keys
 //    for the serialized value.
 // 2) Wrapping - DoneCloning() transitions the instance to an internal
-//    reprensetation optimized for wrapping via WrapIfBiggerThan().
+//    representation optimized for wrapping via WrapIfBiggerThan().
 // 3) Reading results - After any desired wrapping is performed, the Take*()
 //    methods yield the serialized value components passed to the backing store.
 //    To avoid unnecessary copies, the Take*() methods move out parts of the