Change PassiveLogCollector to clear before adding the |max|th entry.

That way the final entry will not be cleared once the maximum has been reached.

(This addresses an earlier review comment from willchan).
Review URL: http://codereview.chromium.org/2848005

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

2 files changed, 19 insertions, 17 deletions

diff --git a/chrome/browser/net/passive_log_collector.cc b/chrome/browser/net/passive_log_collector.cc
index c1e6c9b..2f01d3a 100644
--- a/chrome/browser/net/passive_log_collector.cc
+++ b/chrome/browser/net/passive_log_collector.cc
@@ -141,8 +141,20 @@ PassiveLogCollector::SourceTracker::SourceTracker(
 PassiveLogCollector::SourceTracker::~SourceTracker() {}
 
 void PassiveLogCollector::SourceTracker::OnAddEntry(const Entry& entry) {
-  SourceInfo& info = sources_[entry.source.id];
-  info.source_id = entry.source.id;  // In case this is a new entry.
+  // Lookup or insert a new entry into the bounded map.
+  SourceIDToInfoMap::iterator it = sources_.find(entry.source.id);
+  if (it == sources_.end()) {
+    if (sources_.size() >= max_num_sources_) {
+      LOG(WARNING) << "The passive log data has grown larger "
+                      "than expected, resetting";
+      Clear();
+    }
+    it = sources_.insert(
+        SourceIDToInfoMap::value_type(entry.source.id, SourceInfo())).first;
+    it->second.source_id = entry.source.id;
+  }
+
+  SourceInfo& info = it->second;
   Action result = DoAddEntry(entry, &info);
 
   if (result != ACTION_NONE) {
@@ -164,14 +176,6 @@ void PassiveLogCollector::SourceTracker::OnAddEntry(const Entry& entry) {
       }
     }
   }
-
-  if (sources_.size() > max_num_sources_) {
-    // This is a safety net in case something went wrong, to avoid continually
-    // growing memory.
-    LOG(WARNING) << "The passive log data has grown larger "
-                    "than expected, resetting";
-    Clear();
-  }
 }
 
 void PassiveLogCollector::SourceTracker::DeleteSourceInfo(
diff --git a/chrome/browser/net/passive_log_collector_unittest.cc b/chrome/browser/net/passive_log_collector_unittest.cc
index fcef0fb..d0ccc06 100644
--- a/chrome/browser/net/passive_log_collector_unittest.cc
+++ b/chrome/browser/net/passive_log_collector_unittest.cc
@@ -100,14 +100,12 @@ TEST(RequestTrackerTest, DropsAfterMaximumSize) {
 
   EXPECT_EQ(RequestTracker::kMaxNumSources, GetLiveSources(tracker).size());
 
-  // Add 5 more -- this should cause it to exceed its expected peak, and
+  // Add 1 more -- this should cause it to exceed its expected peak, and
   // therefore reset all of its data.
-  for (size_t i = 0; i < 5u; ++i) {
-    tracker.OnAddEntry(
-        MakeStartLogEntry(i + RequestTracker::kMaxNumSources));
-  }
+  tracker.OnAddEntry(
+      MakeStartLogEntry(1 + RequestTracker::kMaxNumSources));
 
-  EXPECT_EQ(4u, GetLiveSources(tracker).size());
+  EXPECT_EQ(1u, GetLiveSources(tracker).size());
 }
 
 TEST(RequestTrackerTest, BasicBounded) {
@@ -439,7 +437,7 @@ TEST(PassiveLogCollectorTest, HoldReferenceToDeletedSource) {
                    NetLog::PHASE_BEGIN,
                    NULL);
   }
-  ASSERT_EQ(0u, GetLiveSources(log.socket_tracker_).size());
+  ASSERT_EQ(1u, GetLiveSources(log.socket_tracker_).size());
 
   // End the original request. Then saturate the graveyard with enough other
   // requests to cause it to be deleted. Once that source is deleted, it will