Display DNS probe results.

chrome/browser/net/dns_probe_browsertest.cc

+// 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 "chrome/browser/browser_process.h"
+#include "chrome/browser/google/google_util.h"
+#include "chrome/browser/io_thread.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/url_request/url_request_filter.h"
+#include "net/url_request/url_request_job.h"
+#include "net/url_request/url_request_job_factory.h"
+
+namespace {
+
+using base::Bind;
+using base::Callback;
+using base::Closure;
+using base::ConstRef;
+using base::FilePath;
+using base::Unretained;
+using chrome_browser_net::DnsProbeService;
+using chrome_browser_net::NetErrorTabHelper;
+using chrome_browser_net::SetUrlRequestMocksEnabled;
+using chrome_common_net::DnsProbeStatus;
+using content::BrowserThread;
+using content::URLRequestFailedJob;
+using content::URLRequestMockHTTPJob;
+using content::WebContents;
+using google_util::LinkDoctorBaseURL;
+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;
+
+// TODO(ttuttle): Dedupe this and net/base/test_completion_callback.*.
+class TestCompletionCallback {
+ public:
+  // TODO(ttuttle): Weak pointer instead of Unretained?
+  TestCompletionCallback()
+      : callback_(Bind(&TestCompletionCallback::DidCall, Unretained(this))),
+        waiting_(false),
+        called_(false) {
+  }
+
+  void DidCall() {
+    called_ = true;
+    if (waiting_)
+      MessageLoop::current()->Quit();
+  }
+
+  void WaitForCall() {
+    DCHECK(!waiting_);
+    while (!called_) {
+      waiting_ = true;
+      MessageLoop::current()->Run();
+      waiting_ = false;
+    }
+    called_ = false;
+  }
+
+  const Closure& callback() {
+    return callback_;
+  }
+
+ private:
+  Closure callback_;
+  bool waiting_;
+  bool called_;
+};
+
+class MockDnsProbeService : public DnsProbeService {

[mmenke, 2013/05/28 18:20:13]

I'd rather use the real DnsProbeService.  Also think it's much cleaner if we
mock out the same class in both sets of tests.  My suggestion is to have the
JobFactory manage the probes - creates them and owns them, and passes both their
results back to the probe service in a single function.

In that case, JobManager may be a better name.

WDYT?

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

Done.

+ public:
+  explicit MockDnsProbeService(const Closure& probe_started_callback)
+      : DnsProbeService(),
+        probe_started_callback_(probe_started_callback) {
+  }
+
+  virtual ~MockDnsProbeService() { }
+
+  virtual void ProbeDns(const CallbackType& callback) OVERRIDE {
+    callbacks_.push_back(callback);
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE, probe_started_callback_);
+  }
+
+  void CallCallbacks(DnsProbeStatus result) {
+    std::vector<CallbackType> callbacks = callbacks_;
+    callbacks_.clear();
+
+    for (std::vector<CallbackType>::const_iterator i = callbacks.begin();
+         i != callbacks.end(); ++i) {
+      i->Run(result);
+    }
+  }
+
+  int callback_count() {
+    return callbacks_.size();
+  }
+
+ private:
+  const Closure probe_started_callback_;
+  std::vector<CallbackType> callbacks_;
+};
+
+class DnsProbeBrowserTestIOThreadHelper {
+ public:
+  DnsProbeBrowserTestIOThreadHelper()
+      : io_thread_(NULL),
+        real_service_(NULL),
+        mock_service_(NULL),
+        link_doctor_net_error_(net::OK) {
+  }
+
+  void SetUpOnIOThread(
+      IOThread* io_thread,
+      const Closure& probe_started_callback,
+      const Closure& callback);
+  void CleanUpOnIOThread(const Closure& callback);
+
+  void SimulateProbeResult(
+      DnsProbeStatus result,
+      const Closure& callback);
+
+  int link_doctor_net_error() const {
+    return link_doctor_net_error_;
+  }
+  void set_link_doctor_net_error(int link_doctor_net_error) {
+    link_doctor_net_error_ = link_doctor_net_error;
+  }
+
+ private:
+  IOThread* io_thread_;
+  DnsProbeService* real_service_;
+  MockDnsProbeService* mock_service_;
+  int link_doctor_net_error_;
+};
+
+class DnsProbeBrowserTest : public InProcessBrowserTest {
+ public:
+  DnsProbeBrowserTest();
+
+  virtual void SetUpOnMainThread() OVERRIDE;
+  virtual void CleanUpOnMainThread() OVERRIDE;
+
+ protected:

[mmenke, 2013/05/28 18:20:13]

optional:  Most test fixtures don't use protected, since they aren't subclassed
and can't be accessed from other files.

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

The test cases are implemented as subclasses, I thought.  I use protected for
stuff that can be called from test cases and private for stuff that's only
accessed through the test fixture itself.

+  void SetLinkDoctorBroken(bool broken);
+  void NavigateToDnsError();
+  void NavigateToOtherError();
+  void WaitForProbe();

[mmenke, 2013/05/28 18:20:13]

I think "WaitForProbeStart" would be clearer.

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

Done.

+  void SimulateProbeResult(DnsProbeStatus result);
+
+  bool TitleContains(const std::string& expected);
+  bool PageContains(const std::string& expected);
+
+  int probe_count() const { return probe_started_count_; }
+
+ private:
+  void SetLinkDoctorNetError(int net_error);
+  void OnProbeStarted();
+
+  DnsProbeBrowserTestIOThreadHelper helper_;
+  TestCompletionCallback probe_started_callback_;

[mmenke, 2013/05/28 18:20:13]

I think having two variables in this file called probe_started_callback_, one of
which is a closure and the other a TestCompletionCallback is a bit confusion.

Suggest renaming this one and adding a comment.

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

Obsolete.

+  int probe_started_count_;
+};
+
+class BrokenLinkDoctorProtocolHandler
+    : public URLRequestJobFactory::ProtocolHandler {
+ public:
+  explicit BrokenLinkDoctorProtocolHandler(
+      DnsProbeBrowserTestIOThreadHelper* helper)
+      : helper_(helper) { }
+
+  virtual URLRequestJob* MaybeCreateJob(
+      URLRequest* request, NetworkDelegate* network_delegate) const OVERRIDE {
+    int net_error = helper_->link_doctor_net_error();
+    if (net_error != net::OK) {
+      return new URLRequestFailedJob(request, network_delegate, net_error);
+    } else {
+      FilePath file_path = GetMockLinkDoctorFilePath();
+      return new URLRequestMockHTTPJob(request, network_delegate, file_path);
+    }
+  }
+
+ private:
+  FilePath GetMockLinkDoctorFilePath() const {
+    FilePath root_http;
+    PathService::Get(chrome::DIR_TEST_DATA, &root_http);
+    return root_http.AppendASCII("mock-link-doctor.html");
+  }
+
+  const DnsProbeBrowserTestIOThreadHelper* helper_;
+};
+
+DnsProbeBrowserTest::DnsProbeBrowserTest()
+    : probe_started_count_(0) {
+}
+
+void DnsProbeBrowserTest::SetUpOnMainThread() {
+  NetErrorTabHelper::set_state_for_testing(
+      NetErrorTabHelper::TESTING_FORCE_ENABLED);
+
+  Closure on_probe_started =
+      Bind(&DnsProbeBrowserTest::OnProbeStarted, Unretained(this));
+
+  TestCompletionCallback io_thread_done;
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      Bind(&DnsProbeBrowserTestIOThreadHelper::SetUpOnIOThread,
+           Unretained(&helper_),
+           g_browser_process->io_thread(),
+           ConstRef(on_probe_started),
+           ConstRef(io_thread_done.callback())));
+  io_thread_done.WaitForCall();

[mmenke, 2013/05/28 18:20:13]

Waiting on this does not seem to be needed.  Any task we later post to the IO
thread will necessarily happen after SetUpOnIOThread is run.

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

Done.

+}
+
+void DnsProbeBrowserTestIOThreadHelper::SetUpOnIOThread(

[mmenke, 2013/05/28 18:20:13]

DnsProbeBrowserTestIOThreadHelper's functions should be defined right under the
definition of DnsProbeBrowserTestIOThreadHelper.

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

My goal was to arrange them in the order of what actually happens, since methods
from the two are interleaved.  I can separate them if you want.

+    IOThread* io_thread,
+    const Closure& probe_started_callback,
+    const Closure& callback) {
+  DCHECK(io_thread);
+  DCHECK(!io_thread_ && !real_service_ && !mock_service_);
+
+  io_thread_ = io_thread;
+
+  mock_service_ = new MockDnsProbeService(probe_started_callback);
+
+  real_service_ = io_thread_->globals()->dns_probe_service.release();
+  io_thread_->globals()->dns_probe_service.reset(mock_service_);
+
+  // SetUrlRequestMocksEnabled clears the filter list and then adds filters
+  // for several things, including the mock link doctor.  So, we call it
+  // first, then remove the handler it's added for the mock link doctor and
+  // add our own.
+  SetUrlRequestMocksEnabled(true);
+
+  URLRequestFilter* filter = URLRequestFilter::GetInstance();
+  const GURL link_doctor_base_url = LinkDoctorBaseURL();
+  const std::string host = link_doctor_base_url.host();
+  scoped_ptr<URLRequestJobFactory::ProtocolHandler> handler(
+      new BrokenLinkDoctorProtocolHandler(this));
+  filter->RemoveHostnameHandler("http", host);
+  filter->AddHostnameProtocolHandler("http", host, handler.Pass());
+
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+}
+
+void DnsProbeBrowserTest::CleanUpOnMainThread() {
+  NetErrorTabHelper::set_state_for_testing(
+      NetErrorTabHelper::TESTING_DEFAULT);
+
+  TestCompletionCallback io_thread_done;
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      Bind(&DnsProbeBrowserTestIOThreadHelper::CleanUpOnIOThread,
+           Unretained(&helper_),
+           ConstRef(io_thread_done.callback())));
+  io_thread_done.WaitForCall();

[mmenke, 2013/05/28 18:20:13]

Rather than waiting on a callback, how about having
DnsProbeBrowserTestIOThreadHelper destroyed on the IO thread, in the function
called from CleanUpOnMainThread.

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

Done.

+}
+
+void DnsProbeBrowserTestIOThreadHelper::CleanUpOnIOThread(
+    const Closure& callback) {
+  DCHECK(io_thread_ && real_service_ && mock_service_);
+
+  SetUrlRequestMocksEnabled(false);
+
+  DnsProbeService* mock_service;
+  mock_service = io_thread_->globals()->dns_probe_service.release();
+  DCHECK_EQ(mock_service_, mock_service);
+  io_thread_->globals()->dns_probe_service.reset(real_service_);
+  real_service_ = NULL;
+
+  delete mock_service_;
+  mock_service_ = NULL;
+
+  io_thread_ = NULL;
+
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+}
+
+void DnsProbeBrowserTest::SetLinkDoctorBroken(bool broken) {
+  SetLinkDoctorNetError(broken ? net::ERR_NAME_NOT_RESOLVED : net::OK);
+}
+
+// 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::WaitForProbe() {
+  probe_started_callback_.WaitForCall();
+}
+
+void DnsProbeBrowserTest::SimulateProbeResult(DnsProbeStatus result) {
+  TestCompletionCallback io_thread_done;
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      Bind(&DnsProbeBrowserTestIOThreadHelper::SimulateProbeResult,
+           Unretained(&helper_),
+           result,
+           ConstRef(io_thread_done.callback())));
+  io_thread_done.WaitForCall();
+}
+
+void DnsProbeBrowserTestIOThreadHelper::SimulateProbeResult(
+    DnsProbeStatus result,
+    const Closure& callback) {
+  DCHECK(mock_service_);
+  mock_service_->CallCallbacks(result);
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, callback);
+}
+
+bool DnsProbeBrowserTest::TitleContains(const std::string& expected) {

[mmenke, 2013/05/28 18:20:13]

Is there a reason not to use TitleWatcher?

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

I don't want to wait for it; I expect it to have this title right now.

(I could move to TitleWatcher if you want, though.)

+  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.find(expected) != std::string::npos;
+}
+
+bool DnsProbeBrowserTest::PageContains(const std::string& expected) {
+  std::string textContent;

[mmenke, 2013/05/28 18:20:13]

text_content.

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

Done.

+
+  WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();

[mmenke, 2013/05/28 18:20:13]

optional:  Suggest you just inline this, to reduce code length a bit (For
readability).

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

Done.

+
+  bool rv = content::ExecuteScriptAndExtractString(
+      contents,
+      "domAutomationController.send(document.body.textContent);",
+      &textContent);
+  if (!rv)
+    return false;
+
+  return textContent.find(expected) != std::string::npos;
+}
+
+void DnsProbeBrowserTest::SetLinkDoctorNetError(int net_error) {

[mmenke, 2013/05/28 18:20:13]

Instead of this, can we just change the link doctor URL?  I seem to recall that
didn't work for some reason, but just double checking.

[Deprecated (see juliatuttle), 2013/06/05 21:01:47]

We can't change it once the tab is created.  That's why I have to do the weird
thing where we either serve the actual mock link doctor page or a redirect to a
mock error.

+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      Bind(&DnsProbeBrowserTestIOThreadHelper::set_link_doctor_net_error,
+           Unretained(&helper_),
+           net_error));
+}
+
+void DnsProbeBrowserTest::OnProbeStarted() {
+  probe_started_count_++;
+  probe_started_callback_.callback().Run();
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, OtherErrorWithLinkDoctor) {
+  SetLinkDoctorBroken(false);
+
+  NavigateToOtherError();
+  EXPECT_EQ(0, probe_count());
+  EXPECT_TRUE(TitleContains("Mock Link Doctor"));
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, OtherErrorWithoutLinkDoctor) {
+  SetLinkDoctorBroken(true);
+
+  NavigateToOtherError();
+  EXPECT_EQ(0, probe_count());
+  EXPECT_TRUE(PageContains("CONNECTION_REFUSED"));
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, NxdomainWithLinkDoctor) {
+  SetLinkDoctorBroken(false);
+
+  NavigateToDnsError();
+  WaitForProbe();
+  SimulateProbeResult(chrome_common_net::DNS_PROBE_FINISHED_NXDOMAIN);
+  EXPECT_TRUE(TitleContains("Mock Link Doctor"));
+}
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, NoInternetWithoutLinkDoctor) {
+  SetLinkDoctorBroken(true);
+
+  NavigateToDnsError();
+  WaitForProbe();
+  EXPECT_TRUE(PageContains("DNS_PROBE_STARTED"));
+
+  SimulateProbeResult(chrome_common_net::DNS_PROBE_FINISHED_NO_INTERNET);
+  EXPECT_TRUE(PageContains("DNS_PROBE_FINISHED_NO_INTERNET"));
+}
+
+static const FilePath::CharType kIframeDnsErrorHtmlName[] =
+    FILE_PATH_LITERAL("iframe_dns_error.html");
+
+IN_PROC_BROWSER_TEST_F(DnsProbeBrowserTest, NoProbeInSubframe) {
+  SetLinkDoctorBroken(false);
+
+  NavigateToURL(
+      browser(),
+      URLRequestMockHTTPJob::GetMockUrl(FilePath(kIframeDnsErrorHtmlName)));
+  EXPECT_EQ(0, probe_count());
+}
+
+}  // namespace