Watch the Internet Explorer registry keys so we can detect proxy settings changes faster.

Previously we would poll on a background thread every 5 seconds (triggered by network activity) to check for changes. This meant there was a 5 second window during which new requests might still be using the old settings.

With this CL, the settings are noticed almost immediately and the continuous polling is de-emphasized.

(We still poll, but less often. This is a precaution since we are relying on implementation details of a winhttp function, and can't be sure it won't change in the future, or is even completely accurate right now).

BUG=52120
TEST=Open chrome://net-internals/#requests. Request some webpage. Now make changes to the Internet explorer proxy settinsg. You should see a PROXY_CONFIG_CHANGED event emitted immediately after saving each change in the dialog box.

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

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

4 files changed, 191 insertions, 18 deletions

diff --git a/net/proxy/polling_proxy_config_service.cc b/net/proxy/polling_proxy_config_service.cc
index ac42d9c..2db9792 100644
--- a/net/proxy/polling_proxy_config_service.cc
+++ b/net/proxy/polling_proxy_config_service.cc
@@ -22,10 +22,11 @@ class PollingProxyConfigService::Core
   Core(base::TimeDelta poll_interval,
        GetConfigFunction get_config_func)
       : get_config_func_(get_config_func),
+        poll_interval_(poll_interval),
+        have_initialized_origin_loop_(false),
         has_config_(false),
         poll_task_outstanding_(false),
-        poll_interval_(poll_interval),
-        have_initialized_origin_loop_(false) {
+        poll_task_queued_(false) {
   }
 
   // Called when the parent PollingProxyConfigService is destroyed
@@ -66,20 +67,31 @@ class PollingProxyConfigService::Core
     LazyInitializeOriginLoop();
     DCHECK(origin_loop_proxy_->BelongsToCurrentThread());
 
-    if (poll_task_outstanding_)
-      return;  // Still waiting for earlier test to finish.
+    if (last_poll_time_.is_null() ||
+        (base::TimeTicks::Now() - last_poll_time_) > poll_interval_) {
+      CheckForChangesNow();
+    }
+  }
 
-    base::TimeTicks now = base::TimeTicks::Now();
+  void CheckForChangesNow() {
+    LazyInitializeOriginLoop();
+    DCHECK(origin_loop_proxy_->BelongsToCurrentThread());
 
-    if (last_poll_time_.is_null() ||
-        (now - last_poll_time_) > poll_interval_) {
-      last_poll_time_ = now;
-      poll_task_outstanding_ = true;
-      WorkerPool::PostTask(
-          FROM_HERE,
-          NewRunnableMethod(
-              this, &Core::PollOnWorkerThread, get_config_func_), true);
+    if (poll_task_outstanding_) {
+      // Only allow one task to be outstanding at a time. If we get a poll
+      // request while we are busy, we will defer it until the current poll
+      // completes.
+      poll_task_queued_ = true;
+      return;
     }
+
+    last_poll_time_ = base::TimeTicks::Now();
+    poll_task_outstanding_ = true;
+    poll_task_queued_ = false;
+    WorkerPool::PostTask(
+        FROM_HERE,
+        NewRunnableMethod(
+            this, &Core::PollOnWorkerThread, get_config_func_), true);
   }
 
  private:
@@ -111,6 +123,9 @@ class PollingProxyConfigService::Core
       last_config_ = config;
       FOR_EACH_OBSERVER(Observer, observers_, OnProxyConfigChanged(config));
     }
+
+    if (poll_task_queued_)
+      CheckForChangesNow();
   }
 
   void LazyInitializeOriginLoop() {
@@ -126,15 +141,17 @@ class PollingProxyConfigService::Core
 
   GetConfigFunction get_config_func_;
   ObserverList<Observer> observers_;
-  bool has_config_;
-  bool poll_task_outstanding_;
   ProxyConfig last_config_;
   base::TimeTicks last_poll_time_;
   base::TimeDelta poll_interval_;
-  bool have_initialized_origin_loop_;
 
   Lock lock_;
   scoped_refptr<base::MessageLoopProxy> origin_loop_proxy_;
+
+  bool have_initialized_origin_loop_;
+  bool has_config_;
+  bool poll_task_outstanding_;
+  bool poll_task_queued_;
 };
 
 PollingProxyConfigService::PollingProxyConfigService(
@@ -163,4 +180,8 @@ void PollingProxyConfigService::OnLazyPoll() {
   core_->OnLazyPoll();
 }
 
+void PollingProxyConfigService::CheckForChangesNow() {
+  core_->CheckForChangesNow();
+}
+
 }  // namespace net
diff --git a/net/proxy/polling_proxy_config_service.h b/net/proxy/polling_proxy_config_service.h
index 455359a..b4295ec 100644
--- a/net/proxy/polling_proxy_config_service.h
+++ b/net/proxy/polling_proxy_config_service.h
@@ -40,6 +40,9 @@ class PollingProxyConfigService : public ProxyConfigService {
 
   virtual ~PollingProxyConfigService();
 
+  // Polls for changes by posting a task to the worker pool.
+  void CheckForChangesNow();
+
  private:
   class Core;
   scoped_refptr<Core> core_;
diff --git a/net/proxy/proxy_config_service_win.cc b/net/proxy/proxy_config_service_win.cc
index 403d149..1cd8a8c 100644
--- a/net/proxy/proxy_config_service_win.cc
+++ b/net/proxy/proxy_config_service_win.cc
@@ -8,8 +8,11 @@
 #include <winhttp.h>
 
 #include "base/logging.h"
+#include "base/registry.h"
+#include "base/scoped_ptr.h"
 #include "base/string_tokenizer.h"
 #include "base/string_util.h"
+#include "base/stl_util-inl.h"
 #include "net/base/net_errors.h"
 #include "net/proxy/proxy_config.h"
 
@@ -19,7 +22,7 @@ namespace net {
 
 namespace {
 
-const int kPollIntervalSec = 5;
+const int kPollIntervalSec = 10;
 
 void FreeIEConfig(WINHTTP_CURRENT_USER_IE_PROXY_CONFIG* ie_config) {
   if (ie_config->lpszAutoConfigUrl)
@@ -32,12 +35,115 @@ void FreeIEConfig(WINHTTP_CURRENT_USER_IE_PROXY_CONFIG* ie_config) {
 
 }  // namespace
 
+// RegKey and ObjectWatcher pair.
+class ProxyConfigServiceWin::KeyEntry {
+ public:
+  bool StartWatching(base::ObjectWatcher::Delegate* delegate) {
+    // Try to create a watch event for the registry key (which watches the
+    // sibling tree as well).
+    if (!key_.StartWatching())
+      return false;
+
+    // Now setup an ObjectWatcher for this event, so we get OnObjectSignaled()
+    // invoked on this message loop once it is signalled.
+    if (!watcher_.StartWatching(key_.watch_event(), delegate))
+      return false;
+
+    return true;
+  }
+
+  bool CreateRegKey(HKEY rootkey, const wchar_t* subkey) {
+    return key_.Create(rootkey, subkey, KEY_NOTIFY);
+  }
+
+  HANDLE watch_event() const {
+    return key_.watch_event();
+  }
+
+ private:
+  RegKey key_;
+  base::ObjectWatcher watcher_;
+};
+
 ProxyConfigServiceWin::ProxyConfigServiceWin()
     : PollingProxyConfigService(
           base::TimeDelta::FromSeconds(kPollIntervalSec),
           &ProxyConfigServiceWin::GetCurrentProxyConfig) {
 }
 
+ProxyConfigServiceWin::~ProxyConfigServiceWin() {
+  STLDeleteElements(&keys_to_watch_);
+}
+
+void ProxyConfigServiceWin::AddObserver(Observer* observer) {
+  // Lazily-initialize our registry watcher.
+  StartWatchingRegistryForChanges();
+
+  // Let the super-class do its work now.
+  PollingProxyConfigService::AddObserver(observer);
+}
+
+void ProxyConfigServiceWin::StartWatchingRegistryForChanges() {
+  if (!keys_to_watch_.empty())
+    return;  // Already initialized.
+
+  // There are a number of different places where proxy settings can live
+  // in the registry. In some cases it appears in a binary value, in other
+  // cases string values. Furthermore winhttp and wininet appear to have
+  // separate stores, and proxy settings can be configured per-machine
+  // or per-user.
+  //
+  // This function is probably not exhaustive in the registry locations it
+  // watches for changes, however it should catch the majority of the
+  // cases. In case we have missed some less common triggers (likely), we
+  // will catch them during the periodic (10 second) polling, so things
+  // will recover.
+
+  AddKeyToWatchList(
+      HKEY_CURRENT_USER,
+      L"Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings");
+
+  AddKeyToWatchList(
+      HKEY_LOCAL_MACHINE,
+      L"Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings");
+
+  AddKeyToWatchList(
+      HKEY_LOCAL_MACHINE,
+      L"SOFTWARE\\Policies\\Microsoft\\Windows\\CurrentVersion\\"
+      L"Internet Settings");
+}
+
+bool ProxyConfigServiceWin::AddKeyToWatchList(HKEY rootkey,
+                                              const wchar_t* subkey) {
+  scoped_ptr<KeyEntry> entry(new KeyEntry);
+  if (!entry->CreateRegKey(rootkey, subkey))
+    return false;
+
+  if (!entry->StartWatching(this))
+    return false;
+
+  keys_to_watch_.push_back(entry.release());
+  return true;
+}
+
+void ProxyConfigServiceWin::OnObjectSignaled(HANDLE object) {
+  // Figure out which registry key signalled this change.
+  KeyEntryList::iterator it;
+  for (it = keys_to_watch_.begin(); it != keys_to_watch_.end(); ++it) {
+    if ((*it)->watch_event() == object)
+      break;
+  }
+
+  DCHECK(it != keys_to_watch_.end());
+
+  // Keep watching the registry key.
+  if (!(*it)->StartWatching(this))
+    keys_to_watch_.erase(it);
+
+  // Have the PollingProxyConfigService test for changes.
+  CheckForChangesNow();
+}
+
 // static
 void ProxyConfigServiceWin::GetCurrentProxyConfig(ProxyConfig* config) {
   WINHTTP_CURRENT_USER_IE_PROXY_CONFIG ie_config = {0};
diff --git a/net/proxy/proxy_config_service_win.h b/net/proxy/proxy_config_service_win.h
index 308f445..e10e9a0 100644
--- a/net/proxy/proxy_config_service_win.h
+++ b/net/proxy/proxy_config_service_win.h
@@ -9,19 +9,60 @@
 #include <windows.h>
 #include <winhttp.h>
 
+#include <vector>
+
 #include "base/gtest_prod_util.h"
+#include "base/object_watcher.h"
 #include "net/proxy/polling_proxy_config_service.h"
 
 namespace net {
 
 // Implementation of ProxyConfigService that retrieves the system proxy
 // settings.
-class ProxyConfigServiceWin : public PollingProxyConfigService {
+//
+// It works by calling WinHttpGetIEProxyConfigForCurrentUser() to fetch the
+// Internet Explorer proxy settings.
+//
+// We use two different strategies to notice when the configuration has
+// changed:
+//
+// (1) Watch the internet explorer settings registry keys for changes. When
+//     one of the registry keys pertaining to proxy settings has changed, we
+//     call WinHttpGetIEProxyConfigForCurrentUser() again to read the
+//     configuration's new value.
+//
+// (2) Do regular polling every 10 seconds during network activity to see if
+//     WinHttpGetIEProxyConfigForCurrentUser() returns something different.
+//
+// Ideally strategy (1) should be sufficient to pick up all of the changes.
+// However we still do the regular polling as a precaution in case the
+// implementation details of  WinHttpGetIEProxyConfigForCurrentUser() ever
+// change, or in case we got it wrong (and are not checking all possible
+// registry dependencies).
+class ProxyConfigServiceWin : public PollingProxyConfigService,
+                              public base::ObjectWatcher::Delegate {
  public:
   ProxyConfigServiceWin();
+  virtual ~ProxyConfigServiceWin();
+
+  // Overrides a function from PollingProxyConfigService.
+  virtual void AddObserver(Observer* observer);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(ProxyConfigServiceWinTest, SetFromIEConfig);
+  class KeyEntry;
+  typedef std::vector<KeyEntry*> KeyEntryList;
+
+  // Registers change observers on the registry keys relating to proxy settings.
+  void StartWatchingRegistryForChanges();
+
+  // Creates a new KeyEntry and appends it to |keys_to_watch_|. If the key
+  // fails to be created, it is not appended to the list and we return false.
+  bool AddKeyToWatchList(HKEY rootkey, const wchar_t* subkey);
+
+  // ObjectWatcher::Delegate methods:
+  // This is called whenever one of the registry keys we are watching change.
+  virtual void OnObjectSignaled(HANDLE object);
 
   static void GetCurrentProxyConfig(ProxyConfig* config);
 
@@ -29,6 +70,8 @@ class ProxyConfigServiceWin : public PollingProxyConfigService {
   static void SetFromIEConfig(
       ProxyConfig* config,
       const WINHTTP_CURRENT_USER_IE_PROXY_CONFIG& ie_config);
+
+  KeyEntryList keys_to_watch_;
 };
 
 }  // namespace net