Handle race between deleting a low use referrer acruing value

Prevent a DCHECK from firing.

As we adaptively learn about referrers to enhance DNS
pre-resolution, we are forced to evict some entries
to prevent accidentally getting really large lists
(when we make mistakes).  Sometimes we evict a 
referred item from a list *while* that item is being
resolved (because it was in teh list).  In that case,
we have to be careful when we try to acrue credit 
(latency savings).

r=paulg
Review URL: http://codereview.chromium.org/18527

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

2 files changed, 21 insertions, 14 deletions

diff --git a/chrome/browser/net/referrer.cc b/chrome/browser/net/referrer.cc
index 45982fa..464c990 100644
--- a/chrome/browser/net/referrer.cc
+++ b/chrome/browser/net/referrer.cc
@@ -9,11 +9,12 @@
 namespace chrome_browser_net {
 
 void Referrer::SuggestHost(const std::string& host) {
-  // Limit how large our list can get, in case we start make mistakes about
-  // what hostnames are in sub-resources (example: Some advertisments have
-  // a link to the ad agency, and then provide a "surprising" redirect to
-  // the advertised entity, which appears to be a subresource on the page
+  // Limit how large our list can get, in case we make mistakes about what
+  // hostnames are in sub-resources (example: Some advertisments have a link to
+  // the ad agency, and then provide a "surprising" redirect to the advertised
+  // entity, which then (mistakenly) appears to be a subresource on the page
   // hosting the ad).
+  // TODO(jar): Do experiments to optimize the max count of suggestions.
   static const size_t kMaxSuggestions = 8;
 
   if (host.empty())
@@ -57,12 +58,18 @@ void Referrer::DeleteLeastUseful() {
     least_useful_lifetime = lifetime;
   }
   erase(least_useful_name);
+  // Note: there is a small chance that we will discard a least_useful_name
+  // that is currently being prefetched because it *was* in this referer list.
+  // In that case, when a benefit appears in AccrueValue() below, we are careful
+  // to check before accessing the member.
 }
 
 void Referrer::AccrueValue(const base::TimeDelta& delta,
                            const std::string host) {
-  DCHECK(this->find(host) != this->end());
-  (*this)[host].AccrueValue(delta);
+  HostNameMap::iterator it = this->find(host);
+  // Be careful that we weren't evicted from this referrer in DeleteLeastUseful.
+  if (it != this->end())
+    it->second.AccrueValue(delta);
 }
 
 }  // namespace chrome_browser_net
diff --git a/chrome/browser/net/referrer.h b/chrome/browser/net/referrer.h
index 455b9b7..d3fb12e 100644
--- a/chrome/browser/net/referrer.h
+++ b/chrome/browser/net/referrer.h
@@ -47,13 +47,13 @@ typedef std::map<std::string, ReferrerValue> HostNameMap;
 
 //------------------------------------------------------------------------------
 // There is one Referrer instance for each hostname that has acted as an HTTP
-// referer (note mispelling is intentional) for a domain that was otherwise
-// unexpectedly navgated towards ("unexpected" in the sense that the domain was
-// probably for a subresource of a page, and was not otherwise predictable until
-// the content with the reference arrived).  Most typically, an outer page was a
-// page fetched by the user, and this instance lists names in HostNameMap which
-// are subresources and that were needed to complete the rendering of the outer
-// page.
+// referer (note mispelling is intentional) for a hostname that was otherwise
+// unexpectedly navgated towards ("unexpected" in the sense that the hostname
+// was probably needad as a subresource of a page, and was not otherwise
+// predictable until the content with the reference arrived).  Most typically,
+// an outer page was a page fetched by the user, and this instance lists names
+// in HostNameMap which are subresources and that were needed to complete the
+// rendering of the outer page.
 class Referrer : public HostNameMap {
  public:
   // Add the indicated host to the list of hosts that are resolved via DNS when
@@ -69,7 +69,7 @@ class Referrer : public HostNameMap {
   // Helper function for pruning list.  Metric for usefulness is "large accrued
   // value," in the form of latency_ savings associated with a host name.  We
   // also give credit for a name being newly added, by scalling latency per
-  // lifetime (time since birth).  For instance, when to names have accrued
+  // lifetime (time since birth).  For instance, when two names have accrued
   // the same latency_ savings, the older one is less valuable as it didn't
   // accrue savings as quickly.
   void DeleteLeastUseful();