Don't poll the PAC script during periods of network inactivity.

net/proxy/proxy_service.cc

 // 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 "net/proxy/proxy_service.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 59 matching lines @@
 // set up. Network failures can happen at any time though, so we additionally
 // poll the PAC script for changes, which will allow us to recover from these
 // sorts of problems.
 const int64 kDelayAfterNetworkChangesMs = 2000;
 
 // The initial number of milliseconds to wait before refetching PAC script after
 // a fetch error/success.
 const int64 kInitialPollDelayForErrorMs = 4000;
 const int64 kInitialPollDelayForSuccessMs = 16000;
 
+// Once the poll delay becomes larger than this, we will stop scheduling using
+// a timer, and instead wait for an incoming request to trigger the poll.
+const int64 kMaxPollDelayUsingTimer = 16000;
+
 // The maximum poll delay for checking PAC scripts.
 const int64 kMaxPollDelayMs = 120000;  // 2 minutes.
 
+// This is the default policy for polling the PAC script. Checks happen after an
+// exponentially increasing delay. The first couple checks will be scheduled
+// using a timer. After that the checks will only be done lazily in response to
+// a user request.
+class DefaultPollPolicy : public ProxyService::PacPollPolicy {
+ public:
+  DefaultPollPolicy() {}
+
+  virtual Mode GetInitialDelay(int error, int64* next_delay_ms) const OVERRIDE {
+    // Use a shorter wait delay if the initial fetch resulted in an error.
+    *next_delay_ms = (error != OK) ?
+        kInitialPollDelayForErrorMs : kInitialPollDelayForSuccessMs;
+    return MODE_USE_TIMER;
+  }
+
+  virtual Mode GetNextDelay(int64 current_delay_ms,
+                            int64* next_delay_ms) const OVERRIDE {
+    // Increase the wait delay exponentially up to a maximum.
+    *next_delay_ms = std::min(current_delay_ms * 2, kMaxPollDelayMs);
+
+    // Once the delays are large enough, stop scheduling using a timer and
+    // instead only poll during periods of activity.
+    return (*next_delay_ms <= kMaxPollDelayUsingTimer) ?
+        MODE_USE_TIMER: MODE_START_AFTER_ACTIVITY;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DefaultPollPolicy);
+};
+
 // Config getter that always returns direct settings.
 class ProxyConfigServiceDirect : public ProxyConfigService {
  public:
   // ProxyConfigService implementation:
   virtual void AddObserver(Observer* observer) OVERRIDE {}
   virtual void RemoveObserver(Observer* observer) OVERRIDE {}
   virtual ConfigAvailability GetLatestProxyConfig(ProxyConfig* config)
       OVERRIDE {
     *config = ProxyConfig::CreateDirect();
     return CONFIG_VALID;
@@ skipping 206 matching lines @@
     return dict;
   }
 
  private:
   std::vector<std::string> proxy_list_;
   DISALLOW_COPY_AND_ASSIGN(BadProxyListNetLogParam);
 };
 
 }  // namespace
 
+// ProxyService::PacPollPolicy ------------------------------------------------
+
+ProxyService::PacPollPolicy::~PacPollPolicy() {}

[wtc, 2012/01/13 01:08:45]

Nit: why can't this destructor be defined in the header?

[eroman, 2012/01/13 01:29:10]

@erg has gone to a lot of effort to remove code from headers in order to speed
up compile speeds.

I believe it is desirable to keep the implementation for destructors (even
trivial ones) in the .cc file rather than the header for this reason.

I have cc-ed Eliot to verify that my interpretation of the rules is correct.

[eroman, 2012/01/13 20:05:27]

Done.

+
 // ProxyService::InitProxyResolver --------------------------------------------
 
 // This glues together two asynchronous steps:
 //   (1) ProxyScriptDecider -- try to fetch/validate a sequence of PAC scripts to
 //       figure out what we should configure against.
 //   (2) Feed the fetched PAC script into the ProxyResolver.
 //
 // InitProxyResolver is a single-use class which encapsulates cancellation as
 // part of its destructor. Start() or StartSkipDecider() should be called just
 // once. The instance can be destroyed at any time, and the request will be
@@ skipping 205 matching lines @@
                            int init_net_error,
                            ProxyResolverScriptData* init_script_data,
                            NetLog* net_log)
       : ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
         change_callback_(callback),
         config_(config),
         proxy_resolver_expects_pac_bytes_(proxy_resolver_expects_pac_bytes),
         proxy_script_fetcher_(proxy_script_fetcher),
         dhcp_proxy_script_fetcher_(dhcp_proxy_script_fetcher),
         last_error_(init_net_error),
-        last_script_data_(init_script_data) {
+        last_script_data_(init_script_data),
+        last_poll_time_(base::TimeTicks::Now()) {
     // Set the initial poll delay -- we check more aggressively right after a
     // failure in case it was due to a spurious network error.
-    current_poll_delay_ms_ = GetInitialWaitDelayMs(init_net_error);
-
-    StartPollTimer();
+    next_poll_mode_ = poll_policy()->GetInitialDelay(
+        init_net_error, &next_poll_delay_ms_);
+    TryToStartNextPoll(false);
   }
 
-  // ----------------------------------
-  // Policy for the retry scheduling.
-  // ----------------------------------
-
-  static int64 GetInitialWaitDelayMs(int error) {
-    switch (poll_policy_) {
-      case POLL_POLICY_REGULAR:
-        // Use a shorter wait delay if the initial fetch resulted in an error.
-        return (error != OK) ?
-            kInitialPollDelayForErrorMs : kInitialPollDelayForSuccessMs;
-      case POLL_POLICY_IMMEDIATE:
-        // Wait a mere 1 millisecond.
-        return 1;
-      case POLL_POLICY_NEVER:
-        // Unreasonably large wait delay, will never fire the check.
-        return 0xFFFFFFFF;
-    }
-
-    // Shouldn't be reached
-    return -1;
+  void OnLazyPoll() {
+    // We have just been notified of network activity. Use this opportunity to
+    // see if we can start our next poll.
+    TryToStartNextPoll(true);
   }
 
-  static int64 GetNextWaitDelayMs(int64 current_delay_ms) {
-    switch (poll_policy_) {
-      case POLL_POLICY_REGULAR:
-        // Increase the delay exponentially up to 2 minutes.
-        return std::min(current_delay_ms * 2, kMaxPollDelayMs);
-      case POLL_POLICY_IMMEDIATE:
-      case POLL_POLICY_NEVER:
-        return current_delay_ms;
-    }
-
-    // Shouldn't be reached
-    return -1;
-  }
-
-  static PollPolicy set_policy(PollPolicy policy) {
-    PollPolicy prev = poll_policy_;
+  static const PacPollPolicy* set_policy(const PacPollPolicy* policy) {
+    const PacPollPolicy* prev = poll_policy_;
     poll_policy_ = policy;
     return prev;
   }
 
  private:
+  // Returns the effective poll policy (the one injected by unit-tests, or the
+  // default).
+  const PacPollPolicy* poll_policy() {
+    if (poll_policy_)
+      return poll_policy_;
+    return &default_poll_policy_;
+  }
+
   void StartPollTimer() {
     DCHECK(!decider_.get());
 
     MessageLoop::current()->PostDelayedTask(
         FROM_HERE,
-        base::Bind(&ProxyScriptDeciderPoller::OnPollTimerFired,
+        base::Bind(&ProxyScriptDeciderPoller::DoPoll,
                    weak_factory_.GetWeakPtr()),
-        current_poll_delay_ms_);
+        next_poll_delay_ms_);
   }
 
-  void OnPollTimerFired() {
+  void TryToStartNextPoll(bool triggered_by_activity) {
+    switch (next_poll_mode_) {
+      case PacPollPolicy::MODE_USE_TIMER:
+        if (!triggered_by_activity)
+          StartPollTimer();
+        break;
+
+      case PacPollPolicy::MODE_START_AFTER_ACTIVITY:
+        if (triggered_by_activity && !decider_.get()) {
+          base::TimeDelta elapsed_time =
+              base::TimeTicks::Now() - last_poll_time_;
+          if (elapsed_time.InMilliseconds() >= next_poll_delay_ms_)
+            DoPoll();
+        }
+        break;
+    }
+  }
+
+  void DoPoll() {
+    last_poll_time_ = base::TimeTicks::Now();
+
     // Start the proxy script decider to see if anything has changed.
     // TODO(eroman): Pass a proper NetLog rather than NULL.
     decider_.reset(new ProxyScriptDecider(
         proxy_script_fetcher_, dhcp_proxy_script_fetcher_, NULL));
     int result = decider_->Start(
         config_, base::TimeDelta(), proxy_resolver_expects_pac_bytes_,
         base::Bind(&ProxyScriptDeciderPoller::OnProxyScriptDeciderCompleted,
                    base::Unretained(this)));
 
     if (result != ERR_IO_PENDING)
@@ skipping 13 matching lines @@
             &ProxyScriptDeciderPoller::NotifyProxyServiceOfChange,
             weak_factory_.GetWeakPtr(),
             result,
             make_scoped_refptr(decider_->script_data()),
             decider_->effective_config()));
       return;
     }
 
     decider_.reset();
 
-    // Schedule the next poll check.
-    current_poll_delay_ms_ = GetNextWaitDelayMs(current_poll_delay_ms_);
-    StartPollTimer();
+    // Decide when the next poll should take place, and possibly start the
+    // next timer.
+    next_poll_mode_ =
+        poll_policy()->GetNextDelay(next_poll_delay_ms_, &next_poll_delay_ms_);
+    TryToStartNextPoll(false);
   }
 
   bool HasScriptDataChanged(int result, ProxyResolverScriptData* script_data) {
     if (result != last_error_) {
       // Something changed -- it was failing before and now it succeeded, or
       // conversely it succeeded before and now it failed. Or it failed in
       // both cases, however the specific failure error codes differ.
       return true;
     }
 
@@ skipping 22 matching lines @@
   ChangeCallback change_callback_;
   ProxyConfig config_;
   bool proxy_resolver_expects_pac_bytes_;
   ProxyScriptFetcher* proxy_script_fetcher_;
   DhcpProxyScriptFetcher* dhcp_proxy_script_fetcher_;
 
   int last_error_;
   scoped_refptr<ProxyResolverScriptData> last_script_data_;
 
   scoped_ptr<ProxyScriptDecider> decider_;
-  int64 current_poll_delay_ms_;
+  int64 next_poll_delay_ms_;
+  PacPollPolicy::Mode next_poll_mode_;
 
-  static PollPolicy poll_policy_;
+  base::TimeTicks last_poll_time_;
+
+  // Polling policy injected by unit-tests. Otherwise this is NULL and the
+  // default policy will be used.
+  static const PacPollPolicy* poll_policy_;
+
+  const DefaultPollPolicy default_poll_policy_;
 
   DISALLOW_COPY_AND_ASSIGN(ProxyScriptDeciderPoller);
 };
 
 // static
-ProxyService::PollPolicy ProxyService::ProxyScriptDeciderPoller::poll_policy_ =
-    ProxyService::POLL_POLICY_REGULAR;
+const ProxyService::PacPollPolicy*
+    ProxyService::ProxyScriptDeciderPoller::poll_policy_ = NULL;
 
 // ProxyService::PacRequest ---------------------------------------------------
 
 class ProxyService::PacRequest
     : public base::RefCounted<ProxyService::PacRequest> {
  public:
     PacRequest(ProxyService* service,
              const GURL& url,
              ProxyInfo* results,
              const net::CompletionCallback& user_callback,
@@ skipping 250 matching lines @@
 int ProxyService::ResolveProxy(const GURL& raw_url,
                                ProxyInfo* result,
                                const net::CompletionCallback& callback,
                                PacRequest** pac_request,
                                const BoundNetLog& net_log) {
   DCHECK(CalledOnValidThread());
   DCHECK(!callback.is_null());
 
   net_log.BeginEvent(NetLog::TYPE_PROXY_SERVICE, NULL);
 
+  // Notify our polling-based dependencies that a resolve is taking place.
+  // This way they can schedule their polls in response to network activity.
   config_service_->OnLazyPoll();
+  if (script_poller_.get())
+     script_poller_->OnLazyPoll();
+
   if (current_state_ == STATE_NONE)
     ApplyProxyConfigIfAvailable();
 
   // Strip away any reference fragments and the username/password, as they
   // are not relevant to proxy resolution.
   GURL url = SimplifyUrlForRequest(raw_url);
 
   // Check if the request can be completed right away. (This is the case when
   // using a direct connection for example).
   int rv = TryToCompleteSynchronously(url, result);
@@ skipping 381 matching lines @@
 
   return linux_config_service;
 #else
   LOG(WARNING) << "Failed to choose a system proxy settings fetcher "
                   "for this platform.";
   return new ProxyConfigServiceDirect();
 #endif
 }
 
 // static
-ProxyService::PollPolicy ProxyService::set_pac_script_poll_policy(
-    PollPolicy policy) {
+const ProxyService::PacPollPolicy* ProxyService::set_pac_script_poll_policy(
+    const PacPollPolicy* policy) {
   return ProxyScriptDeciderPoller::set_policy(policy);
 }
 
+// static
+scoped_ptr<ProxyService::PacPollPolicy>
+  ProxyService::CreateDefaultPacPollPolicy() {
+  return scoped_ptr<PacPollPolicy>(new DefaultPollPolicy());
+}
+
 void ProxyService::OnProxyConfigChanged(
     const ProxyConfig& config,
     ProxyConfigService::ConfigAvailability availability) {
   // Retrieve the current proxy configuration from the ProxyConfigService.
   // If a configuration is not available yet, we will get called back later
   // by our ProxyConfigService::Observer once it changes.
   ProxyConfig effective_config;
   switch (availability) {
     case ProxyConfigService::CONFIG_PENDING:
       // ProxyConfigService implementors should never pass CONFIG_PENDING.
@@ skipping 160 matching lines @@
     OnCompletion(result_);
   }
 }
 
 void SyncProxyServiceHelper::OnCompletion(int rv) {
   result_ = rv;
   event_.Signal();
 }
 
 }  // namespace net