Cleanup and minor refactoring of LogDnsClient

components/certificate_transparency/log_dns_client.cc

Index: components/certificate_transparency/log_dns_client.cc
diff --git a/components/certificate_transparency/log_dns_client.cc b/components/certificate_transparency/log_dns_client.cc
index dd7810d04c9072d930e3b8c6095439daf13c5178..fabbbac4698c36a0e36c9d5690f38ab9aa85587a 100644
--- a/components/certificate_transparency/log_dns_client.cc
+++ b/components/certificate_transparency/log_dns_client.cc
@@ -105,22 +105,22 @@ class LogDnsClient::AuditProofQuery {
  public:
   // The LogDnsClient is guaranteed to outlive the AuditProofQuery, so it's safe
   // to leave ownership of |dns_client| with LogDnsClient.
   AuditProofQuery(net::DnsClient* dns_client,
                   const std::string& domain_for_log,
-                  uint64_t tree_size,
                   const net::NetLogWithSource& net_log);
 
-  // Begins the process of getting an audit |proof| for the CT log entry with a
-  // leaf hash of |leaf_hash|. If it cannot be obtained synchronously,
-  // net::ERR_IO_PENDING will be returned and |callback| will be invoked when
-  // the operation has completed asynchronously.
-  // Ownership of |proof| remains with the caller, and it must not be deleted
-  // until the operation is complete.
+  // Begins the process of getting an audit proof for the CT log entry with a
+  // leaf hash of |leaf_hash|. The proof will be for a tree of size |tree_size|.
+  // If it cannot be obtained synchronously, net::ERR_IO_PENDING will be
+  // returned and |callback| will be invoked when the operation has completed
+  // asynchronously. Ownership of |proof| remains with the caller, and it must
+  // not be deleted until the operation is complete.
   net::Error Start(std::string leaf_hash,
+                   uint64_t tree_size,
                    const net::CompletionCallback& callback,
-                   net::ct::MerkleAuditProof* proof);
+                   net::ct::MerkleAuditProof* out_proof);
 
  private:
   enum class State {
     NONE,
     REQUEST_LEAF_INDEX,
@@ -176,13 +176,10 @@ class LogDnsClient::AuditProofQuery {
   State next_state_;
   // The DNS domain of the CT log that is being queried.
   std::string domain_for_log_;
   // The Merkle leaf hash of the CT log entry an audit proof is required for.
   std::string leaf_hash_;
-  // The size of the CT log's tree, from which the proof is requested.
-  // TODO(robpercival): Remove |tree_size| once |proof_| has a tree_size member.
-  uint64_t tree_size_;
   // The audit proof to populate.
   net::ct::MerkleAuditProof* proof_;
   // The callback to invoke when the query is complete.
   net::CompletionCallback callback_;
   // The DnsClient to use for sending DNS requests to the CT log.
@@ -199,15 +196,13 @@ class LogDnsClient::AuditProofQuery {
 };
 
 LogDnsClient::AuditProofQuery::AuditProofQuery(
     net::DnsClient* dns_client,
     const std::string& domain_for_log,
-    uint64_t tree_size,
     const net::NetLogWithSource& net_log)
     : next_state_(State::NONE),
       domain_for_log_(domain_for_log),
-      tree_size_(tree_size),
       dns_client_(dns_client),
       net_log_(net_log),
       weak_ptr_factory_(this) {
   DCHECK(dns_client_);
   DCHECK(!domain_for_log_.empty());
@@ -215,15 +210,17 @@ LogDnsClient::AuditProofQuery::AuditProofQuery(
 
 // |leaf_hash| is not a const-ref to allow callers to std::move that string into
 // the method, avoiding the need to make a copy.
 net::Error LogDnsClient::AuditProofQuery::Start(
     std::string leaf_hash,
+    uint64_t tree_size,
     const net::CompletionCallback& callback,
     net::ct::MerkleAuditProof* proof) {
   // It should not already be in progress.
   DCHECK_EQ(State::NONE, next_state_);
   proof_ = proof;
+  proof_->tree_size = tree_size;
   leaf_hash_ = std::move(leaf_hash);
   callback_ = callback;
   // The first step in the query is to request the leaf index corresponding to
   // |leaf_hash| from the CT log.
   next_state_ = State::REQUEST_LEAF_INDEX;
@@ -310,29 +307,29 @@ net::Error LogDnsClient::AuditProofQuery::RequestLeafIndexComplete(
   // This indicates either:
   // a) the wrong tree_size was provided.
   // b) the wrong leaf hash was provided.
   // c) there is a bug server-side.
   // The first two are more likely, so return ERR_INVALID_ARGUMENT.
-  if (proof_->leaf_index >= tree_size_) {
+  if (proof_->leaf_index >= proof_->tree_size) {
     return net::ERR_INVALID_ARGUMENT;
   }
 
   next_state_ = State::REQUEST_AUDIT_PROOF_NODES;
   return net::OK;
 }
 
 net::Error LogDnsClient::AuditProofQuery::RequestAuditProofNodes() {
   // Test pre-conditions (should be guaranteed by DNS response validation).
-  if (proof_->leaf_index >= tree_size_ ||
-      proof_->nodes.size() >=
-          net::ct::CalculateAuditPathLength(proof_->leaf_index, tree_size_)) {
+  if (proof_->leaf_index >= proof_->tree_size ||
+      proof_->nodes.size() >= net::ct::CalculateAuditPathLength(
+                                  proof_->leaf_index, proof_->tree_size)) {
     return net::ERR_UNEXPECTED;
   }
 
   std::string qname = base::StringPrintf(
       "%zu.%" PRIu64 ".%" PRIu64 ".tree.%s.", proof_->nodes.size(),
-      proof_->leaf_index, tree_size_, domain_for_log_.c_str());
+      proof_->leaf_index, proof_->tree_size, domain_for_log_.c_str());
 
   if (!StartDnsTransaction(qname)) {
     return net::ERR_NAME_RESOLUTION_FAILED;
   }
 
@@ -345,11 +342,11 @@ net::Error LogDnsClient::AuditProofQuery::RequestAuditProofNodesComplete(
   if (result != net::OK) {
     return result;
   }
 
   const uint64_t audit_path_length =
-      net::ct::CalculateAuditPathLength(proof_->leaf_index, tree_size_);
+      net::ct::CalculateAuditPathLength(proof_->leaf_index, proof_->tree_size);
 
   // The calculated |audit_path_length| can't ever be greater than 64, so
   // deriving the amount of memory to reserve from the untrusted |leaf_index|
   // is safe.
   proof_->nodes.reserve(audit_path_length);
@@ -430,15 +427,15 @@ net::Error LogDnsClient::QueryAuditProof(
   if (HasMaxConcurrentQueriesInProgress()) {
     return net::ERR_TEMPORARILY_THROTTLED;
   }
 
   AuditProofQuery* query = new AuditProofQuery(
-      dns_client_.get(), domain_for_log.as_string(), tree_size, net_log_);
+      dns_client_.get(), domain_for_log.as_string(), net_log_);
   // Transfers ownership of |query| to |audit_proof_queries_|.
   audit_proof_queries_.emplace_back(query);
 
-  return query->Start(std::move(leaf_hash),
+  return query->Start(std::move(leaf_hash), tree_size,
                       base::Bind(&LogDnsClient::QueryAuditProofComplete,
                                  weak_ptr_factory_.GetWeakPtr(),
                                  base::Unretained(query), callback),
                       proof);
 }