Switch on use_new_wrapper_types mode for content/common.

content/browser/leveldb_wrapper_impl.cc

Index: content/browser/leveldb_wrapper_impl.cc
diff --git a/content/browser/leveldb_wrapper_impl.cc b/content/browser/leveldb_wrapper_impl.cc
index 07d9fc33a7111662d5eea35534a50d26d24a0b10..e281303d13e083183fb8751189d9e29f6fb60ac5 100644
--- a/content/browser/leveldb_wrapper_impl.cc
+++ b/content/browser/leveldb_wrapper_impl.cc
@@ -40,7 +40,7 @@ size_t LevelDBWrapperImpl::CommitBatch::GetDataSize() const {
 
   size_t count = 0;
   for (const auto& pair : changed_values)
-    count += (pair.first.size(), pair.second.size());
+    count += (pair.first.size(), pair.second.value().size());
   return count;
 }
 
@@ -83,27 +83,26 @@ void LevelDBWrapperImpl::EnableAggressiveCommitDelay() {
   s_aggressive_flushing_enabled_ = true;
 }
 
-void LevelDBWrapperImpl::Put(mojo::Array<uint8_t> key,
-                             mojo::Array<uint8_t> value,
-                             const mojo::String& source,
+void LevelDBWrapperImpl::Put(const std::vector<uint8_t>& key,
+                             const std::vector<uint8_t>& value,
+                             const std::string& source,
                              const PutCallback& callback) {
   if (!map_) {
-    LoadMap(
-        base::Bind(&LevelDBWrapperImpl::Put, base::Unretained(this),
-                   base::Passed(&key), base::Passed(&value), source, callback));
+    LoadMap(base::Bind(&LevelDBWrapperImpl::Put, base::Unretained(this), key,
+                       value, source, callback));
     return;
   }
 
   bool has_old_item = false;
-  mojo::Array<uint8_t> old_value;
+  std::vector<uint8_t> old_value;
   size_t old_item_size = 0;
   auto found = map_->find(key);
   if (found != map_->end()) {
-    if (found->second.Equals(value)) {
+    if (*(found->second) == value) {
       callback.Run(true);  // Key already has this value.
       return;
     }
-    old_value = std::move(found->second);
+    old_value.swap(*(found->second));

[michaeln, 2016/08/02 00:28:29]

Does this mutate the map? I think it clears out the entry for key? Looks like
the early return on line 116 will have cleared out the old value without storing
the new value.

[leonhsl(Using Gerrit), 2016/08/02 11:11:05]

Yeah, this should be a problem.. Maybe we can move old_value
definition/assignment before line 124? I did so in ps#2. WDYT? Thanks.

     old_item_size = key.size() + old_value.size();
     has_old_item = true;
   }
@@ -119,35 +118,33 @@ void LevelDBWrapperImpl::Put(mojo::Array<uint8_t> key,
 
   if (database_) {
     CreateCommitBatchIfNeeded();
-    commit_batch_->changed_values[key.Clone()] = value.Clone();
+    commit_batch_->changed_values[key] = value;
   }
 
-  (*map_)[key.Clone()] = value.Clone();
+  (*map_)[key] = value;
   bytes_used_ = new_bytes_used;
   if (!has_old_item) {
     // We added a new key/value pair.
     observers_.ForAllPtrs(
         [&key, &value, &source](mojom::LevelDBObserver* observer) {
-          observer->KeyAdded(key.Clone(), value.Clone(), source);
+          observer->KeyAdded(key, value, source);
         });
   } else {
     // We changed the value for an existing key.
     observers_.ForAllPtrs(
         [&key, &value, &source, &old_value](mojom::LevelDBObserver* observer) {
-          observer->KeyChanged(
-              key.Clone(), value.Clone(), old_value.Clone(), source);
+          observer->KeyChanged(key, value, old_value, source);
         });
   }
   callback.Run(true);
 }
 
-void LevelDBWrapperImpl::Delete(mojo::Array<uint8_t> key,
-                                const mojo::String& source,
+void LevelDBWrapperImpl::Delete(const std::vector<uint8_t>& key,
+                                const std::string& source,
                                 const DeleteCallback& callback) {
   if (!map_) {
-    LoadMap(
-        base::Bind(&LevelDBWrapperImpl::Delete, base::Unretained(this),
-                   base::Passed(&key), source, callback));
+    LoadMap(base::Bind(&LevelDBWrapperImpl::Delete, base::Unretained(this), key,
+                       source, callback));
     return;
   }
 
@@ -159,21 +156,20 @@ void LevelDBWrapperImpl::Delete(mojo::Array<uint8_t> key,
 
   if (database_) {
     CreateCommitBatchIfNeeded();
-    commit_batch_->changed_values[key.Clone()] = mojo::Array<uint8_t>(nullptr);
+    commit_batch_->changed_values[key] = base::nullopt;
   }
 
-  mojo::Array<uint8_t> old_value = std::move(found->second);
+  std::vector<uint8_t> old_value(std::move(*(found->second)));
   map_->erase(found);
   bytes_used_ -= key.size() + old_value.size();
   observers_.ForAllPtrs(
       [&key, &source, &old_value](mojom::LevelDBObserver* observer) {
-        observer->KeyDeleted(
-            key.Clone(), old_value.Clone(), source);
+        observer->KeyDeleted(key, old_value, source);
       });
   callback.Run(true);
 }
 
-void LevelDBWrapperImpl::DeleteAll(const mojo::String& source,
+void LevelDBWrapperImpl::DeleteAll(const std::string& source,
                                    const DeleteAllCallback& callback) {
   if (!map_) {
     LoadMap(
@@ -196,24 +192,23 @@ void LevelDBWrapperImpl::DeleteAll(const mojo::String& source,
   callback.Run(true);
 }
 
-void LevelDBWrapperImpl::Get(mojo::Array<uint8_t> key,
+void LevelDBWrapperImpl::Get(const std::vector<uint8_t>& key,
                              const GetCallback& callback) {
   if (!map_) {
-    LoadMap(
-        base::Bind(&LevelDBWrapperImpl::Get, base::Unretained(this),
-                   base::Passed(&key), callback));
+    LoadMap(base::Bind(&LevelDBWrapperImpl::Get, base::Unretained(this), key,
+                       callback));
     return;
   }
 
   auto found = map_->find(key);
   if (found == map_->end()) {
-    callback.Run(false, mojo::Array<uint8_t>());
+    callback.Run(false, std::vector<uint8_t>());
     return;
   }
-  callback.Run(true, found->second.Clone());
+  callback.Run(true, *(found->second));
 }
 
-void LevelDBWrapperImpl::GetAll(const mojo::String& source,
+void LevelDBWrapperImpl::GetAll(const std::string& source,
                                 const GetAllCallback& callback) {
   if (!map_) {
     LoadMap(
@@ -222,11 +217,11 @@ void LevelDBWrapperImpl::GetAll(const mojo::String& source,
     return;
   }
 
-  mojo::Array<mojom::KeyValuePtr> all;
+  std::vector<mojom::KeyValuePtr> all;
   for (const auto& it : (*map_)) {
     mojom::KeyValuePtr kv = mojom::KeyValue::New();
-    kv->key = it.first.Clone();
-    kv->value = it.second.Clone();
+    kv->key = it.first;
+    kv->value = *(it.second);
     all.push_back(std::move(kv));
   }
   callback.Run(leveldb::mojom::DatabaseError::OK, std::move(all));
@@ -260,7 +255,7 @@ void LevelDBWrapperImpl::OnLoadComplete(
   DCHECK(!map_);
   map_.reset(new ValueMap);
   for (auto& it : data)
-    (*map_)[std::move(it->key)] = std::move(it->value);
+    (*map_)[it->key.PassStorage()] = it->value.PassStorage();
 
   // We proceed without using a backing store, nothing will be persisted but the
   // class is functional for the lifetime of the object.
@@ -334,12 +329,12 @@ void LevelDBWrapperImpl::CommitChanges() {
   for (auto& it : commit_batch_->changed_values) {
     leveldb::mojom::BatchedOperationPtr item =
         leveldb::mojom::BatchedOperation::New();
-    item->key = it.first.Clone();
-    if (it.second.is_null()) {
+    item->key = std::vector<uint8_t>(it.first);
+    if (!it.second) {

[michaeln, 2016/08/02 00:28:29]

ah, i guess this is why the value is optional, to distinguish between puts and
deletes

[leonhsl(Using Gerrit), 2016/08/02 11:11:05]

Yeah ;-) while mojo::Array has null state inside but std::vector has no, so we
need to use base::optional to enable represent a null value which means a
delete.

       item->type = leveldb::mojom::BatchOperationType::DELETE_KEY;
     } else {
       item->type = leveldb::mojom::BatchOperationType::PUT_KEY;
-      item->value = std::move(it.second);
+      item->value = std::move(*(it.second));
     }
     operations.push_back(std::move(item));
   }