Add cooldown to LeakDetectorImpl leak report generation

components/metrics/leak_detector/leak_detector_impl_unittest.cc

Index: components/metrics/leak_detector/leak_detector_impl_unittest.cc
diff --git a/components/metrics/leak_detector/leak_detector_impl_unittest.cc b/components/metrics/leak_detector/leak_detector_impl_unittest.cc
index 960ac842820a2987040a3015683e3978485ab7cd..5035271fd404b1edfefd458d54e6d5e2ba1a94d6 100644
--- a/components/metrics/leak_detector/leak_detector_impl_unittest.cc
+++ b/components/metrics/leak_detector/leak_detector_impl_unittest.cc
@@ -26,6 +26,9 @@ namespace {
 // Makes working with complex numbers easier.
 using Complex = std::complex<double>;
 
+using InternalLeakReport = LeakDetectorImpl::LeakReport;
+using AllocationBreakdown = LeakDetectorImpl::LeakReport::AllocationBreakdown;
+
 // The mapping location in memory for a fictional executable.
 const uintptr_t kMappingAddr = 0x800000;
 const size_t kMappingSize = 0x200000;
@@ -82,6 +85,13 @@ const size_t kAllocedSizeAnalysisInterval = 8192;
 const uint32_t kSizeSuspicionThreshold = 4;
 const uint32_t kCallStackSuspicionThreshold = 4;
 
+// Because it takes N+1 analyses to reach a suspicion threshold of N (the
+// suspicion score is only calculated based on deltas from the previous
+// analysis), the actual number of analyses it takes to generate a report for
+// the first time is:
+const uint32_t kMinNumAnalysesToGenerateReport =
+    kSizeSuspicionThreshold + 1 + kCallStackSuspicionThreshold + 1;
+
 // Returns the offset within [kMappingAddr, kMappingAddr + kMappingSize) if
 // |addr| falls in that range. Otherwise, returns |addr|.
 uintptr_t GetOffsetInMapping(uintptr_t addr) {
@@ -96,6 +106,21 @@ uint32_t SizeToIndex(size_t size) {
   return size / sizeof(uint32_t);
 }
 
+// Returns true if the |alloc_breakdown_history_| field of the two LeakReports
+// |a| and |b| are the same.
+bool CompareReportAllocHistory(const InternalLeakReport& a,
+                               const InternalLeakReport& b) {
+  auto alloc_breakdown_compare_func = [](AllocationBreakdown a,
+                                         AllocationBreakdown b) -> bool {
+    return std::equal(a.counts_by_size.begin(), a.counts_by_size.end(),
+                      b.counts_by_size.begin()) &&
+           a.count_for_call_stack == b.count_for_call_stack;
+  };
+  return std::equal(
+      a.alloc_breakdown_history().begin(), a.alloc_breakdown_history().end(),
+      b.alloc_breakdown_history().begin(), alloc_breakdown_compare_func);
+}
+
 }  // namespace
 
 // This test suite will test the ability of LeakDetectorImpl to catch leaks in
@@ -110,7 +135,8 @@ class LeakDetectorImplTest : public ::testing::Test {
       : total_num_allocs_(0),
         total_num_frees_(0),
         total_alloced_size_(0),
-        next_analysis_total_alloced_size_(kAllocedSizeAnalysisInterval) {}
+        next_analysis_total_alloced_size_(kAllocedSizeAnalysisInterval),
+        num_reports_generated_(0) {}
 
   void SetUp() override {
     CustomAllocator::Initialize();
@@ -135,10 +161,8 @@ class LeakDetectorImplTest : public ::testing::Test {
   }
 
  protected:
-  using InternalLeakReport = LeakDetectorImpl::LeakReport;
   template <typename T>
   using InternalVector = LeakDetectorImpl::InternalVector<T>;
-  using AllocationBreakdown = LeakDetectorImpl::LeakReport::AllocationBreakdown;
 
   // Alloc and free functions that allocate and free heap memory and
   // automatically pass alloc/free info to |detector_|. They emulate the
@@ -174,6 +198,7 @@ class LeakDetectorImplTest : public ::testing::Test {
           stored_reports_.insert(report);
         }
       }
+      num_reports_generated_ += reports.size();
 
       // Determine when the next leak analysis should occur.
       while (total_alloced_size_ >= next_analysis_total_alloced_size_)
@@ -226,6 +251,10 @@ class LeakDetectorImplTest : public ::testing::Test {
   // Store leak reports here. Use a set so duplicate reports are not stored.
   std::set<InternalLeakReport> stored_reports_;
 
+  // Keeps track of the actual number of reports (duplicate or not) that were
+  // generated by |detector_|.
+  size_t num_reports_generated_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(LeakDetectorImplTest);
 };
@@ -233,7 +262,7 @@ class LeakDetectorImplTest : public ::testing::Test {
 void LeakDetectorImplTest::SimpleLeakyFunction(bool enable_leaks) {
   std::vector<uint32_t*> ptrs(7);
 
-  const int kNumOuterIterations = 20;
+  const int kNumOuterIterations = 32;
   for (int j = 0; j < kNumOuterIterations; ++j) {
     // The inner loop allocates 256 bytes. Run it 32 times so that 8192 bytes
     // (|kAllocedSizeAnalysisInterval|) are allocated for each iteration of the
@@ -491,7 +520,8 @@ TEST_F(LeakDetectorImplTest, SimpleLeakyFunctionNoLeak) {
   // SimpleLeakyFunction() should have run cleanly without leaking.
   EXPECT_EQ(total_num_allocs_, total_num_frees_);
   EXPECT_EQ(0U, alloced_ptrs_.size());
-  ASSERT_EQ(0U, stored_reports_.size());
+  EXPECT_EQ(0U, num_reports_generated_);
+  EXPECT_EQ(0U, stored_reports_.size());
 }
 
 TEST_F(LeakDetectorImplTest, SimpleLeakyFunctionWithLeak) {
@@ -500,6 +530,7 @@ TEST_F(LeakDetectorImplTest, SimpleLeakyFunctionWithLeak) {
   // SimpleLeakyFunction() should generated some leak reports.
   EXPECT_GT(total_num_allocs_, total_num_frees_);
   EXPECT_GT(alloced_ptrs_.size(), 0U);
+  EXPECT_EQ(2U, num_reports_generated_);
   ASSERT_EQ(2U, stored_reports_.size());
 
   // The reports should be stored in order of size.
@@ -530,11 +561,12 @@ TEST_F(LeakDetectorImplTest, SimpleLeakyFunctionWithLeak) {
   //   that come right after. So it should count the two extra allocs made at
   //   call sites |kStack3| and |kStack4|. The formula is |(i + 1) * 32 + 2|,
   //   where |i| is the iteration index.
-  // There should have been one leak analysis per outer loop iteration, for a
-  // total of 20 history records (|kNumOuterIterations|) per report.
+  // - It takes |kMinNumAnalysesToGenerateReport| analyses for the first report
+  //   to be generated. Subsequent analyises do not generate reports due to the
+  //   cooldown mechanism.
 
   const auto& report1_history = report1.alloc_breakdown_history();
-  EXPECT_EQ(20U, report1_history.size());
+  EXPECT_EQ(kMinNumAnalysesToGenerateReport, report1_history.size());
 
   for (size_t i = 0; i < report1_history.size(); ++i) {
     const AllocationBreakdown& entry = report1_history[i];
@@ -570,15 +602,80 @@ TEST_F(LeakDetectorImplTest, SimpleLeakyFunctionWithLeak) {
 
   // |report2| should have the same size history and call stack history as
   // |report1|.
-  const auto& report2_history = report2.alloc_breakdown_history();
-  auto compare_history_func =
-      [](AllocationBreakdown a, AllocationBreakdown b) -> bool {
-        return std::equal(a.counts_by_size.begin(), a.counts_by_size.end(),
-                          b.counts_by_size.begin()) &&
-               a.count_for_call_stack == b.count_for_call_stack;
-      };
-  EXPECT_TRUE(std::equal(report1_history.begin(), report1_history.end(),
-                         report2_history.begin(), compare_history_func));
+  EXPECT_TRUE(CompareReportAllocHistory(report1, report2));
+}
+
+TEST_F(LeakDetectorImplTest, SimpleLeakyFunctionWithLeakThreeTimes) {
+  // Run three iterations of the leaky function.
+  SimpleLeakyFunction(true /* enable_leaks */);
+  SimpleLeakyFunction(true /* enable_leaks */);
+  SimpleLeakyFunction(true /* enable_leaks */);
+
+  // SimpleLeakyFunction() should have generated three times as many leak
+  // reports, because the number of iterations is the same as the cooldown of
+  // LeakDetectorImpl. But the number of unique reports stored is still two.
+  EXPECT_EQ(6U, num_reports_generated_);
+  ASSERT_EQ(2U, stored_reports_.size());
+
+  // The reports should be stored in order of size.
+
+  // |report1| comes from the call site marked with kStack1, with size=32.
+  const InternalLeakReport& report1 = *stored_reports_.begin();
+  EXPECT_EQ(32U, report1.alloc_size_bytes());
+  ASSERT_EQ(kStack1.depth, report1.call_stack().size());
+  for (size_t i = 0; i < kStack1.depth; ++i) {
+    EXPECT_EQ(GetOffsetInMapping(kStack1.stack[i]), report1.call_stack()[i])
+        << i;
+  }
+
+  // |report2| comes from the call site marked with kStack2, with size=48.
+  const InternalLeakReport& report2 = *(++stored_reports_.begin());
+  EXPECT_EQ(48U, report2.alloc_size_bytes());
+  ASSERT_EQ(kStack2.depth, report2.call_stack().size());
+  for (size_t i = 0; i < kStack2.depth; ++i) {
+    EXPECT_EQ(GetOffsetInMapping(kStack2.stack[i]), report2.call_stack()[i])
+        << i;
+  }
+
+  const auto& report1_history = report1.alloc_breakdown_history();
+  EXPECT_EQ(32U, report1_history.size());
+
+  for (size_t i = 1; i < report1_history.size(); ++i) {
+    const InternalVector<uint32_t>& counts_by_size =
+        report1_history[i].counts_by_size;
+    const InternalVector<uint32_t>& prev_counts_by_size =
+        report1_history[i - 1].counts_by_size;
+    ASSERT_GT(counts_by_size.size(), SizeToIndex(48));
+
+    // Check the two leaky sizes, 32 and 48. At this point, the exact counts
+    // could be computed but the computations are too complex for a unit test.
+    // Instead, check that the counts increase by 32 from the previous count.
+    // Same goes for checking call site counts later.
+    EXPECT_GT(counts_by_size[SizeToIndex(32)], 0U);
+    EXPECT_GT(counts_by_size[SizeToIndex(48)], 0U);
+    EXPECT_EQ(prev_counts_by_size[SizeToIndex(32)] + 32,
+              counts_by_size[SizeToIndex(32)]);
+    EXPECT_EQ(prev_counts_by_size[SizeToIndex(48)] + 32,
+              counts_by_size[SizeToIndex(48)]);
+
+    // Not related to the leaks, but there should be a dangling 16-byte
+    // allocation during each leak analysis, because it hasn't yet been freed.
+    EXPECT_EQ(1U, counts_by_size[SizeToIndex(16)]);
+  }
+
+  // Check call site count over time.
+  ASSERT_LT(kSizeSuspicionThreshold, report1_history.size());
+  // Sufficient time has passed since the first report was generated. The entire
+  // alloc history should contain call site counts.
+  for (size_t i = 1; i < report1_history.size(); ++i) {
+    EXPECT_GT(report1_history[i].count_for_call_stack, 0U);
+    EXPECT_EQ(report1_history[i - 1].count_for_call_stack + 32,
+              report1_history[i].count_for_call_stack);
+  }
+
+  // |report2| should have the same size history and call stack history as
+  // |report1|.
+  EXPECT_TRUE(CompareReportAllocHistory(report1, report2));
 }
 
 TEST_F(LeakDetectorImplTest, JuliaSetNoLeak) {
@@ -587,6 +684,7 @@ TEST_F(LeakDetectorImplTest, JuliaSetNoLeak) {
   // JuliaSet() should have run cleanly without leaking.
   EXPECT_EQ(total_num_allocs_, total_num_frees_);
   EXPECT_EQ(0U, alloced_ptrs_.size());
+  EXPECT_EQ(0U, num_reports_generated_);
   ASSERT_EQ(0U, stored_reports_.size());
 }
 
@@ -596,6 +694,7 @@ TEST_F(LeakDetectorImplTest, JuliaSetWithLeak) {
   // JuliaSet() should have leaked some memory from two call sites.
   EXPECT_GT(total_num_allocs_, total_num_frees_);
   EXPECT_GT(alloced_ptrs_.size(), 0U);
+  EXPECT_GT(num_reports_generated_, 0U);
 
   // There should be one unique leak report generated for each leaky call site.
   ASSERT_EQ(2U, stored_reports_.size());
@@ -644,16 +743,8 @@ TEST_F(LeakDetectorImplTest, JuliaSetWithLeak) {
   EXPECT_GT(counts_by_size[size_1_index], counts_by_size[size_0_index] * 10);
   EXPECT_GT(counts_by_size[size_2_index], counts_by_size[size_0_index] * 10);
 
-  // |report2| should have the same size history as |report1|, but not the same
-  // call stack history.
-  const auto& report2_history = report2.alloc_breakdown_history();
-  auto compare_size_history_func =
-      [](AllocationBreakdown a, AllocationBreakdown b) -> bool {
-        return std::equal(a.counts_by_size.begin(), a.counts_by_size.end(),
-                          b.counts_by_size.begin());
-      };
-  EXPECT_TRUE(std::equal(report1_history.begin(), report1_history.end(),
-                         report2_history.begin(), compare_size_history_func));
+  // |report1| and |report2| do not necessarily have the same allocation history
+  // due to the different rates at which they were generated.

[Simon Que, 2016/04/21 00:33:08]

I forgot to remove this in the previous patch set. This is no longer applicable
now that we have a cooldown.

 }
 
 }  // namespace leak_detector