Small: Serialize the store's hash_prefix_map_ to file in WriteToDisk()

components/safe_browsing_db/v4_store.cc

Index: components/safe_browsing_db/v4_store.cc
diff --git a/components/safe_browsing_db/v4_store.cc b/components/safe_browsing_db/v4_store.cc
index 83f3b999b5e45e383a41f14d2098f39cfd45f9a8..fe75473049b76cf4f0c4871f32d75db0dc02b7c1 100644
--- a/components/safe_browsing_db/v4_store.cc
+++ b/components/safe_browsing_db/v4_store.cc
@@ -219,6 +219,11 @@ ApplyUpdateResult V4Store::ProcessPartialUpdateAndWriteToDisk(
 
   ApplyUpdateResult result =
       ProcessUpdate(metric, hash_prefix_map_old, response);
+  if (result == APPLY_UPDATE_SUCCESS) {
+    Checksum checksum = response->checksum();
+    response.reset();
+    RecordStoreWriteResult(WriteToDisk(checksum));
+  }
   return result;
 }
 
@@ -227,7 +232,9 @@ ApplyUpdateResult V4Store::ProcessFullUpdateAndWriteToDisk(
     std::unique_ptr<ListUpdateResponse> response) {
   ApplyUpdateResult result = ProcessFullUpdate(metric, response);
   if (result == APPLY_UPDATE_SUCCESS) {
-    RecordStoreWriteResult(WriteToDisk(std::move(response)));
+    Checksum checksum = response->checksum();
+    response.reset();
+    RecordStoreWriteResult(WriteToDisk(checksum));
   }
   return result;
 }
@@ -642,6 +649,7 @@ StoreReadResult V4Store::ReadFromDisk() {
   if (!file_format.ParseFromString(contents)) {
     return PROTO_PARSING_FAILURE;
   }
+  contents.clear();

[Nathan Parker, 2016/10/14 00:10:05]

I think this doesn't actually free memory. You probably want to call
shrink_to_fit() after, or else declare the string as a unique_ptr so you can
reset it.

[Scott Hess - ex-Googler, 2016/10/14 00:16:18]

The canonical way to release string memory is something like
std::string().swap(contents), though something horrible like
ignore_result(std::move(contents)) would probably work, too.  Of course you
could just scope it appropriately, albeit at some cost to rearrange your
variable declarations.

Also IMHO, don't unique_ptr it.  It works, but it's not much work to avoid the
extra heap allocation.

 
   if (file_format.magic_number() != kFileMagic) {
     return UNEXPECTED_MAGIC_NUMBER_FAILURE;
@@ -671,28 +679,27 @@ StoreReadResult V4Store::ReadFromDisk() {
   return READ_SUCCESS;
 }
 
-StoreWriteResult V4Store::WriteToDisk(
-    std::unique_ptr<ListUpdateResponse> response) const {
-  // Do not write partial updates to the disk.
-  // After merging the updates, the ListUpdateResponse passed to this method
-  // should be a FULL_UPDATE.
-  if (!response->has_response_type() ||
-      response->response_type() != ListUpdateResponse::FULL_UPDATE) {
-    DVLOG(1) << "Failure: response->has_response_type(): "
-             << response->has_response_type()
-             << " : response->response_type(): " << response->response_type();
-    return INVALID_RESPONSE_TYPE_FAILURE;
+StoreWriteResult V4Store::WriteToDisk(const Checksum& checksum) const {
+  V4StoreFileFormat file_format;
+  ListUpdateResponse* lur = file_format.mutable_list_update_response();
+  *(lur->mutable_checksum()) = checksum;
+  lur->set_new_client_state(state_);
+  lur->set_response_type(ListUpdateResponse::FULL_UPDATE);
+  for (auto map_iter : hash_prefix_map_) {
+    ThreatEntrySet* additions = lur->add_additions();
+    // TODO(vakh): Write RICE encoded hash prefixes on disk. Not doing so
+    // currently since it takes a long time to decode them on startup, which
+    // blocks resource load. See: http://crbug.com/654819
+    additions->set_compression_type(RAW);
+    additions->mutable_raw_hashes()->set_prefix_size(map_iter.first);
+    additions->mutable_raw_hashes()->set_raw_hashes(map_iter.second);
   }
 
   // Attempt writing to a temporary file first and at the end, swap the files.
   const base::FilePath new_filename = TemporaryFileForFilename(store_path_);
 
-  V4StoreFileFormat file_format;
   file_format.set_magic_number(kFileMagic);
   file_format.set_version_number(kFileVersion);
-  ListUpdateResponse* response_to_write =
-      file_format.mutable_list_update_response();
-  response_to_write->Swap(response.get());
   std::string file_format_string;
   file_format.SerializeToString(&file_format_string);
   size_t written = base::WriteFile(new_filename, file_format_string.data(),