Display DNS probe results.

chrome/browser/net/dns_probe_browsertest.cc

Index: chrome/browser/net/dns_probe_browsertest.cc
diff --git a/chrome/browser/net/dns_probe_browsertest.cc b/chrome/browser/net/dns_probe_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..194105400462984be6bd27faf928c4795c92b325
--- /dev/null
+++ b/chrome/browser/net/dns_probe_browsertest.cc
@@ -0,0 +1,439 @@
+// 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.
+
+#include "base/bind.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/path_service.h"
+#include "base/threading/thread_restrictions.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/google/google_util.h"
+#include "chrome/browser/io_thread.h"
+#include "chrome/browser/net/dns_probe_test_util.h"
+#include "chrome/browser/net/net_error_tab_helper.h"
+#include "chrome/browser/net/url_request_mock_util.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/net/net_error_info.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/test/net/url_request_failed_job.h"
+#include "content/test/net/url_request_mock_http_job.h"
+#include "net/base/net_errors.h"
+#include "net/dns/dns_test_util.h"
+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_job.h"
+#include "net/url_request/url_request_job_factory.h"
+
+using base::Bind;
+using base::Callback;
+using base::Closure;
+using base::ConstRef;
+using base::FilePath;
+using base::MessageLoop;
+using base::Unretained;
+using chrome_common_net::DnsProbeStatus;
+using content::BrowserThread;
+using content::URLRequestFailedJob;
+using content::URLRequestMockHTTPJob;
+using content::WebContents;
+using google_util::LinkDoctorBaseURL;
+using net::MockDnsClientRule;
+using net::NetworkDelegate;
+using net::URLRequest;
+using net::URLRequestFilter;
+using net::URLRequestJob;
+using net::URLRequestJobFactory;
+using ui_test_utils::NavigateToURL;
+using ui_test_utils::NavigateToURLBlockUntilNavigationsComplete;
+
+namespace chrome_browser_net {
+
+namespace {
+
+FilePath GetMockLinkDoctorFilePath() {
+  FilePath root_http;
+  PathService::Get(chrome::DIR_TEST_DATA, &root_http);
+  return root_http.AppendASCII("mock-link-doctor.html");
+}
+
+class BrokenLinkDoctorProtocolHandler
+    : public URLRequestJobFactory::ProtocolHandler {
+ public:
+  explicit BrokenLinkDoctorProtocolHandler(FilePath mock_link_doctor_file_path)

[mmenke, 2013/06/26 15:48:20]

nit:  const FilePath&?

[Deprecated (see juliatuttle), 2013/06/26 22:23:56]

Done.

+      : mock_link_doctor_file_path_(mock_link_doctor_file_path),
+        net_error_(net::OK) {}

[mmenke, 2013/06/26 15:48:20]

nit:  Google style guide requires a virtual destructor for any class with
virtual functions.

[Deprecated (see juliatuttle), 2013/06/26 22:23:56]

Done.

+
+  virtual URLRequestJob* MaybeCreateJob(
+      URLRequest* request, NetworkDelegate* network_delegate) const OVERRIDE {
+    if (net_error_ != net::OK) {
+      return new URLRequestFailedJob(request, network_delegate, net_error_);
+    } else {
+      return new URLRequestMockHTTPJob(
+          request, network_delegate, mock_link_doctor_file_path_);
+    }
+  }
+
+  void set_net_error(int net_error) { net_error_ = net_error; }
+
+ private:
+  const FilePath mock_link_doctor_file_path_;
+  int net_error_;
+};
+
+class DnsProbeBrowserTestIOThreadHelper {
+ public:
+  DnsProbeBrowserTestIOThreadHelper();
+  virtual ~DnsProbeBrowserTestIOThreadHelper() {}

[mmenke, 2013/06/26 15:48:20]

nit:  virtual no longer needed.

[Deprecated (see juliatuttle), 2013/06/26 22:23:56]

Done.

+
+  void SetUpOnIOThread(IOThread* io_thread);
+  void CleanUpOnIOThreadAndDeleteHelper();
+
+  void SetMockDnsClientRules(MockDnsClientRule::Result system_good_result,
+                             MockDnsClientRule::Result public_good_result);
+  void SetLinkDoctorNetError(int link_doctor_net_error);
+
+ private:
+  IOThread* io_thread_;
+  BrokenLinkDoctorProtocolHandler* protocol_handler_;
+  FilePath mock_link_doctor_file_path_;
+};
+
+DnsProbeBrowserTestIOThreadHelper::DnsProbeBrowserTestIOThreadHelper()
+    : io_thread_(NULL),
+      protocol_handler_(NULL),
+      mock_link_doctor_file_path_(GetMockLinkDoctorFilePath()) {}
+
+void DnsProbeBrowserTestIOThreadHelper::SetUpOnIOThread(IOThread* io_thread) {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  CHECK(io_thread);
+  CHECK(!io_thread_);
+  CHECK(!protocol_handler_);
+
+  io_thread_ = io_thread;
+  protocol_handler_ =
+      new BrokenLinkDoctorProtocolHandler(mock_link_doctor_file_path_);
+
+  URLRequestFailedJob::AddUrlHandler();
+
+  const GURL link_doctor_base_url = LinkDoctorBaseURL();
+  const std::string link_doctor_host = link_doctor_base_url.host();
+  scoped_ptr<URLRequestJobFactory::ProtocolHandler>
+      protocol_handler_scoped_ptr_(protocol_handler_);

[mmenke, 2013/06/26 15:48:20]

protocol_handler_scoped_ptr_ -> protocol_handler_scoped_ptr

[mmenke, 2013/06/26 15:48:20]

Think it's a little cleaner to always have something own the protocol handler. 
Suggest creating it in the scoped pointer first, and then setting
protocol_handler.

[Deprecated (see juliatuttle), 2013/06/26 22:23:56]

Done.

[Deprecated (see juliatuttle), 2013/06/26 22:23:56]

Done, but it requires an extra cast.

+  URLRequestFilter::GetInstance()->AddHostnameProtocolHandler(
+      "http", link_doctor_host, protocol_handler_scoped_ptr_.Pass());
+}
+
+void DnsProbeBrowserTestIOThreadHelper::CleanUpOnIOThreadAndDeleteHelper() {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  URLRequestFilter::GetInstance()->ClearHandlers();
+
+  delete this;
+}
+
+void DnsProbeBrowserTestIOThreadHelper::SetMockDnsClientRules(
+    MockDnsClientRule::Result system_good_result,
+    MockDnsClientRule::Result public_good_result) {
+  DnsProbeService* service = io_thread_->globals()->dns_probe_service.get();
+  service->SetSystemClientForTesting(
+      CreateMockDnsClientForProbes(system_good_result));
+  service->SetPublicClientForTesting(
+      CreateMockDnsClientForProbes(public_good_result));
+}
+
+void DnsProbeBrowserTestIOThreadHelper::SetLinkDoctorNetError(
+    int link_doctor_net_error) {
+  protocol_handler_->set_net_error(link_doctor_net_error);
+}
+
+class DnsProbeBrowserTest : public InProcessBrowserTest {
+ public:
+  DnsProbeBrowserTest();
+
+  virtual void SetUpOnMainThread() OVERRIDE;
+  virtual void CleanUpOnMainThread() OVERRIDE;
+
+ protected:
+  void SetLinkDoctorBroken(bool broken);
+  void SetMockDnsClientRules(MockDnsClientRule::Result system_good_result,
+                MockDnsClientRule::Result public_good_result);

[mmenke, 2013/06/26 15:48:20]

nit:  Don't need the goods.

[mmenke, 2013/06/26 15:48:20]

nit:  Indent here is a little weird.

[Deprecated (see juliatuttle), 2013/06/26 22:23:56]

Done.

[Deprecated (see juliatuttle), 2013/06/26 22:23:56]

Done.

+  void NavigateToDnsError();
+  void NavigateToOtherError();
+
+  void WaitForNetErrorInfo();
+
+  bool TitleIs(const std::string& expected);
+  bool PageContains(const std::string& expected);
+
+  DnsProbeStatus last_dns_probe_status() const {
+    return last_dns_probe_status_;
+  }
+  int dns_probe_status_count() const { return dns_probe_status_count_; }
+
+ private:
+  void OnDnsProbeStatusSent(DnsProbeStatus dns_probe_status);
+
+  DnsProbeBrowserTestIOThreadHelper* helper_;
+
+  bool awaiting_dns_probe_status_;
+  bool received_dns_probe_status_;
+  int dns_probe_status_count_;
+  DnsProbeStatus last_dns_probe_status_;
+};
+
+DnsProbeBrowserTest::DnsProbeBrowserTest()
+    : helper_(new DnsProbeBrowserTestIOThreadHelper()),
+      awaiting_dns_probe_status_(false),
+      received_dns_probe_status_(false),
+      dns_probe_status_count_(0) {
+}
+
+void DnsProbeBrowserTest::SetUpOnMainThread() {
+  NetErrorTabHelper::set_state_for_testing(
+      NetErrorTabHelper::TESTING_FORCE_ENABLED);
+
+  CHECK(helper_);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      Bind(&DnsProbeBrowserTestIOThreadHelper::SetUpOnIOThread,
+           Unretained(helper_),
+           g_browser_process->io_thread()));
+
+  NetErrorTabHelper* tab_helper = NetErrorTabHelper::FromWebContents(
+      browser()->tab_strip_model()->GetActiveWebContents());
+  tab_helper->set_dns_probe_status_snoop_callback_for_testing(Bind(
+      &DnsProbeBrowserTest::OnDnsProbeStatusSent,
+      Unretained(this)));
+}
+
+void DnsProbeBrowserTest::CleanUpOnMainThread() {
+  CHECK(helper_);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      Bind(&DnsProbeBrowserTestIOThreadHelper::CleanUpOnIOThreadAndDeleteHelper,
+           Unretained(helper_)));
+
+  NetErrorTabHelper::set_state_for_testing(
+      NetErrorTabHelper::TESTING_DEFAULT);
+}
+
+void DnsProbeBrowserTest::SetLinkDoctorBroken(bool broken) {
+  int net_error = broken ? net::ERR_NAME_NOT_RESOLVED : net::OK;
+
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      Bind(&DnsProbeBrowserTestIOThreadHelper::SetLinkDoctorNetError,
+           Unretained(helper_),
+           net_error));
+}
+
+// These two functions wait for two navigations because Link Doctor loads two
+// pages: a blank page, so the user stops seeing the previous page, and then
+// either the Link Doctor page or a regular error page.  We want to wait for
+// the error page, so we wait for both loads to finish.
+
+void DnsProbeBrowserTest::NavigateToDnsError() {
+  NavigateToURLBlockUntilNavigationsComplete(
+      browser(),
+      URLRequestFailedJob::GetMockHttpUrl(net::ERR_NAME_NOT_RESOLVED),
+      2);
+}
+
+void DnsProbeBrowserTest::NavigateToOtherError() {
+  NavigateToURLBlockUntilNavigationsComplete(
+      browser(),
+      URLRequestFailedJob::GetMockHttpUrl(net::ERR_CONNECTION_REFUSED),
+      2);
+}
+
+void DnsProbeBrowserTest::SetMockDnsClientRules(
+    MockDnsClientRule::Result system_good_result,
+    MockDnsClientRule::Result public_good_result) {

[mmenke, 2013/06/26 15:48:20]

nit:  Again, don't think "good" is needed any more.

[Deprecated (see juliatuttle), 2013/06/26 22:23:56]

Done.

+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      Bind(&DnsProbeBrowserTestIOThreadHelper::SetMockDnsClientRules,
+           Unretained(helper_),
+           system_good_result,
+           public_good_result));
+}
+
+void DnsProbeBrowserTest::WaitForNetErrorInfo() {
+  CHECK(!awaiting_dns_probe_status_);
+  while (!received_dns_probe_status_) {
+    awaiting_dns_probe_status_ = true;
+    MessageLoop::current()->Run();
+    awaiting_dns_probe_status_ = false;
+  }
+  received_dns_probe_status_ = false;
+}
+
+// Check title by roundtripping to renderer, to make sure any probe results
+// sent before this have been applied.
+bool DnsProbeBrowserTest::TitleIs(const std::string& expected) {
+  std::string title;
+
+  WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  bool rv = content::ExecuteScriptAndExtractString(
+      contents,
+      "domAutomationController.send(document.title);",
+      &title);
+  if (!rv)
+    return false;
+
+  return title == expected;
+}
+
+// Check text by roundtripping to renderer, to make sure any probe results
+// sent before this have been applied.
+bool DnsProbeBrowserTest::PageContains(const std::string& expected) {
+  std::string text_content;
+
+  bool rv = content::ExecuteScriptAndExtractString(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      "domAutomationController.send(document.body.textContent);",
+      &text_content);
+  if (!rv)
+    return false;
+
+  return text_content.find(expected) != std::string::npos;
+}
+
+void DnsProbeBrowserTest::OnDnsProbeStatusSent(
+    DnsProbeStatus dns_probe_status) {
+  last_dns_probe_status_ = dns_probe_status;
+
+  dns_probe_status_count_++;
+  received_dns_probe_status_ = true;
+  if (awaiting_dns_probe_status_)
+    MessageLoop::current()->Quit();
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, OtherErrorWithLinkDoctor) {
+  SetLinkDoctorBroken(false);
+
+  NavigateToOtherError();
+  EXPECT_TRUE(TitleIs("Mock Link Doctor"));
+
+  EXPECT_EQ(0, dns_probe_status_count());
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, OtherErrorWithoutLinkDoctor) {
+  SetLinkDoctorBroken(true);
+
+  NavigateToOtherError();
+  EXPECT_TRUE(PageContains("CONNECTION_REFUSED"));
+
+  EXPECT_EQ(0, dns_probe_status_count());
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, NxdomainWithLinkDoctor) {
+  SetLinkDoctorBroken(false);
+  SetMockDnsClientRules(MockDnsClientRule::OK, MockDnsClientRule::OK);
+
+  NavigateToDnsError();
+  EXPECT_TRUE(TitleIs("Mock Link Doctor"));
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, NoInternetWithoutLinkDoctor) {
+  SetLinkDoctorBroken(true);
+  SetMockDnsClientRules(MockDnsClientRule::TIMEOUT,
+                        MockDnsClientRule::TIMEOUT);
+
+  NavigateToDnsError();
+
+  bool seen_finished = false;
+  bool page_updated = false;
+
+  // Wait for zero or more DNS_PROBE_STARTED followed by one or more
+  // DNS_PROBE_FINISHED_NO_INTERNET.  After each of the latter, check to see if
+  // the error page has been updated.
+  while (!page_updated) {
+    WaitForNetErrorInfo();

[mmenke, 2013/06/26 15:48:20]

Re-entering the message loop in a loop like this is a bit of an anti-pattern
that's probably best to avoid.

Think it's a bit cleaner and simpler to have "WaitForNetErrorInfo" wait until we
send the final result for a probe, keeping an array of all intermediary results.
 Then we can just check the array at the end.

[Deprecated (see juliatuttle), 2013/06/26 22:23:56]

The problem is that the loop isn't, overall, waiting for the final result; it's
waiting for the page to be updated, and using the probe results as a way to know
when to check the page.

[mmenke, 2013/06/27 14:49:37]

Ah, right...Think the simplest solution would be to have a result queue, and
exit the message loop at the end of the queue (And check each result as they
come in).  Seen this done before, to avoid this general code flow.

Also, non-determinism in tests is a big cause of flake.

[Deprecated (see juliatuttle), 2013/06/28 16:59:46]

Done.

+    switch (last_dns_probe_status()) {
+      case chrome_common_net::DNS_PROBE_STARTED:
+        ASSERT_FALSE(seen_finished);
+        break;
+      case chrome_common_net::DNS_PROBE_FINISHED_NO_INTERNET:
+        seen_finished = true;
+        if (PageContains("DNS_PROBE_FINISHED_NO_INTERNET"))

[mmenke, 2013/06/26 15:48:20]

This is needed because of the blank page before we load the link doctor page? 
This seems like a bad idea to me.  PageContains itself runs a message loop, at
which point more probe statuses can be sent.  It's suddenly not clear what order
things happen in.

[Deprecated (see juliatuttle), 2013/06/26 22:23:56]

Hmm.  How do you think I should clarify this?  Basically, I want to say "check
the page for this string every time we get a probe status, until it contains
this text.  also, if you get any status other than this or that, it's bad".

[Deprecated (see juliatuttle), 2013/06/28 16:59:46]

Obsolete.

+          page_updated = true;
+        break;
+      default:
+        NOTREACHED();
+    }
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, SyncFailureWithoutLinkDoctor) {
+  SetLinkDoctorBroken(true);
+  SetMockDnsClientRules(MockDnsClientRule::FAIL_SYNC,
+                        MockDnsClientRule::FAIL_SYNC);
+
+  NavigateToDnsError();
+
+  bool seen_finished = false;
+  bool page_updated = false;
+
+  // Wait for zero or more DNS_PROBE_STARTED followed by one or more
+  // DNS_PROBE_FINISHED_NO_INTERNET.  After each of the latter, check to see if
+  // the error page has been updated.
+  while (!page_updated) {
+    WaitForNetErrorInfo();
+    switch (last_dns_probe_status()) {
+      case chrome_common_net::DNS_PROBE_STARTED:
+        ASSERT_FALSE(seen_finished);
+        break;
+      case chrome_common_net::DNS_PROBE_FINISHED_UNKNOWN:
+        seen_finished = true;
+        if (PageContains("NAME_NOT_RESOLVED"))
+          page_updated = true;
+        break;
+      default:
+        NOTREACHED();
+    }
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, NoProbeInSubframe) {
+  SetLinkDoctorBroken(false);
+
+  const FilePath::CharType kIframeDnsErrorHtmlName[] =
+      FILE_PATH_LITERAL("iframe_dns_error.html");
+
+  NavigateToURL(
+      browser(),
+      URLRequestMockHTTPJob::GetMockUrl(FilePath(kIframeDnsErrorHtmlName)));
+
+  // By the time NavigateToURL returns, the browser will have seen the failed
+  // provisional load.  If a probe was started (or considered but not run),
+  // then the NetErrorTabHelper would have sent a NetErrorInfo message.  Thus,
+  // if one hasn't been sent by now, the NetErrorTabHelper has not (and won't)
+  // start a probe for this DNS error.
+  EXPECT_EQ(0, dns_probe_status_count());
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, ProbesDisabled) {
+  NetErrorTabHelper::set_state_for_testing(
+      NetErrorTabHelper::TESTING_FORCE_DISABLED);
+
+  SetLinkDoctorBroken(true);
+  SetMockDnsClientRules(MockDnsClientRule::TIMEOUT,
+                        MockDnsClientRule::TIMEOUT);
+
+  NavigateToDnsError();
+
+  WaitForNetErrorInfo();
+  EXPECT_EQ(chrome_common_net::DNS_PROBE_NOT_RUN, last_dns_probe_status());
+}
+
+}  // namespace
+
+}  // namespace chrome_browser_net