Display DNS probe results.

chrome/browser/net/dns_probe_runner.h

+// Copyright (c) 2013 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 CHROME_BROWSER_NET_DNS_PROBE_RUNNER_H
+
+#define CHROME_BROWSER_NET_DNS_PROBE_RUNNER_H

[mmenke, 2013/06/11 16:15:35]

nit:  Blank line not needed between these two lines.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

+
+#include <list>

[mmenke, 2013/06/11 16:15:35]

Not needed.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

+
+#include "base/bind.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "net/base/net_log.h"
+
+namespace net {
+class DnsClient;
+struct DnsConfig;
+class DnsResponse;
+class DnsTransaction;
+}
+
+namespace chrome_browser_net {
+
+// Runs DNS probes using a single DnsClient and evaluates the responses.

[mmenke, 2013/06/11 16:15:35]

nit:  responses -> response

[mmenke, 2013/06/11 16:15:35]

Think it's worth mentioning that we use google.com for testing.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Disagree; a DnsProbeRunner can run multiple probes over the course of its
lifetime, and it evaluates all of those responses.

+// Used by DnsProbeService to probe the system and public DNS configurations.
+class DnsProbeRunner {
+ public:
+  enum Result {
+    UNKNOWN,
+    CORRECT,
+    INCORRECT,

[mmenke, 2013/06/11 16:15:35]

Think it's worth commenting that any response that includes an IP address is
considered correct.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

+    FAILING,

[mmenke, 2013/06/11 16:15:35]

Also think it's worth mentioning this means we received a response, but it was a
malformed response.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

+    UNREACHABLE
+  };
+
+  typedef base::Callback<void(Result)> ProbeCallback;
+
+  DnsProbeRunner();
+  ~DnsProbeRunner();
+
+  void RunProbe(const ProbeCallback& callback);
+
+  void set_client(scoped_ptr<net::DnsClient> client);
+  bool is_running() const;

[mmenke, 2013/06/11 16:15:35]

If you use this naming scheme, the function should be inlined - so should either
change naming scheme, or inline the functions.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

+
+ private:
+  void OnTransactionComplete(bool async,
+                             net::DnsTransaction* transaction,
+                             int net_error,
+                             const net::DnsResponse* response);
+
+  // Evaluates the result of a DnsTransaction (whether the response was

[mmenke, 2013/06/11 16:15:35]

nit:  Think this comment could be clearer.  Suggest:

"Evaluates the result of a DnsTransaction, based on whether the response..."

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

+  // |async| or not, the |net_error| returned by the transaction, and the
+  // DNS |response| received from the server).
+  //
+  // Returns a judgment of whether the DNS configuration is returning
+  // correct results, incorrect results, errors, or nothing.
+  static Result EvaluateResponse(bool async,
+                                 int net_error,
+                                 const net::DnsResponse* response);

[mmenke, 2013/06/11 16:15:35]

Seems like this doesn't need to be declared here - can just go in a private
namespace in the cc file.  Then you can also get rid of a forward declaration.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

+
+  base::WeakPtrFactory<DnsProbeRunner> weak_factory_;
+  net::BoundNetLog bound_net_log_;

[mmenke, 2013/06/11 16:15:35]

No need for this, or the include in this file.  You can just use BoundNetLog()
whereever you need the BoundNetLog.  Passing in a NULL NetLog, like you're doing
in the other file, effectively makes the BoundNetLog do nothing.

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

+  scoped_ptr<net::DnsClient> client_;
+  ProbeCallback callback_;
+  scoped_ptr<net::DnsTransaction> transaction_;

[mmenke, 2013/06/11 16:15:35]

DISALLOW_COPY_AND_ASSIGN (And also the correspoding include).

[Deprecated (see juliatuttle), 2013/06/13 14:37:04]

Done.

+};
+
+}  // namespace chrome_browser_net
+
+#endif  // CHROME_BROWSER_NET_DNS_PROBE_RUNNER_H