SafeBrowsing PVer4: Send mutable response to the database and the stores

components/safe_browsing_db/v4_update_protocol_manager.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/safe_browsing_db/v4_update_protocol_manager.h"
 
 #include <utility>
 
 #include "base/base64.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/rand_util.h"
 #include "base/timer/timer.h"
 #include "components/safe_browsing_db/safebrowsing.pb.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_context_getter.h"
 
@@ skipping 194 matching lines @@
   // Serialize and Base64 encode.
   std::string req_data, req_base64;
   request.SerializeToString(&req_data);
   base::Base64Encode(req_data, &req_base64);
 
   return req_base64;
 }
 
 bool V4UpdateProtocolManager::ParseUpdateResponse(
     const std::string& data,
-    std::vector<ListUpdateResponse>* list_update_responses) {
+    ParsedServerResponse* parsed_server_response) {
   FetchThreatListUpdatesResponse response;
 
   if (!response.ParseFromString(data)) {
     RecordParseUpdateResult(PARSE_FROM_STRING_ERROR);
     return false;
   }
 
   if (response.has_minimum_wait_duration()) {
     // Seconds resolution is good enough so we ignore the nanos field.
     int64_t minimum_wait_duration_seconds =
         response.minimum_wait_duration().seconds();
 
     // Do not let the next_update_interval_ to be too low.
     if (minimum_wait_duration_seconds < kV4TimerStartIntervalSecMin) {
       minimum_wait_duration_seconds = kV4TimerStartIntervalSecMin;
     }
     next_update_interval_ =
         base::TimeDelta::FromSeconds(minimum_wait_duration_seconds);
   }
 
   // TODO(vakh): Do something useful with this response.
-  for (const ListUpdateResponse& list_update_response :
-       response.list_update_responses()) {
+  for (ListUpdateResponse list_update_response :

[Nathan Parker, 2016/06/28 19:00:38]

I think this should be a reference, so you can swap them out below.

[vakh (use Gerrit instead), 2016/06/28 21:34:14]

Done.

+       *response.mutable_list_update_responses()) {
     if (!list_update_response.has_platform_type()) {
       RecordParseUpdateResult(NO_PLATFORM_TYPE_ERROR);
     } else if (!list_update_response.has_threat_entry_type()) {
       RecordParseUpdateResult(NO_THREAT_ENTRY_TYPE_ERROR);
     } else if (!list_update_response.has_threat_type()) {
       RecordParseUpdateResult(NO_THREAT_TYPE_ERROR);
     } else if (!list_update_response.has_new_client_state()) {
       RecordParseUpdateResult(NO_STATE_ERROR);
     } else {
-      list_update_responses->push_back(list_update_response);
+      ListUpdateResponse* add = new ListUpdateResponse(list_update_response);

[Nathan Parker, 2016/06/28 19:00:38]

Rather than copying, you could swap:

ListUpdateResponse* add = new ListUpdateResponse;
add->Swap(&list_update_response);
..push_back(..)

[vakh (use Gerrit instead), 2016/06/28 21:34:14]

Done.

+      parsed_server_response->push_back(base::WrapUnique(add));
     }
   }
   return true;
 }
 
 void V4UpdateProtocolManager::IssueUpdateRequest() {
   DCHECK(CalledOnValidThread());
 
   // If an update request is already pending, record and return silently.
   if (request_.get()) {
@@ skipping 26 matching lines @@
     const net::URLFetcher* source) {
   DCHECK(CalledOnValidThread());
 
   int response_code = source->GetResponseCode();
   net::URLRequestStatus status = source->GetStatus();
   V4ProtocolManagerUtil::RecordHttpResponseOrErrorCode(
       "SafeBrowsing.V4UpdateHttpResponseOrErrorCode", status, response_code);
 
   last_response_time_ = Time::Now();
 
-  std::vector<ListUpdateResponse> list_update_responses;
+  std::unique_ptr<ParsedServerResponse> parsed_server_response(
+      new ParsedServerResponse);
   if (status.is_success() && response_code == net::HTTP_OK) {
     RecordUpdateResult(V4OperationResult::STATUS_200);
     ResetUpdateErrors();
     std::string data;
     source->GetResponseAsString(&data);
-    if (!ParseUpdateResponse(data, &list_update_responses)) {
-      list_update_responses.clear();
+    if (!ParseUpdateResponse(data, parsed_server_response.get())) {
+      parsed_server_response->clear();
       RecordUpdateResult(V4OperationResult::PARSE_ERROR);
     }
     request_.reset();
 
     UMA_HISTOGRAM_COUNTS("SafeBrowsing.V4UpdateResponseSizeKB",
                          data.size() / 1024);
 
-    // Invoke the callback with list_update_responses.
-    // The caller should update its state now, based on list_update_responses.
+    // The caller should update its state now, based on parsed_server_response.
     // The callback must call ScheduleNextUpdate() at the end to resume
     // downloading updates.
-    update_callback_.Run(list_update_responses);
+    update_callback_.Run(std::move(parsed_server_response));
   } else {
     DVLOG(1) << "SafeBrowsing GetEncodedUpdates request for: "
              << source->GetURL() << " failed with error: " << status.error()
              << " and response code: " << response_code;
 
     if (status.status() == net::URLRequestStatus::FAILED) {
       RecordUpdateResult(V4OperationResult::NETWORK_ERROR);
     } else {
       RecordUpdateResult(V4OperationResult::HTTP_ERROR);
     }
     // TODO(vakh): Figure out whether it is just a network error vs backoff vs
     // another condition and RecordUpdateResult more accurately.
 
     request_.reset();
     ScheduleNextUpdateWithBackoff(true);
   }
 }
 
 void V4UpdateProtocolManager::GetUpdateUrlAndHeaders(
     const std::string& req_base64,
     GURL* gurl,
     net::HttpRequestHeaders* headers) const {
   V4ProtocolManagerUtil::GetRequestUrlAndHeaders(
       req_base64, "threatListUpdates:fetch", config_, gurl, headers);
 }
 
 }  // namespace safe_browsing