Fixed minor atomicity issue in PrefProxyConfigService::GetLatestProxyConfig.

chrome/browser/net/pref_proxy_config_service.cc

 // Copyright (c) 2010 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/pref_proxy_config_service.h"
 
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/pref_set_observer.h"
 #include "chrome/common/notification_details.h"
@@ skipping 20 matching lines @@
 }
 
 void PrefProxyConfigTracker::DetachFromPrefService() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // Stop notifications.
   proxy_prefs_observer_.reset();
   pref_service_ = NULL;
 }
 
 void PrefProxyConfigTracker::AddObserver(
-    PrefProxyConfigTracker::ObserverInterface* observer) {
+    PrefProxyConfigTracker::Observer* observer) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   observers_.AddObserver(observer);
 }
 
 void PrefProxyConfigTracker::RemoveObserver(
-    PrefProxyConfigTracker::ObserverInterface* observer) {
+    PrefProxyConfigTracker::Observer* observer) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   observers_.RemoveObserver(observer);
 }
 
 void PrefProxyConfigTracker::Observe(NotificationType type,
                                      const NotificationSource& source,
                                      const NotificationDetails& details) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (type == NotificationType::PREF_CHANGED &&
       Source<PrefService>(source).ptr() == pref_service_) {
     net::ProxyConfig new_config;
     bool valid = ReadPrefConfig(&new_config);
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         NewRunnableMethod(this,
                           &PrefProxyConfigTracker::InstallProxyConfig,
                           new_config, valid));
   } else {
     NOTREACHED() << "Unexpected notification of type " << type.value;
   }
 }
 
 void PrefProxyConfigTracker::InstallProxyConfig(const net::ProxyConfig& config,
                                                 bool valid) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (valid_ != valid || !pref_config_.Equals(config)) {
+  if (valid && (!valid_ || !pref_config_.Equals(config))) {

[eroman, 2010/12/02 19:31:22]

I don't think this is right, since it looks like we will never be able to return
to "use system settings (valid=false)".

I think what you want here instead is something like:

if (valid_ != valid || (valid && pref_config_.Equals(config))

Basically the only restriction we add is to avoid comparing config to
pref_config_ when !valid.

[eroman, 2010/12/02 19:32:40]

Oops my pseudo-code wasn't right either :-)
Let me try again:

if (valid_ != valid || (valid && !pref_config_.Equals(config))

[battre (please use the other), 2010/12/03 08:57:23]

Done.

     valid_ = valid;
     if (valid_)
       pref_config_ = config;
-    FOR_EACH_OBSERVER(ObserverInterface, observers_,
-                      OnPrefProxyConfigChanged());
+    FOR_EACH_OBSERVER(Observer, observers_, OnPrefProxyConfigChanged());
   }
 }
 
 bool PrefProxyConfigTracker::ReadPrefConfig(net::ProxyConfig* config) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Clear the configuration.
   *config = net::ProxyConfig();
 
   // Scan for all "enable" type proxy switches.
@@ skipping 73 matching lines @@
   observers_.AddObserver(observer);
 }
 
 void PrefProxyConfigService::RemoveObserver(
     net::ProxyConfigService::Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
 bool PrefProxyConfigService::GetLatestProxyConfig(net::ProxyConfig* config) {
   RegisterObservers();
-  const net::ProxyConfig pref_config;
-  if (pref_config_tracker_->GetProxyConfig(config))
+  net::ProxyConfig pref_config;
+  if (pref_config_tracker_->GetProxyConfig(&pref_config)) {
+    *config = pref_config;
     return true;
+  }
 
   return base_service_->GetLatestProxyConfig(config);
 }
 
 void PrefProxyConfigService::OnLazyPoll() {
   base_service_->OnLazyPoll();
 }
 
 void PrefProxyConfigService::OnProxyConfigChanged(
     const net::ProxyConfig& config) {
@@ skipping 28 matching lines @@
 }
 
 void PrefProxyConfigService::RegisterObservers() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (!registered_observers_) {
     base_service_->AddObserver(this);
     pref_config_tracker_->AddObserver(this);
     registered_observers_ = true;
   }
 }