Fix race condition in ChromeProxyConfigService

The ChromeProxyConfigService was initialized to CONFIG_UNSET (i.e. effectively
a fallback to the system configuration) until the UI thread reported the
preference stored in the PrefService. This created a race condition during
startup where restored tabs could send requests while the
ChromeProxyConfigService was not configured with the values from the
PrefService, yet.

BUG=107326
TEST=no


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

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

8 files changed, 28 insertions, 9 deletions

diff --git a/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc b/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc
index 0eb9462..6a9aa50 100644
--- a/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc
+++ b/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc
@@ -241,7 +241,7 @@ class ProxyConfigServiceImplTestBase : public TESTBASE {
     DBusThreadManager::Initialize();
     PrefProxyConfigTrackerImpl::RegisterPrefs(pref_service);
     ProxyConfigServiceImpl::RegisterPrefs(pref_service);
-    proxy_config_service_.reset(new ChromeProxyConfigService(NULL));
+    proxy_config_service_.reset(new ChromeProxyConfigService(NULL, true));
     config_service_impl_.reset(new ProxyConfigServiceImpl(pref_service));
     config_service_impl_->SetChromeProxyConfigService(
         proxy_config_service_.get());
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 63c7601..59f0c71 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -623,8 +623,9 @@ void IOThread::InitSystemRequestContext() {
   // If we're in unit_tests, IOThread may not be run.
   if (!BrowserThread::IsMessageLoopValid(BrowserThread::IO))
     return;
+  bool wait_for_first_update = (pref_proxy_config_tracker_.get() != NULL);
   ChromeProxyConfigService* proxy_config_service =
-      ProxyServiceFactory::CreateProxyConfigService();
+      ProxyServiceFactory::CreateProxyConfigService(wait_for_first_update);
   system_proxy_config_service_.reset(proxy_config_service);
   if (pref_proxy_config_tracker_.get()) {
     pref_proxy_config_tracker_->SetChromeProxyConfigService(
diff --git a/chrome/browser/net/pref_proxy_config_tracker_impl.cc b/chrome/browser/net/pref_proxy_config_tracker_impl.cc
index 319e9a0..6b4c30f 100644
--- a/chrome/browser/net/pref_proxy_config_tracker_impl.cc
+++ b/chrome/browser/net/pref_proxy_config_tracker_impl.cc
@@ -20,9 +20,11 @@ using content::BrowserThread;
 //============================= ChromeProxyConfigService =======================
 
 ChromeProxyConfigService::ChromeProxyConfigService(
-    net::ProxyConfigService* base_service)
+    net::ProxyConfigService* base_service,
+    bool wait_for_first_update)
     : base_service_(base_service),
       pref_config_state_(ProxyPrefs::CONFIG_UNSET),
+      pref_config_read_pending_(wait_for_first_update),
       registered_observer_(false) {
 }
 
@@ -46,6 +48,9 @@ net::ProxyConfigService::ConfigAvailability
     ChromeProxyConfigService::GetLatestProxyConfig(net::ProxyConfig* config) {
   RegisterObserver();
 
+  if (pref_config_read_pending_)
+    return net::ProxyConfigService::CONFIG_PENDING;
+
   // Ask the base service if available.
   net::ProxyConfig system_config;
   ConfigAvailability system_availability =
@@ -70,6 +75,7 @@ void ChromeProxyConfigService::UpdateProxyConfig(
     const net::ProxyConfig& config) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
+  pref_config_read_pending_ = false;
   pref_config_state_ = config_state;
   pref_config_ = config;
 
diff --git a/chrome/browser/net/pref_proxy_config_tracker_impl.h b/chrome/browser/net/pref_proxy_config_tracker_impl.h
index 9868a5e..72fb9f1 100644
--- a/chrome/browser/net/pref_proxy_config_tracker_impl.h
+++ b/chrome/browser/net/pref_proxy_config_tracker_impl.h
@@ -28,7 +28,10 @@ class ChromeProxyConfigService
       public net::ProxyConfigService::Observer {
  public:
   // Takes ownership of the passed |base_service|.
-  explicit ChromeProxyConfigService(net::ProxyConfigService* base_service);
+  // If |wait_for_first_update| is true, GetLatestProxyConfig returns
+  // ConfigAvailability::CONFIG_PENDING until UpdateProxyConfig has been called.
+  explicit ChromeProxyConfigService(net::ProxyConfigService* base_service,
+                                    bool wait_for_first_update);
   virtual ~ChromeProxyConfigService();
 
   // ProxyConfigService implementation:
@@ -63,6 +66,10 @@ class ChromeProxyConfigService
   // Configuration as defined by prefs.
   net::ProxyConfig pref_config_;
 
+  // Flag that indicates that a PrefProxyConfigTracker needs to inform us
+  // about a proxy configuration before we may return any configuration.
+  bool pref_config_read_pending_;
+
   // Indicates whether the base service registration is done.
   bool registered_observer_;
 
diff --git a/chrome/browser/net/pref_proxy_config_tracker_impl_unittest.cc b/chrome/browser/net/pref_proxy_config_tracker_impl_unittest.cc
index 1a11816..0abd380 100644
--- a/chrome/browser/net/pref_proxy_config_tracker_impl_unittest.cc
+++ b/chrome/browser/net/pref_proxy_config_tracker_impl_unittest.cc
@@ -85,7 +85,7 @@ class PrefProxyConfigTrackerImplTestBase : public TESTBASE {
         new TestProxyConfigService(fixed_config_,
                                    net::ProxyConfigService::CONFIG_VALID);
     proxy_config_service_.reset(
-        new ChromeProxyConfigService(delegate_service_));
+        new ChromeProxyConfigService(delegate_service_, true));
     proxy_config_tracker_.reset(new PrefProxyConfigTrackerImpl(pref_service));
     proxy_config_tracker_->SetChromeProxyConfigService(
         proxy_config_service_.get());
diff --git a/chrome/browser/net/proxy_service_factory.cc b/chrome/browser/net/proxy_service_factory.cc
index dfbaf46..331de6c 100644
--- a/chrome/browser/net/proxy_service_factory.cc
+++ b/chrome/browser/net/proxy_service_factory.cc
@@ -26,7 +26,8 @@
 using content::BrowserThread;
 
 // static
-ChromeProxyConfigService* ProxyServiceFactory::CreateProxyConfigService() {
+ChromeProxyConfigService* ProxyServiceFactory::CreateProxyConfigService(
+    bool wait_for_first_update) {
   // The linux gconf-based proxy settings getter relies on being initialized
   // from the UI thread.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -50,7 +51,7 @@ ChromeProxyConfigService* ProxyServiceFactory::CreateProxyConfigService() {
       BrowserThread::UnsafeGetMessageLoopForThread(BrowserThread::FILE));
 #endif  // !defined(OS_CHROMEOS)
 
-  return new ChromeProxyConfigService(base_service);
+  return new ChromeProxyConfigService(base_service, wait_for_first_update);
 }
 
 #if defined(OS_CHROMEOS)
diff --git a/chrome/browser/net/proxy_service_factory.h b/chrome/browser/net/proxy_service_factory.h
index b060f42..20157c8 100644
--- a/chrome/browser/net/proxy_service_factory.h
+++ b/chrome/browser/net/proxy_service_factory.h
@@ -30,7 +30,11 @@ class ProxyServiceFactory {
  public:
   // Creates a ProxyConfigService that delivers the system preferences
   // (or the respective ChromeOS equivalent).
-  static ChromeProxyConfigService* CreateProxyConfigService();
+  // If |wait_for_first_update| is true, the ChromeProxyConfigService
+  // returns "pending" until it has been informed about the proxy configuration
+  // by calling its UpdateProxyConfig method.
+  static ChromeProxyConfigService* CreateProxyConfigService(
+      bool wait_for_first_update);
 
 #if defined(OS_CHROMEOS)
   static chromeos::ProxyConfigServiceImpl* CreatePrefProxyConfigTracker(
diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc
index 297cee0..0675f7d 100644
--- a/chrome/browser/profiles/profile_io_data.cc
+++ b/chrome/browser/profiles/profile_io_data.cc
@@ -245,7 +245,7 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) {
   params->protocol_handler_registry = profile->GetProtocolHandlerRegistry();
 
   ChromeProxyConfigService* proxy_config_service =
-      ProxyServiceFactory::CreateProxyConfigService();
+      ProxyServiceFactory::CreateProxyConfigService(true);
   params->proxy_config_service.reset(proxy_config_service);
   profile->GetProxyConfigTracker()->SetChromeProxyConfigService(
       proxy_config_service);