From 4060442a0cd7c8521f8d034c9f9e855cb61627e6 Mon Sep 17 00:00:00 2001 From: "ttuttle@chromium.org" Date: Tue, 23 Oct 2012 23:40:43 +0000 Subject: Beginnings of DnsProbeService. BUG=156415 TEST=DnsProbeServiceTest, included TBR=ben@chromium.org Review URL: https://chromiumcodereview.appspot.com/11189025 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@163726 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/net/dns_probe_job.cc | 90 +++++++++-- chrome/browser/net/dns_probe_job.h | 46 ++---- chrome/browser/net/dns_probe_job_unittest.cc | 5 +- chrome/browser/net/dns_probe_service.cc | 170 +++++++++++++++++++++ chrome/browser/net/dns_probe_service.h | 77 ++++++++++ chrome/browser/net/dns_probe_service_unittest.cc | 184 +++++++++++++++++++++++ 6 files changed, 520 insertions(+), 52 deletions(-) create mode 100644 chrome/browser/net/dns_probe_service.cc create mode 100644 chrome/browser/net/dns_probe_service.h create mode 100644 chrome/browser/net/dns_probe_service_unittest.cc (limited to 'chrome/browser/net') diff --git a/chrome/browser/net/dns_probe_job.cc b/chrome/browser/net/dns_probe_job.cc index 0abe6ba..cb2555f 100644 --- a/chrome/browser/net/dns_probe_job.cc +++ b/chrome/browser/net/dns_probe_job.cc @@ -5,18 +5,65 @@ #include "chrome/browser/net/dns_probe_job.h" #include "base/memory/scoped_ptr.h" +#include "base/message_loop.h" #include "net/base/net_errors.h" #include "net/base/net_log.h" #include "net/dns/dns_client.h" #include "net/dns/dns_protocol.h" +#include "net/dns/dns_transaction.h" + +using net::BoundNetLog; +using net::DnsClient; +using net::DnsResponse; +using net::DnsTransaction; +using net::NetLog; namespace chrome_browser_net { -DnsProbeJob::DnsProbeJob(scoped_ptr dns_client, - const CallbackType& callback, - net::NetLog* net_log) +namespace { + +class DnsProbeJobImpl : public DnsProbeJob { + public: + DnsProbeJobImpl(scoped_ptr dns_client, + const DnsProbeJob::CallbackType& callback, + NetLog* net_log); + virtual ~DnsProbeJobImpl(); + + private: + enum QueryStatus { + QUERY_UNKNOWN, + QUERY_CORRECT, + QUERY_INCORRECT, + QUERY_ERROR, + QUERY_RUNNING, + }; + + void MaybeFinishProbe(); + scoped_ptr CreateTransaction( + const std::string& hostname); + void StartTransaction(DnsTransaction* transaction); + void OnTransactionComplete(DnsTransaction* transaction, + int net_error, + const DnsResponse* response); + void RunCallback(DnsProbeJob::Result result); + + BoundNetLog bound_net_log_; + scoped_ptr dns_client_; + const DnsProbeJob::CallbackType callback_; + bool probe_running_; + scoped_ptr good_transaction_; + scoped_ptr bad_transaction_; + QueryStatus good_status_; + QueryStatus bad_status_; + + DISALLOW_COPY_AND_ASSIGN(DnsProbeJobImpl); +}; + +DnsProbeJobImpl::DnsProbeJobImpl(scoped_ptr dns_client, + const DnsProbeJob::CallbackType& callback, + NetLog* net_log) : bound_net_log_( - net::BoundNetLog::Make(net_log, net::NetLog::SOURCE_DNS_PROBER)), + BoundNetLog::Make(net_log, NetLog::SOURCE_DNS_PROBER)), dns_client_(dns_client.Pass()), callback_(callback), probe_running_(false), @@ -42,11 +89,11 @@ DnsProbeJob::DnsProbeJob(scoped_ptr dns_client, StartTransaction(bad_transaction_.get()); } -DnsProbeJob::~DnsProbeJob() { +DnsProbeJobImpl::~DnsProbeJobImpl() { // TODO(ttuttle): Cleanup? (Transactions stop themselves on destruction.) } -void DnsProbeJob::MaybeFinishProbe(void) { +void DnsProbeJobImpl::MaybeFinishProbe(void) { DCHECK(probe_running_); if (good_status_ == QUERY_RUNNING || bad_status_ == QUERY_RUNNING) @@ -63,17 +110,17 @@ void DnsProbeJob::MaybeFinishProbe(void) { // TODO(ttuttle): Log probe finished. } -scoped_ptr DnsProbeJob::CreateTransaction( +scoped_ptr DnsProbeJobImpl::CreateTransaction( const std::string& hostname) { return dns_client_->GetTransactionFactory()->CreateTransaction( hostname, net::dns_protocol::kTypeA, - base::Bind(&DnsProbeJob::OnTransactionComplete, + base::Bind(&DnsProbeJobImpl::OnTransactionComplete, base::Unretained(this)), bound_net_log_); } -void DnsProbeJob::StartTransaction(net::DnsTransaction* transaction) { +void DnsProbeJobImpl::StartTransaction(DnsTransaction* transaction) { int rv = transaction->Start(); if (rv != net::ERR_IO_PENDING) { // TODO(ttuttle): Make sure this counts as unreachable, not error. @@ -83,9 +130,9 @@ void DnsProbeJob::StartTransaction(net::DnsTransaction* transaction) { // TODO(ttuttle): Log transaction started. } -void DnsProbeJob::OnTransactionComplete(net::DnsTransaction* transaction, - int net_error, - const net::DnsResponse* response) { +void DnsProbeJobImpl::OnTransactionComplete(DnsTransaction* transaction, + int net_error, + const DnsResponse* response) { QueryStatus* status; if (transaction == good_transaction_.get()) { @@ -113,8 +160,23 @@ void DnsProbeJob::OnTransactionComplete(net::DnsTransaction* transaction, MaybeFinishProbe(); } -void DnsProbeJob::RunCallback(Result result) { - callback_.Run(this, result); +void DnsProbeJobImpl::RunCallback(DnsProbeJob::Result result) { + // Make sure we're not running the callback in the constructor. + // This avoids a race where our owner tries to destroy us while we're still + // being created, then ends up with a dangling pointer to us. + MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(callback_, base::Unretained(this), result)); +} + +} // namespace + +scoped_ptr DnsProbeJob::CreateJob( + scoped_ptr dns_client, + const CallbackType& callback, + NetLog* net_log) { + return scoped_ptr( + new DnsProbeJobImpl(dns_client.Pass(), callback, net_log)); } } // namespace chrome_browser_net diff --git a/chrome/browser/net/dns_probe_job.h b/chrome/browser/net/dns_probe_job.h index b2412e2..462ec46 100644 --- a/chrome/browser/net/dns_probe_job.h +++ b/chrome/browser/net/dns_probe_job.h @@ -8,13 +8,11 @@ #include "base/basictypes.h" #include "base/bind.h" #include "base/memory/scoped_ptr.h" -#include "net/base/net_log.h" -#include "net/dns/dns_transaction.h" namespace net { class DnsClient; -struct DnsConfig; -}; +class NetLog; +} namespace chrome_browser_net { @@ -31,43 +29,21 @@ class DnsProbeJob { }; typedef base::Callback CallbackType; + virtual ~DnsProbeJob() { } + // Creates and starts a probe job. // - // |dns_client| should be a DnsClient with the DnsConfig alreay set. + // |dns_client| should be a DnsClient with the DnsConfig already set. // |callback| will be called when the probe finishes, which may happen // before the constructor returns (for example, if we can't create the DNS // transactions). - DnsProbeJob(scoped_ptr dns_client, - const CallbackType& callback, - net::NetLog* net_log); - ~DnsProbeJob(); - - private: - enum QueryStatus { - QUERY_UNKNOWN, - QUERY_CORRECT, - QUERY_INCORRECT, - QUERY_ERROR, - QUERY_RUNNING, - }; - - void MaybeFinishProbe(); - scoped_ptr CreateTransaction( - const std::string& hostname); - void StartTransaction(net::DnsTransaction* transaction); - void OnTransactionComplete(net::DnsTransaction* transaction, - int net_error, - const net::DnsResponse* response); - void RunCallback(Result result); + static scoped_ptr CreateJob( + scoped_ptr dns_client, + const CallbackType& callback, + net::NetLog* net_log); - net::BoundNetLog bound_net_log_; - scoped_ptr dns_client_; - const CallbackType callback_; - bool probe_running_; - scoped_ptr good_transaction_; - scoped_ptr bad_transaction_; - QueryStatus good_status_; - QueryStatus bad_status_; + protected: + DnsProbeJob() { } DISALLOW_COPY_AND_ASSIGN(DnsProbeJob); }; diff --git a/chrome/browser/net/dns_probe_job_unittest.cc b/chrome/browser/net/dns_probe_job_unittest.cc index 90297d7..10c2946 100644 --- a/chrome/browser/net/dns_probe_job_unittest.cc +++ b/chrome/browser/net/dns_probe_job_unittest.cc @@ -72,12 +72,11 @@ void DnsProbeJobTest::RunProbe(MockDnsClientRule::Result good_result, scoped_ptr message_loop_(new MessageLoopForIO()); scoped_ptr job( - new DnsProbeJob(dns_client.Pass(), callback, net_log)); + DnsProbeJob::CreateJob(dns_client.Pass(), callback, net_log)); // Force callback to run. base::RunLoop run_loop; - MessageLoop::current()->PostTask(FROM_HERE, run_loop.QuitClosure()); - run_loop.Run(); + run_loop.RunUntilIdle(); } void DnsProbeJobTest::OnProbeFinished(DnsProbeJob* job, diff --git a/chrome/browser/net/dns_probe_service.cc b/chrome/browser/net/dns_probe_service.cc new file mode 100644 index 0000000..6280b0c --- /dev/null +++ b/chrome/browser/net/dns_probe_service.cc @@ -0,0 +1,170 @@ +// Copyright (c) 2012 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 "chrome/browser/net/dns_probe_service.h" + +#include "chrome/browser/net/dns_probe_job.h" +#include "net/base/ip_endpoint.h" +#include "net/base/net_util.h" +#include "net/base/network_change_notifier.h" +#include "net/dns/dns_client.h" +#include "net/dns/dns_config_service.h" +#include "net/dns/dns_protocol.h" + +using net::DnsClient; +using net::DnsConfig; +using net::IPAddressNumber; +using net::IPEndPoint; +using net::ParseIPLiteralToNumber; +using net::NetworkChangeNotifier; + +namespace chrome_browser_net { + +namespace { + +const char* kPublicDnsPrimary = "8.8.8.8"; +const char* kPublicDnsSecondary = "8.8.4.4"; + +IPEndPoint MakeDnsEndPoint(const std::string& dns_ip_literal) { + IPAddressNumber dns_ip_number; + bool rv = ParseIPLiteralToNumber(dns_ip_literal, &dns_ip_number); + DCHECK(rv); + return IPEndPoint(dns_ip_number, net::dns_protocol::kDefaultPort); +} + +} + +DnsProbeService::DnsProbeService() + : system_result_(DnsProbeJob::DNS_UNKNOWN), + public_result_(DnsProbeJob::DNS_UNKNOWN), + state_(STATE_NO_RESULTS), + result_(DNS_PROBE_UNKNOWN) { +} + +DnsProbeService::~DnsProbeService() { +} + +void DnsProbeService::ProbeDns(const DnsProbeService::CallbackType& callback) { + callbacks_.push_back(callback); + + // TODO(ttuttle): Check for cache expiration. + + switch (state_) { + case STATE_NO_RESULTS: + StartProbes(); + break; + case STATE_RESULTS_CACHED: + CallCallbacks(); + break; + case STATE_PROBE_RUNNING: + // do nothing; probe is already running, and will call the callback + break; + } +} + +scoped_ptr DnsProbeService::CreateSystemProbeJob( + const DnsProbeJob::CallbackType& job_callback) { + DnsConfig system_config; + GetSystemDnsConfig(&system_config); + return CreateProbeJob(system_config, job_callback); +} + +scoped_ptr DnsProbeService::CreatePublicProbeJob( + const DnsProbeJob::CallbackType& job_callback) { + DnsConfig public_config; + GetPublicDnsConfig(&public_config); + return CreateProbeJob(public_config, job_callback); +} + +void DnsProbeService::ExpireResults() { + DCHECK_EQ(STATE_RESULTS_CACHED, state_); + + state_ = STATE_NO_RESULTS; + result_ = DNS_PROBE_UNKNOWN; +} + +void DnsProbeService::StartProbes() { + DCHECK_NE(STATE_PROBE_RUNNING, state_); + DCHECK(!system_job_.get()); + DCHECK(!public_job_.get()); + + DnsProbeJob::CallbackType job_callback = + base::Bind(&DnsProbeService::OnProbeJobComplete, + base::Unretained(this)); + + // TODO(ttuttle): Do we want to keep explicit flags for "job done"? + // Or maybe DnsProbeJob should have a "finished" flag? + system_result_ = DnsProbeJob::DNS_UNKNOWN; + public_result_ = DnsProbeJob::DNS_UNKNOWN; + + system_job_ = CreateSystemProbeJob(job_callback); + public_job_ = CreatePublicProbeJob(job_callback); + + state_ = STATE_PROBE_RUNNING; +} + +void DnsProbeService::OnProbesComplete() { + DCHECK_EQ(STATE_PROBE_RUNNING, state_); + + state_ = STATE_RESULTS_CACHED; + // TODO(ttuttle): Calculate result. + result_ = DNS_PROBE_NXDOMAIN; + + CallCallbacks(); +} + +void DnsProbeService::CallCallbacks() { + DCHECK_EQ(STATE_RESULTS_CACHED, state_); + DCHECK(!callbacks_.empty()); + + std::vector callbacks = callbacks_; + callbacks_.clear(); + + for (std::vector::const_iterator i = callbacks.begin(); + i != callbacks.end(); ++i) { + i->Run(result_); + } +} + +scoped_ptr DnsProbeService::CreateProbeJob( + const DnsConfig& dns_config, + const DnsProbeJob::CallbackType& job_callback) { + scoped_ptr dns_client(DnsClient::CreateClient(NULL)); + dns_client->SetConfig(dns_config); + return DnsProbeJob::CreateJob(dns_client.Pass(), job_callback, NULL); +} + +void DnsProbeService::OnProbeJobComplete(DnsProbeJob* job, + DnsProbeJob::Result result) { + DCHECK_EQ(STATE_PROBE_RUNNING, state_); + + if (job == system_job_.get()) { + system_result_ = result; + system_job_.reset(); + } else if (job == public_job_.get()) { + public_result_ = result; + public_job_.reset(); + } else { + NOTREACHED(); + return; + } + + if (system_result_ != DnsProbeJob::DNS_UNKNOWN && + public_result_ != DnsProbeJob::DNS_UNKNOWN) { + OnProbesComplete(); + } +} + +void DnsProbeService::GetSystemDnsConfig(DnsConfig* config) { + // TODO(ttuttle): Make sure we handle missing config properly + NetworkChangeNotifier::GetDnsConfig(config); +} + +void DnsProbeService::GetPublicDnsConfig(DnsConfig* config) { + *config = DnsConfig(); + config->nameservers.push_back(MakeDnsEndPoint(kPublicDnsPrimary)); + config->nameservers.push_back(MakeDnsEndPoint(kPublicDnsSecondary)); +} + +} // namespace chrome_browser_net diff --git a/chrome/browser/net/dns_probe_service.h b/chrome/browser/net/dns_probe_service.h new file mode 100644 index 0000000..dfcc921 --- /dev/null +++ b/chrome/browser/net/dns_probe_service.h @@ -0,0 +1,77 @@ +// Copyright (c) 2012 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_SERVICE_H_ +#define CHROME_BROWSER_NET_DNS_PROBE_SERVICE_H_ + +#include + +#include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" +#include "chrome/browser/net/dns_probe_job.h" + +namespace net { +struct DnsConfig; +} + +namespace chrome_browser_net { + +class DnsProbeService { + public: + enum Result { + DNS_PROBE_UNKNOWN, + DNS_PROBE_NO_INTERNET, + DNS_PROBE_BAD_CONFIG, + DNS_PROBE_NXDOMAIN + }; + typedef base::Callback CallbackType; + + DnsProbeService(); + ~DnsProbeService(); + + void ProbeDns(const CallbackType& callback); + + protected: + // This can be called by tests to pretend the cached reuslt has expired. + void ExpireResults(); + + private: + enum State { + STATE_NO_RESULTS, + STATE_PROBE_RUNNING, + STATE_RESULTS_CACHED, + }; + + void StartProbes(); + void OnProbesComplete(); + void CallCallbacks(); + + void OnProbeJobComplete(DnsProbeJob* job, DnsProbeJob::Result result); + + // These are expected to be overridden by tests to return mock jobs. + virtual scoped_ptr CreateSystemProbeJob( + const DnsProbeJob::CallbackType& job_callback); + virtual scoped_ptr CreatePublicProbeJob( + const DnsProbeJob::CallbackType& job_callback); + + scoped_ptr CreateProbeJob( + const net::DnsConfig& dns_config, + const DnsProbeJob::CallbackType& job_callback); + void GetSystemDnsConfig(net::DnsConfig* config); + void GetPublicDnsConfig(net::DnsConfig* config); + + scoped_ptr system_job_; + scoped_ptr public_job_; + DnsProbeJob::Result system_result_; + DnsProbeJob::Result public_result_; + std::vector callbacks_; + State state_; + Result result_; + + DISALLOW_COPY_AND_ASSIGN(DnsProbeService); +}; + +} // namespace chrome_browser_net + +#endif // CHROME_BROWSER_NET_DNS_PROBE_SERVICE_H_ diff --git a/chrome/browser/net/dns_probe_service_unittest.cc b/chrome/browser/net/dns_probe_service_unittest.cc new file mode 100644 index 0000000..3d8ac03 --- /dev/null +++ b/chrome/browser/net/dns_probe_service_unittest.cc @@ -0,0 +1,184 @@ +// Copyright (c) 2012 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 "chrome/browser/net/dns_probe_service.h" + +#include "base/bind.h" +#include "base/message_loop.h" +#include "base/run_loop.h" +#include "chrome/browser/net/dns_probe_job.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace chrome_browser_net { + +namespace { + +class MockDnsProbeJob : public DnsProbeJob { + public: + MockDnsProbeJob(const CallbackType& callback, + Result result) { + MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(callback, base::Unretained(this), result)); + } + + virtual ~MockDnsProbeJob() { } +}; + +class TestDnsProbeService : public DnsProbeService { + public: + TestDnsProbeService() + : DnsProbeService(), + system_job_created_(false), + public_job_created_(false), + mock_system_result_(DnsProbeJob::DNS_UNKNOWN), + mock_public_result_(DnsProbeJob::DNS_UNKNOWN) { + } + + virtual ~TestDnsProbeService() { } + + void SetMockJobResults( + DnsProbeJob::Result mock_system_result, + DnsProbeJob::Result mock_public_result) { + mock_system_result_ = mock_system_result; + mock_public_result_ = mock_public_result; + } + + void MockExpireResults() { + ExpireResults(); + } + + bool system_job_created_; + bool public_job_created_; + + private: + // Override methods in DnsProbeService to return mock jobs: + + virtual scoped_ptr CreateSystemProbeJob( + const DnsProbeJob::CallbackType& job_callback) OVERRIDE { + system_job_created_ = true; + return scoped_ptr( + new MockDnsProbeJob(job_callback, + mock_system_result_)); + } + + virtual scoped_ptr CreatePublicProbeJob( + const DnsProbeJob::CallbackType& job_callback) OVERRIDE { + public_job_created_ = true; + return scoped_ptr( + new MockDnsProbeJob(job_callback, + mock_public_result_)); + } + + DnsProbeJob::Result mock_system_result_; + DnsProbeJob::Result mock_public_result_; +}; + +class DnsProbeServiceTest : public testing::Test { + public: + DnsProbeServiceTest() + : callback_called_(false), + callback_result_(DnsProbeService::DNS_PROBE_UNKNOWN) { + } + + void SetMockJobResults(DnsProbeJob::Result mock_system_result, + DnsProbeJob::Result mock_public_result) { + service_.SetMockJobResults(mock_system_result, mock_public_result); + } + + void Probe() { + service_.ProbeDns(base::Bind(&DnsProbeServiceTest::ProbeCallback, + base::Unretained(this))); + } + + void RunUntilIdle() { + base::RunLoop run_loop; + run_loop.RunUntilIdle(); + } + + void ExpireResults() { + service_.MockExpireResults(); + } + + MessageLoopForIO message_loop_; + TestDnsProbeService service_; + bool callback_called_; + DnsProbeService::Result callback_result_; + + private: + void ProbeCallback(DnsProbeService::Result result) { + callback_called_ = true; + callback_result_ = result; + } +}; + +TEST_F(DnsProbeServiceTest, Null) { +} + +TEST_F(DnsProbeServiceTest, Probe) { + SetMockJobResults(DnsProbeJob::DNS_WORKING, DnsProbeJob::DNS_WORKING); + + Probe(); + EXPECT_TRUE(service_.system_job_created_); + EXPECT_TRUE(service_.public_job_created_); + EXPECT_FALSE(callback_called_); + + RunUntilIdle(); + EXPECT_TRUE(callback_called_); + EXPECT_EQ(DnsProbeService::DNS_PROBE_NXDOMAIN, callback_result_); +} + +TEST_F(DnsProbeServiceTest, Cache) { + SetMockJobResults(DnsProbeJob::DNS_WORKING, DnsProbeJob::DNS_WORKING); + + Probe(); + EXPECT_TRUE(service_.system_job_created_); + EXPECT_TRUE(service_.public_job_created_); + + RunUntilIdle(); + EXPECT_TRUE(callback_called_); + EXPECT_EQ(DnsProbeService::DNS_PROBE_NXDOMAIN, callback_result_); + + callback_called_ = false; + service_.system_job_created_ = false; + service_.public_job_created_ = false; + + Probe(); + EXPECT_FALSE(service_.system_job_created_); + EXPECT_FALSE(service_.public_job_created_); + + RunUntilIdle(); + EXPECT_TRUE(callback_called_); + EXPECT_EQ(DnsProbeService::DNS_PROBE_NXDOMAIN, callback_result_); +} + +TEST_F(DnsProbeServiceTest, Expired) { + SetMockJobResults(DnsProbeJob::DNS_WORKING, DnsProbeJob::DNS_WORKING); + + Probe(); + EXPECT_TRUE(service_.system_job_created_); + EXPECT_TRUE(service_.public_job_created_); + + RunUntilIdle(); + EXPECT_TRUE(callback_called_); + EXPECT_EQ(DnsProbeService::DNS_PROBE_NXDOMAIN, callback_result_); + + callback_called_ = false; + service_.system_job_created_ = false; + service_.public_job_created_ = false; + + ExpireResults(); + + Probe(); + EXPECT_TRUE(service_.system_job_created_); + EXPECT_TRUE(service_.public_job_created_); + + RunUntilIdle(); + EXPECT_TRUE(callback_called_); + EXPECT_EQ(DnsProbeService::DNS_PROBE_NXDOMAIN, callback_result_); +} + +} // namespace + +} // namespace chrome_browser_net -- cgit v1.1