components/metrics: Add runtime memory leak detector

components/metrics/leak_detector/call_stack_manager_unittest.cc

Index: components/metrics/leak_detector/call_stack_manager_unittest.cc
diff --git a/components/metrics/leak_detector/call_stack_manager_unittest.cc b/components/metrics/leak_detector/call_stack_manager_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..571ceef17252ab25bf23b80c7f432d91a78f4185
--- /dev/null
+++ b/components/metrics/leak_detector/call_stack_manager_unittest.cc
@@ -0,0 +1,191 @@
+// Copyright 2015 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 "components/metrics/leak_detector/call_stack_manager.h"
+
+#include <gperftools/custom_allocator.h>
+
+#include "base/macros.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace leak_detector {
+
+namespace {
+
+// Some test call stacks.
+const void* kRawStack0[] = {
+  reinterpret_cast<const void*>(0xaabbccdd),
+  reinterpret_cast<const void*>(0x11223344),
+  reinterpret_cast<const void*>(0x55667788),

[jar (doing other things), 2015/09/04 18:01:56]

I noticed all these addresses were 32 bits long.  Is this code intended to work
on 64 bit platforms?

You could use the address of various items to transparently acquire the "native"
pointer sizes.

[Simon Que, 2015/09/07 06:40:32]

Done. I'll rely on truncation from 64 to 32 bits on 32-bit platforms instead.

+  reinterpret_cast<const void*>(0x99887766),
+};
+const void* kRawStack1[] = {

[jar (doing other things), 2015/09/04 18:01:56]

For purposes of testing... it was nice that this stack had the same length as
kRawStack0. That strengthened the tests.

IMO, you may as well make it nearly identical, with only one central element
differing.  Maybe something like (I'm not sure if this is legal... but you could
do it with shared constants if not):

const void* kRawStack[] = {
  kRawStack0[0],
  kRawStack0[1],
  reinterpret_cast<const void*>(0x87654321),  // Or make it a function of the
related value.
  kRowStack0[2],
};

[Simon Que, 2015/09/07 06:40:32]

Done.

+  reinterpret_cast<const void*>(0xdeadbeef),
+  reinterpret_cast<const void*>(0x900df00d),
+  reinterpret_cast<const void*>(0xcafedeed),
+  reinterpret_cast<const void*>(0xdeafbabe),
+};
+const void* kRawStack2[] = {
+  reinterpret_cast<const void*>(0x12345678),
+  reinterpret_cast<const void*>(0xabcdef01),
+  reinterpret_cast<const void*>(0xfdecab98),
+};
+const void* kRawStack3[] = {
+  reinterpret_cast<const void*>(0xdead0001),
+  reinterpret_cast<const void*>(0xbeef0002),
+  reinterpret_cast<const void*>(0x900d0003),
+  reinterpret_cast<const void*>(0xf00d0004),
+  reinterpret_cast<const void*>(0xcafe0005),
+  reinterpret_cast<const void*>(0xdeed0006),
+  reinterpret_cast<const void*>(0xdeaf0007),
+  reinterpret_cast<const void*>(0xbabe0008),
+};
+
+}  // namespace
+
+class CallStackManagerTest : public ::testing::Test {
+ public:
+  CallStackManagerTest() {}
+
+  void SetUp() override {
+    CustomAllocator::InitializeForUnitTest();
+  }
+  void TearDown() override {
+    CustomAllocator::Shutdown();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CallStackManagerTest);
+};
+
+TEST_F(CallStackManagerTest, NewStacks) {
+  CallStackManager manager;
+  EXPECT_EQ(0U, manager.size());
+
+  const CallStack* stack0 =
+      manager.GetCallStack(arraysize(kRawStack0), kRawStack0);
+  EXPECT_EQ(1U, manager.size());
+
+  const CallStack* stack1 =
+      manager.GetCallStack(arraysize(kRawStack1), kRawStack1);
+  EXPECT_EQ(2U, manager.size());
+
+  const CallStack* stack2 =
+      manager.GetCallStack(arraysize(kRawStack2), kRawStack2);
+  EXPECT_EQ(3U, manager.size());
+
+  const CallStack* stack3 =
+      manager.GetCallStack(arraysize(kRawStack3), kRawStack3);
+  EXPECT_EQ(4U, manager.size());
+
+  // Call stack objects should be unique.
+  EXPECT_NE(stack0, stack1);
+  EXPECT_NE(stack0, stack2);
+  EXPECT_NE(stack0, stack3);
+  EXPECT_NE(stack1, stack2);
+  EXPECT_NE(stack1, stack3);
+  EXPECT_NE(stack2, stack3);
+}
+
+TEST_F(CallStackManagerTest, Hashes) {
+  CallStackManager manager;
+
+  const CallStack* stack0 =
+      manager.GetCallStack(arraysize(kRawStack0), kRawStack0);
+  const CallStack* stack1 =
+      manager.GetCallStack(arraysize(kRawStack1), kRawStack1);
+  const CallStack* stack2 =
+      manager.GetCallStack(arraysize(kRawStack2), kRawStack2);
+  const CallStack* stack3 =
+      manager.GetCallStack(arraysize(kRawStack3), kRawStack3);
+
+  // Hash values should be nonzero.
+  EXPECT_NE(0U, stack0->hash);
+  EXPECT_NE(0U, stack1->hash);
+  EXPECT_NE(0U, stack2->hash);
+  EXPECT_NE(0U, stack3->hash);
+
+  // Hash values should be unique.
+  EXPECT_NE(stack0->hash, stack1->hash);
+  EXPECT_NE(stack0->hash, stack2->hash);
+  EXPECT_NE(stack0->hash, stack3->hash);
+  EXPECT_NE(stack1->hash, stack2->hash);
+  EXPECT_NE(stack1->hash, stack3->hash);
+  EXPECT_NE(stack2->hash, stack3->hash);
+}
+
+TEST_F(CallStackManagerTest, HashWithReducedDepth) {
+  CallStackManager manager;
+  const CallStack* stack =
+      manager.GetCallStack(arraysize(kRawStack3), kRawStack3);

[jar (doing other things), 2015/09/04 18:01:56]

Please add:
EXPECT_EQ(arraysize(kRawStack3), stack->depth);

...or better... would be to add these expectations instead around lines 66-80,
so that it is clear that the attribute is established.

[Simon Que, 2015/09/07 06:40:32]

Done.

+
+  // Hash function should only operate on the first |CallStack::depth| elements
+  // of CallStack::stack. To test this, reduce the depth value of one of the
+  // stacks and make sure the hash changes.
+  EXPECT_NE(stack->hash,
+            manager.GetCallStack(stack->depth - 1, stack->stack)->hash);
+  EXPECT_NE(stack->hash,
+            manager.GetCallStack(stack->depth - 2, stack->stack)->hash);
+  EXPECT_NE(stack->hash,
+            manager.GetCallStack(stack->depth - 3, stack->stack)->hash);
+  EXPECT_NE(stack->hash,
+            manager.GetCallStack(stack->depth - 4, stack->stack)->hash);

[jar (doing other things), 2015/09/04 18:01:56]

This is good... but it just checks that similar base stacks have distinct
hashes.  For instance, if only the last entry in the stack were consulted (and
earlier values were ignored), this would all pass.

You could probably add:
  EXPECT_NE(stack->hash,
            manager.GetCallStack(stack->depth - 1, stack->stack + 1)->hash);
  EXPECT_NE(stack->hash,
            manager.GetCallStack(stack->depth - 2, stack->stack + 2)->hash);
  EXPECT_NE(stack->hash,
            manager.GetCallStack(stack->depth - 3, stack->stack + 3)->hash);
  EXPECT_NE(stack->hash,
            manager.GetCallStack(stack->depth - 4, stack->stack + 4)->hash);

[Simon Que, 2015/09/07 06:40:32]

Done.

+}
+
+TEST_F(CallStackManagerTest, DuplicateStacks) {
+  CallStackManager manager;
+  EXPECT_EQ(0U, manager.size());

[bjanakiraman1, 2015/09/01 21:48:55]

In general, add a brief comment at the beginning saying what its testing.
Similar to the test above, which explains the purpose clearly.

[Simon Que, 2015/09/01 23:23:30]

Done.

+
+  // Add stack0.
+  const CallStack* stack0 =
+      manager.GetCallStack(arraysize(kRawStack0), kRawStack0);
+
+  const CallStack* stack0_duplicate0 =
+      manager.GetCallStack(arraysize(kRawStack0), kRawStack0);

[jar (doing other things), 2015/09/04 18:01:56]

Better might be to replicate the stack into new memory before passing data to
GetCallStack().  This would make it clear that the contents of the stack were
replicated (when the internal records were kept), and the address of the stack
data argument was not used in the calculations (of the hash, etc.)

[Simon Que, 2015/09/07 06:40:32]

Done.

+  EXPECT_EQ(1U, manager.size());
+  EXPECT_EQ(stack0, stack0_duplicate0);
+
+  // Add stack1.
+  const CallStack* stack1 =
+      manager.GetCallStack(arraysize(kRawStack1), kRawStack1);
+
+  const CallStack* stack0_duplicate1 =
+      manager.GetCallStack(arraysize(kRawStack0), kRawStack0);
+  EXPECT_EQ(2U, manager.size());
+  EXPECT_EQ(stack0, stack0_duplicate1);
+
+  const CallStack* stack1_duplicate0 =
+      manager.GetCallStack(arraysize(kRawStack1), kRawStack1);
+  EXPECT_EQ(2U, manager.size());
+  EXPECT_EQ(stack1, stack1_duplicate0);
+
+  // Add stack2 and stack3.
+  const CallStack* stack2 =
+      manager.GetCallStack(arraysize(kRawStack2), kRawStack2);
+  const CallStack* stack3 =
+      manager.GetCallStack(arraysize(kRawStack3), kRawStack3);
+  EXPECT_EQ(4U, manager.size());
+
+  const CallStack* stack1_duplicate1 =
+      manager.GetCallStack(arraysize(kRawStack1), kRawStack1);
+  EXPECT_EQ(4U, manager.size());
+  EXPECT_EQ(stack1, stack1_duplicate1);
+
+  const CallStack* stack0_duplicate2 =
+      manager.GetCallStack(arraysize(kRawStack0), kRawStack0);
+  EXPECT_EQ(4U, manager.size());
+  EXPECT_EQ(stack0, stack0_duplicate2);
+
+  const CallStack* stack3_duplicate0 =
+      manager.GetCallStack(arraysize(kRawStack3), kRawStack3);
+  EXPECT_EQ(4U, manager.size());
+  EXPECT_EQ(stack3, stack3_duplicate0);
+
+  const CallStack* stack2_duplicate0 =
+      manager.GetCallStack(arraysize(kRawStack2), kRawStack2);
+  EXPECT_EQ(4U, manager.size());
+  EXPECT_EQ(stack2, stack2_duplicate0);
+}
+
+}  // namespace leak_detector