Fix a DCHECK that could fire in PassiveLogCollector for a particular stream o...

chrome/browser/net/passive_log_collector_unittest.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/passive_log_collector.h"
 
 #include "base/compiler_specific.h"
 #include "base/format_macros.h"
 #include "base/string_util.h"
 #include "net/url_request/url_request_netlog_params.h"
@@ skipping 420 matching lines @@
     log.OnAddEntry(NetLog::TYPE_REQUEST_ALIVE,
                    base::TimeTicks(),
                    NetLog::Source(NetLog::SOURCE_URL_REQUEST, next_id++),
                    NetLog::PHASE_END,
                    NULL);
   }
   EXPECT_EQ(RequestTracker::kMaxGraveyardSize,
             GetDeadSources(log.url_request_tracker_).size());
 }
 
+// Regression test for http://crbug.com/58847
+TEST(PassiveLogCollectorTest, ReleaseDependencyToUnreferencedSource) {
+  PassiveLogCollector x;

[mmenke, 2010/11/15 16:21:52]

nit:  You're undoubtedly more familiar with Google style guidelines than I, but
you might want to give this a better name.  Would make lines longer and a little
messier, though, so maybe not.

[eroman, 2010/11/15 19:28:10]

Done.

You are right, |x| is a bad name. (carry over from some test code I had).

I have renamed it to |log| to be consistent with the other tests in this file.

+
+  // If these constants are weird, the test won't be testing the right thing.
+  EXPECT_LT(PassiveLogCollector::RequestTracker::kMaxGraveyardSize,
+            PassiveLogCollector::RequestTracker::kMaxNumSources);
+
+  // Add a "reference" to a non-existant source (sourceID=1706 does not exist).
+  scoped_refptr<net::NetLog::EventParameters> params =
+      new net::NetLogSourceParameter(
+          "source_dependency",
+          net::NetLog::Source(net::NetLog::SOURCE_SOCKET, 1263));
+  x.OnAddEntry(net::NetLog::TYPE_SOCKET_POOL_BOUND_TO_SOCKET,
+               base::TimeTicks(),
+               net::NetLog::Source(net::NetLog::SOURCE_URL_REQUEST, 1706),
+               net::NetLog::PHASE_NONE,
+               params);
+
+  // At this point source 1706 has noted 1263 as a dependency. However the
+  // reference count for 1263 was not adjusted since it doesn't actually exist.
+
+  // Move source 1706 to the graveyard.
+  x.OnAddEntry(net::NetLog::TYPE_REQUEST_ALIVE,
+               base::TimeTicks(),
+               net::NetLog::Source(net::NetLog::SOURCE_URL_REQUEST, 1706),
+               net::NetLog::PHASE_END,
+               NULL);
+
+  // Now create a source entry for 1263, such that it is unreferenced and
+  // waiting to be garbage collected.
+  x.OnAddEntry(net::NetLog::TYPE_SOCKET_ALIVE,
+               base::TimeTicks(),
+               net::NetLog::Source(net::NetLog::SOURCE_SOCKET, 1263),
+               net::NetLog::PHASE_END, NULL);
+
+  // Add kMaxGraveyardSize  unreferenced URL_REQUESTS, so the circular buffer
+  // containing source 1706. After adding kMaxGraveyardSize - 1 the buffer
+  // will be full. Now when we add one more more source it will now evict the
+  // oldest item, which is 1706. In doing so, 1706 will try to release the
+  // reference it *thinks* it has on 1263. However 1263 has a reference count
+  // of 0 and is already in a graveyard.
+  for (size_t i = 0;
+       i < PassiveLogCollector::RequestTracker::kMaxGraveyardSize; ++i) {
+    x.OnAddEntry(net::NetLog::TYPE_REQUEST_ALIVE,
+                 base::TimeTicks(),
+                 net::NetLog::Source(net::NetLog::SOURCE_URL_REQUEST, i),
+                 net::NetLog::PHASE_END,
+                 NULL);
+  }
+
+  // To pass, this should simply not have DCHECK-ed above.
+}
+