Third patch in getting rid of caching MessageLoop pointers and always using ChromeThread instead.

BUG=25354

Review URL: http://codereview.chromium.org/342068

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30687 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 51 insertions, 57 deletions

diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 7730127..dec67cb 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -118,6 +118,9 @@ static net::ProxyConfigService* CreateProxyConfigService(
 
   if (!proxy_config_from_cmd_line.get()) {
     // Use system settings.
+    // TODO(port): the IO and FILE message loops are only used by Linux.  Can
+    // that code be moved to chrome/browser instead of being in net, so that it
+    // can use ChromeThread instead of raw MessageLoop pointers? See bug 25354.
     return net::ProxyService::CreateSystemProxyConfigService(
         g_browser_process->io_thread()->message_loop(),
         g_browser_process->file_thread()->message_loop());
@@ -667,7 +670,8 @@ void ChromeURLRequestContextGetter::Observe(NotificationType type,
     if (*pref_name_in == prefs::kAcceptLanguages) {
       std::string accept_language =
           WideToASCII(prefs->GetString(prefs::kAcceptLanguages));
-      g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE,
+      ChromeThread::PostTask(
+          ChromeThread::IO, FROM_HERE,
           NewRunnableMethod(
               this,
               &ChromeURLRequestContextGetter::OnAcceptLanguageChange,
@@ -675,7 +679,8 @@ void ChromeURLRequestContextGetter::Observe(NotificationType type,
     } else if (*pref_name_in == prefs::kCookieBehavior) {
       net::CookiePolicy::Type policy_type = net::CookiePolicy::FromInt(
           prefs_->GetInteger(prefs::kCookieBehavior));
-      g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE,
+      ChromeThread::PostTask(
+          ChromeThread::IO, FROM_HERE,
           NewRunnableMethod(
               this,
               &ChromeURLRequestContextGetter::OnCookiePolicyChange,
@@ -683,7 +688,8 @@ void ChromeURLRequestContextGetter::Observe(NotificationType type,
     } else if (*pref_name_in == prefs::kDefaultCharset) {
       std::string default_charset =
           WideToASCII(prefs->GetString(prefs::kDefaultCharset));
-      g_browser_process->io_thread()->message_loop()->PostTask(FROM_HERE,
+      ChromeThread::PostTask(
+          ChromeThread::IO, FROM_HERE,
           NewRunnableMethod(
               this,
               &ChromeURLRequestContextGetter::OnDefaultCharsetChange,
@@ -869,8 +875,8 @@ net::CookieStore* ChromeURLRequestContextGetter::GetCookieStore() {
   base::WaitableEvent completion(false, false);
   net::CookieStore* result = NULL;
 
-  g_browser_process->io_thread()->message_loop()->PostTask(
-      FROM_HERE,
+  ChromeThread::PostTask(
+      ChromeThread::IO, FROM_HERE,
       NewRunnableMethod(this,
           &ChromeURLRequestContextGetter::GetCookieStoreAsyncHelper,
           &completion,
diff --git a/chrome/browser/net/dns_global.cc b/chrome/browser/net/dns_global.cc
index 7813f77..d70a3a0 100644
--- a/chrome/browser/net/dns_global.cc
+++ b/chrome/browser/net/dns_global.cc
@@ -13,7 +13,6 @@
 #include "base/thread.h"
 #include "base/values.h"
 #include "chrome/browser/browser.h"
-#include "chrome/browser/browser_process.h"
 #include "chrome/browser/net/dns_host_info.h"
 #include "chrome/browser/net/referrer.h"
 #include "chrome/browser/profile.h"
@@ -422,7 +421,6 @@ void InitDnsPrefetch(TimeDelta max_queue_delay, size_t max_concurrent,
     // Have the DnsMaster issue resolve requests through a global HostResolver
     // that is shared by the main URLRequestContext, and lives on the IO thread.
     dns_master = new DnsMaster(GetGlobalHostResolver(),
-                               g_browser_process->io_thread()->message_loop(),
                                max_queue_delay, max_concurrent);
     dns_master->AddRef();
     // We did the initialization, so we should prime the pump, and set up
diff --git a/chrome/browser/net/dns_master.cc b/chrome/browser/net/dns_master.cc
index b30793f..91e2d1f 100644
--- a/chrome/browser/net/dns_master.cc
+++ b/chrome/browser/net/dns_master.cc
@@ -10,11 +10,11 @@
 
 #include "base/compiler_specific.h"
 #include "base/histogram.h"
-#include "base/message_loop.h"
 #include "base/lock.h"
 #include "base/stats_counters.h"
 #include "base/string_util.h"
 #include "base/time.h"
+#include "chrome/browser/chrome_thread.h"
 #include "net/base/address_list.h"
 #include "net/base/completion_callback.h"
 #include "net/base/host_resolver.h"
@@ -70,15 +70,13 @@ class DnsMaster::LookupRequest {
 };
 
 DnsMaster::DnsMaster(net::HostResolver* host_resolver,
-                     MessageLoop* host_resolver_loop,
                      TimeDelta max_queue_delay,
                      size_t max_concurrent)
   : peak_pending_lookups_(0),
     shutdown_(false),
     max_concurrent_lookups_(max_concurrent),
     max_queue_delay_(max_queue_delay),
-    host_resolver_(host_resolver),
-    host_resolver_loop_(host_resolver_loop) {
+    host_resolver_(host_resolver) {
 }
 
 DnsMaster::~DnsMaster() {
@@ -101,11 +99,13 @@ void DnsMaster::ResolveList(const NameList& hostnames,
                             DnsHostInfo::ResolutionMotivation motivation) {
   AutoLock auto_lock(lock_);
 
-  // We need to run this on |host_resolver_loop_| since we may access
+  // We need to run this on the IO thread since we may access
   // |host_resolver_| which is not thread safe.
-  if (MessageLoop::current() != host_resolver_loop_) {
-    host_resolver_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
-        &DnsMaster::ResolveList, hostnames, motivation));
+  if (!ChromeThread::CurrentlyOn(ChromeThread::IO)) {
+    ChromeThread::PostTask(
+        ChromeThread::IO, FROM_HERE,
+        NewRunnableMethod(
+            this, &DnsMaster::ResolveList, hostnames, motivation));
     return;
   }
 
@@ -122,11 +122,12 @@ void DnsMaster::Resolve(const std::string& hostname,
     return;
   AutoLock auto_lock(lock_);
 
-  // We need to run this on |host_resolver_loop_| since we may access
+  // We need to run this on the IO thread since we may access
   // |host_resolver_| which is not thread safe.
-  if (MessageLoop::current() != host_resolver_loop_) {
-    host_resolver_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
-        &DnsMaster::Resolve, hostname, motivation));
+  if (!ChromeThread::CurrentlyOn(ChromeThread::IO)) {
+    ChromeThread::PostTask(
+        ChromeThread::IO, FROM_HERE,
+        NewRunnableMethod(this, &DnsMaster::Resolve, hostname, motivation));
     return;
   }
 
@@ -205,11 +206,12 @@ void DnsMaster::NonlinkNavigation(const GURL& referrer,
 void DnsMaster::NavigatingTo(const std::string& host_name) {
   AutoLock auto_lock(lock_);
 
-  // We need to run this on |host_resolver_loop_| since we may access
+  // We need to run this on the IO thread since we may access
   // |host_resolver_| which is not thread safe.
-  if (MessageLoop::current() != host_resolver_loop_) {
-    host_resolver_loop_->PostTask(FROM_HERE, NewRunnableMethod(this,
-        &DnsMaster::NavigatingTo, host_name));
+  if (!ChromeThread::CurrentlyOn(ChromeThread::IO)) {
+    ChromeThread::PostTask(
+        ChromeThread::IO, FROM_HERE,
+        NewRunnableMethod(this, &DnsMaster::NavigatingTo, host_name));
     return;
   }
 
@@ -414,9 +416,9 @@ DnsHostInfo* DnsMaster::PreLockedResolve(
 }
 
 void DnsMaster::PreLockedScheduleLookups() {
-  // We need to run this on |host_resolver_loop_| since we may access
-  // |host_resolver_| which is not thread safe.
-  DCHECK_EQ(MessageLoop::current(), host_resolver_loop_);
+  // We need to run this on the IO thread since we may access |host_resolver_|
+  // which is not thread safe.
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
 
   while (!work_queue_.IsEmpty() &&
          pending_lookups_.size() < max_concurrent_lookups_) {
@@ -466,7 +468,7 @@ bool DnsMaster::PreLockedCongestionControlPerformed(DnsHostInfo* info) {
 
 void DnsMaster::OnLookupFinished(LookupRequest* request,
                                  const std::string& hostname, bool found) {
-  DCHECK_EQ(MessageLoop::current(), host_resolver_loop_);
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
 
   AutoLock auto_lock(lock_);  // For map access (changing info values).
   PrelockedLookupFinished(request, hostname, found);
diff --git a/chrome/browser/net/dns_master.h b/chrome/browser/net/dns_master.h
index 1bf87d0..451e9ed 100644
--- a/chrome/browser/net/dns_master.h
+++ b/chrome/browser/net/dns_master.h
@@ -32,8 +32,6 @@ namespace net {
 class HostResolver;
 }
 
-class MessageLoop;
-
 namespace chrome_browser_net {
 
 typedef chrome_common_net::NameList NameList;
@@ -42,9 +40,9 @@ typedef std::map<std::string, DnsHostInfo> Results;
 class DnsMaster : public base::RefCountedThreadSafe<DnsMaster> {
  public:
   // |max_concurrent| specifies how many concurrent (paralell) prefetches will
-  // be performed. Host lookups will be issued on the |host_resolver_loop|
-  // thread, using the |host_resolver| instance.
-  DnsMaster(net::HostResolver* host_resolver, MessageLoop* host_resolver_loop,
+  // be performed. Host lookups will be issued on the IO thread, using the
+  // |host_resolver| instance.
+  DnsMaster(net::HostResolver* host_resolver,
             TimeDelta max_queue_delay_ms, size_t max_concurrent);
   ~DnsMaster();
 
@@ -236,9 +234,8 @@ class DnsMaster : public base::RefCountedThreadSafe<DnsMaster> {
   const TimeDelta max_queue_delay_;
 
   // The host resovler we warm DNS entries for. The resolver (which is not
-  // thread safe) should be accessed only on |host_resolver_loop_|.
+  // thread safe) should be accessed only on the IO thread.
   scoped_refptr<net::HostResolver> host_resolver_;
-  MessageLoop* host_resolver_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(DnsMaster);
 };
diff --git a/chrome/browser/net/dns_master_unittest.cc b/chrome/browser/net/dns_master_unittest.cc
index fc8b796d..249956d 100644
--- a/chrome/browser/net/dns_master_unittest.cc
+++ b/chrome/browser/net/dns_master_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
 #include "base/timer.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/net/dns_global.h"
 #include "chrome/browser/net/dns_host_info.h"
 #include "chrome/common/net/dns.h"
@@ -59,7 +60,8 @@ class WaitForResolutionHelper {
 class DnsMasterTest : public testing::Test {
  public:
   DnsMasterTest()
-      : host_resolver_(new net::MockCachingHostResolver()),
+      : io_thread_(ChromeThread::IO, &loop_),
+        host_resolver_(new net::MockCachingHostResolver()),
         default_max_queueing_delay_(TimeDelta::FromMilliseconds(
             DnsPrefetcherInit::kMaxQueueingDelayMs)) {
   }
@@ -91,7 +93,8 @@ class DnsMasterTest : public testing::Test {
   // IMPORTANT: do not move this below |host_resolver_|; the host resolver
   // must not outlive the message loop, otherwise bad things can happen
   // (like posting to a deleted message loop).
-  MessageLoop loop;
+  MessageLoop loop_;
+  ChromeThread io_thread_;
 
  protected:
   scoped_refptr<net::MockCachingHostResolver> host_resolver_;
@@ -105,15 +108,13 @@ class DnsMasterTest : public testing::Test {
 
 TEST_F(DnsMasterTest, StartupShutdownTest) {
   scoped_refptr<DnsMaster> testing_master = new DnsMaster(host_resolver_,
-      MessageLoop::current(), default_max_queueing_delay_,
-      DnsPrefetcherInit::kMaxConcurrentLookups);
+      default_max_queueing_delay_, DnsPrefetcherInit::kMaxConcurrentLookups);
   testing_master->Shutdown();
 }
 
 TEST_F(DnsMasterTest, BenefitLookupTest) {
   scoped_refptr<DnsMaster> testing_master = new DnsMaster(host_resolver_,
-      MessageLoop::current(), default_max_queueing_delay_,
-      DnsPrefetcherInit::kMaxConcurrentLookups);
+      default_max_queueing_delay_, DnsPrefetcherInit::kMaxConcurrentLookups);
 
   std::string goog("www.google.com"),
     goog2("gmail.google.com.com"),
@@ -177,8 +178,7 @@ TEST_F(DnsMasterTest, ShutdownWhenResolutionIsPendingTest) {
   host_resolver_->Reset(resolver_proc);
 
   scoped_refptr<DnsMaster> testing_master = new DnsMaster(host_resolver_,
-      MessageLoop::current(), default_max_queueing_delay_,
-      DnsPrefetcherInit::kMaxConcurrentLookups);
+      default_max_queueing_delay_, DnsPrefetcherInit::kMaxConcurrentLookups);
 
   std::string localhost("127.0.0.1");
   NameList names;
@@ -201,8 +201,7 @@ TEST_F(DnsMasterTest, ShutdownWhenResolutionIsPendingTest) {
 
 TEST_F(DnsMasterTest, SingleLookupTest) {
   scoped_refptr<DnsMaster> testing_master = new DnsMaster(host_resolver_,
-      MessageLoop::current(), default_max_queueing_delay_,
-      DnsPrefetcherInit::kMaxConcurrentLookups);
+      default_max_queueing_delay_, DnsPrefetcherInit::kMaxConcurrentLookups);
 
   std::string goog("www.google.com");
 
@@ -231,8 +230,7 @@ TEST_F(DnsMasterTest, ConcurrentLookupTest) {
   host_resolver_->rules()->AddSimulatedFailure("*.notfound");
 
   scoped_refptr<DnsMaster> testing_master = new DnsMaster(host_resolver_,
-      MessageLoop::current(), default_max_queueing_delay_,
-      DnsPrefetcherInit::kMaxConcurrentLookups);
+      default_max_queueing_delay_, DnsPrefetcherInit::kMaxConcurrentLookups);
 
   std::string goog("www.google.com"),
     goog2("gmail.google.com.com"),
@@ -280,8 +278,7 @@ TEST_F(DnsMasterTest, MassiveConcurrentLookupTest) {
   host_resolver_->rules()->AddSimulatedFailure("*.notfound");
 
   scoped_refptr<DnsMaster> testing_master = new DnsMaster(host_resolver_,
-      MessageLoop::current(), default_max_queueing_delay_,
-      DnsPrefetcherInit::kMaxConcurrentLookups);
+      default_max_queueing_delay_, DnsPrefetcherInit::kMaxConcurrentLookups);
 
   NameList names;
   for (int i = 0; i < 100; i++)
@@ -385,8 +382,7 @@ int GetLatencyFromSerialization(const std::string& motivation,
 // Make sure nil referral lists really have no entries, and no latency listed.
 TEST_F(DnsMasterTest, ReferrerSerializationNilTest) {
   scoped_refptr<DnsMaster> master = new DnsMaster(host_resolver_,
-      MessageLoop::current(), default_max_queueing_delay_,
-      DnsPrefetcherInit::kMaxConcurrentLookups);
+      default_max_queueing_delay_, DnsPrefetcherInit::kMaxConcurrentLookups);
   ListValue referral_list;
   master->SerializeReferrers(&referral_list);
   EXPECT_EQ(0U, referral_list.GetSize());
@@ -401,8 +397,7 @@ TEST_F(DnsMasterTest, ReferrerSerializationNilTest) {
 // serialization without being changed.
 TEST_F(DnsMasterTest, ReferrerSerializationSingleReferrerTest) {
   scoped_refptr<DnsMaster> master = new DnsMaster(host_resolver_,
-      MessageLoop::current(), default_max_queueing_delay_,
-      DnsPrefetcherInit::kMaxConcurrentLookups);
+      default_max_queueing_delay_, DnsPrefetcherInit::kMaxConcurrentLookups);
   std::string motivation_hostname = "www.google.com";
   std::string subresource_hostname = "icons.google.com";
   const int kLatency = 3;
@@ -426,8 +421,7 @@ TEST_F(DnsMasterTest, ReferrerSerializationSingleReferrerTest) {
 // Make sure the Trim() functionality works as expected.
 TEST_F(DnsMasterTest, ReferrerSerializationTrimTest) {
   scoped_refptr<DnsMaster> master = new DnsMaster(host_resolver_,
-      MessageLoop::current(), default_max_queueing_delay_,
-      DnsPrefetcherInit::kMaxConcurrentLookups);
+      default_max_queueing_delay_, DnsPrefetcherInit::kMaxConcurrentLookups);
   std::string motivation_hostname = "www.google.com";
   std::string icon_subresource_hostname = "icons.google.com";
   std::string img_subresource_hostname = "img.google.com";
@@ -556,7 +550,4 @@ TEST_F(DnsMasterTest, PriorityQueueReorderTest) {
   EXPECT_TRUE(queue.IsEmpty());
 }
 
-
-
-
 }  // namespace chrome_browser_net