Merge LogDnsClient's QueryLeafIndex and QueryAuditProof methods

components/certificate_transparency/log_dns_client.h

 // 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.
 
 #ifndef COMPONENTS_CERTIFICATE_TRANSPARENCY_LOG_DNS_CLIENT_H_
 #define COMPONENTS_CERTIFICATE_TRANSPARENCY_LOG_DNS_CLIENT_H_
 
 #include <stdint.h>
 
 #include <list>
@@ skipping 16 matching lines @@
 
 namespace certificate_transparency {
 
 // Queries Certificate Transparency (CT) log servers via DNS.
 // All queries are performed asynchronously.
 // For more information, see
 // https://github.com/google/certificate-transparency-rfcs/blob/master/dns/draft-ct-over-dns.md.
 // It must be created and deleted on the same thread. It is not thread-safe.
 class LogDnsClient : public net::NetworkChangeNotifier::DNSObserver {
  public:
-  // Invoked when a leaf index query completes.
-  // If an error occured, |net_error| will be a net::Error code, otherwise it
-  // will be net::OK and |leaf_index| will be the leaf index that was received.
-  using LeafIndexCallback =
-      base::Callback<void(int net_error, uint64_t leaf_index)>;
   // Invoked when an audit proof query completes.
   // If an error occurred, |net_error| will be a net::Error code, otherwise it
   // will be net::OK and |proof| will be the audit proof that was received.
   // The log ID of |proof| will not be set, as that is not known by this class,
   // but the leaf index will be set.
   using AuditProofCallback =
       base::Callback<void(int net_error,
                           std::unique_ptr<net::ct::MerkleAuditProof> proof)>;
 
   // Creates a log client that will take ownership of |dns_client| and use it
@@ skipping 11 matching lines @@
   ~LogDnsClient() override;
 
   // Called by NetworkChangeNotifier when the DNS config changes.
   // The DnsClient's config will be updated in response.
   void OnDNSChanged() override;
 
   // Called by NetworkChangeNotifier when the DNS config is first read.
   // The DnsClient's config will be updated in response.
   void OnInitialDNSConfigRead() override;
 
-  // Queries a CT log to discover the index of the leaf with |leaf_hash|.
-  // The log is identified by |domain_for_log|, which is the DNS name used as a
-  // suffix for all queries.
-  // The |leaf_hash| is the SHA-256 hash of a Merkle tree leaf in that log.
-  // The |callback| is invoked when the query is complete, or an error occurs.
-  void QueryLeafIndex(base::StringPiece domain_for_log,
-                      base::StringPiece leaf_hash,
-                      const LeafIndexCallback& callback);
-
-  // Queries a CT log to retrieve an audit proof for the leaf at |leaf_index|.
+  // Queries a CT log to retrieve an audit proof for the leaf with |leaf_hash|.
+  // The |leaf_hash| is the SHA-256 Merkle leaf hash (see RFC6962, section 2.1).
   // The size of the CT log tree must be provided in |tree_size|.
   // The log is identified by |domain_for_log|, which is the DNS name used as a
   // suffix for all queries.
   // The |callback| is invoked when the query is complete, or an error occurs.
   void QueryAuditProof(base::StringPiece domain_for_log,
-                       uint64_t leaf_index,
+                       base::StringPiece leaf_hash,
                        uint64_t tree_size,
                        const AuditProofCallback& callback);
 
  private:
-  void QueryLeafIndexComplete(net::DnsTransaction* transaction,
+  void QueryLeafIndex(base::StringPiece domain_for_log,
+                      base::StringPiece leaf_hash,
+                      uint64_t tree_size,
+                      const AuditProofCallback& callback);
+
+  void QueryLeafIndexComplete(base::StringPiece domain_for_log,
+                              uint64_t tree_size,
+                              net::DnsTransaction* transaction,
                               int neterror,
                               const net::DnsResponse* response);
 
   // Queries a CT log to retrieve part of an audit |proof|. The |node_index|
   // indicates which node of the audit proof/ should be requested. The CT log
   // may return up to 7 nodes, starting from |node_index| (this is the maximum
   // that will fit in a DNS UDP packet). The nodes will be appended to
   // |proof->nodes|.
   void QueryAuditProofNodes(std::unique_ptr<net::ct::MerkleAuditProof> proof,
                             base::StringPiece domain_for_log,
                             uint64_t tree_size,
                             uint64_t node_index,
                             const AuditProofCallback& callback);
 
   void QueryAuditProofNodesComplete(
       std::unique_ptr<net::ct::MerkleAuditProof> proof,
       base::StringPiece domain_for_log,
       uint64_t tree_size,
       net::DnsTransaction* transaction,
       int net_error,
       const net::DnsResponse* response);

[Ryan Sleevi, 2016/09/23 21:59:29]

Can these four methods be encapsulated into an internal implementation? It seems
like they could - that is, the only sort of private structures necessary are the
dns_client and maybe the bound_net_log, but those could be passed internally in
the constructor of the internal "Query" object - because they're guaranteed to
outlive the query (c.f. 126ish)

[Rob Percival, 2016/09/27 17:44:17]

Done.

 
   // Returns true if the maximum number of queries are currently in flight.
   // If the maximum number of concurrency queries is set to 0, this will always
   // return false.
   bool HasMaxConcurrentQueriesInProgress() const;
 
   // Updates the |dns_client_| config using NetworkChangeNotifier.
   void UpdateDnsConfig();
 
   // A DNS query that is in flight.
-  template <typename CallbackType>
-  struct Query {
-    std::unique_ptr<net::DnsTransaction> transaction;
-    CallbackType callback;
-  };
+  struct Query;
 
   // Used to perform DNS queries.
   std::unique_ptr<net::DnsClient> dns_client_;
   // Passed to the DNS client for logging.
   net::BoundNetLog net_log_;
-  // Leaf index queries that haven't completed yet.
-  std::list<Query<LeafIndexCallback>> leaf_index_queries_;
   // Audit proof queries that haven't completed yet.
-  std::list<Query<AuditProofCallback>> audit_proof_queries_;
+  std::list<Query> audit_proof_queries_;
   // The maximum number of queries that can be in flight at one time.
   size_t max_concurrent_queries_;
   // Creates weak_ptrs to this, for callback purposes.
   base::WeakPtrFactory<LogDnsClient> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(LogDnsClient);
 };
 
 }  // namespace certificate_transparency
 #endif  // COMPONENTS_CERTIFICATE_TRANSPARENCY_LOG_DNS_CLIENT_H_