Oilpan: blink_heap_unittests: Test case have memory leak checked by Lsan

Source/platform/heap/HeapTest.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 4134 matching lines @@
     EXPECT_EQ(4u, map1->size());
     Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
     EXPECT_EQ(2, OffHeapInt::s_destructorCalls);
     EXPECT_EQ(2u, map1->size());
     Iterator i1 = map1->begin();
     EXPECT_TRUE(i1->key.first->value() == 103 || i1->key.first == livingInt);
     EXPECT_EQ(livingInt, i1->key.second);
     ++i1;
     EXPECT_TRUE(i1->key.first->value() == 103 || i1->key.first == livingInt);
     EXPECT_EQ(livingInt, i1->key.second);
+
+    map1.clear();
+    livingInt.clear();

[haraken, 2014/08/11 02:00:07]

Hmm, I don't fully understand why we need to call clear() here. These persistent
handles will be anyway destructed when the current scope exists.

Can we probably move Heap::collectGarbage() to HeapTest::TearDown? It's a pain
that if we have to explicitly clear all persistent handles and call
Heap::collectGarabge() in each test.

[Daniel Chow, 2014/08/11 02:44:21]

it's a good idea, I will make a try today

On 2014/08/11 02:00:07, haraken wrote:

+    Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
 }
 
 TEST(HeapTest, MapWithCustomWeaknessHandling2)
 {
     typedef HeapHashMap<RefPtr<OffHeapInt>, PairWithWeakHandling> Map;
     typedef Map::iterator Iterator;
     HeapStats initialHeapSize;
     clearOutOldGarbage(&initialHeapSize);
     OffHeapInt::s_destructorCalls = 0;
 
@@ skipping 51 matching lines @@
     EXPECT_EQ(4u, map1->size());
     Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
     EXPECT_EQ(2, OffHeapInt::s_destructorCalls);
     EXPECT_EQ(2u, map1->size());
     Iterator i1 = map1->begin();
     EXPECT_TRUE(i1->value.first->value() == 103 || i1->value.first == livingInt);
     EXPECT_EQ(livingInt, i1->value.second);
     ++i1;
     EXPECT_TRUE(i1->value.first->value() == 103 || i1->value.first == livingInt);
     EXPECT_EQ(livingInt, i1->value.second);
+
+    map1.clear();
+    livingInt.clear();
+    Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
 }
 
 TEST(HeapTest, Bind)
 {
     Closure closure = bind(&Bar::trace, Bar::create(), static_cast<Visitor*>(0));
     Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
     // The closure should have a persistent handle to the Bar.
     EXPECT_EQ(1u, Bar::s_live);
 
     Closure closure2 = bind(&Bar::trace, RawPtr<Bar>(Bar::create()), static_cast<Visitor*>(0));
@@ skipping 732 matching lines @@
 
     static volatile IntWrapper* s_workerObjectPointer;
 };
 
 TEST(HeapTest, RecursiveMutex)
 {
     RecursiveLockingTester::test();
 }
 
 template<typename T>
-class TraceIfNeededTester : public GarbageCollected<TraceIfNeededTester<T> > {
+class TraceIfNeededTester : public GarbageCollectedFinalized<TraceIfNeededTester<T> > {

[haraken, 2014/08/11 02:00:07]

This change looks correct.

Ian: It would be great if the plugin can catch this error :)

 public:
     static TraceIfNeededTester<T>* create() { return new TraceIfNeededTester<T>(); }
     static TraceIfNeededTester<T>* create(const T& obj) { return new TraceIfNeededTester<T>(obj); }
     void trace(Visitor* visitor) { TraceIfNeeded<T>::trace(visitor, &m_obj); }
     T& obj() { return m_obj; }
+    ~TraceIfNeededTester() { }
 private:
     TraceIfNeededTester() { }
     explicit TraceIfNeededTester(const T& obj) : m_obj(obj) { }
     T m_obj;
 };
 
 class PartObject {
     DISALLOW_ALLOCATION();
 public:
     PartObject() : m_obj(SimpleObject::create()) { }
@@ skipping 27 matching lines @@
         EXPECT_EQ(1u, visitor.count());
     }
 
     {
         TraceIfNeededTester<HeapVector<Member<SimpleObject> > >* m_vec = TraceIfNeededTester<HeapVector<Member<SimpleObject> > >::create();
         m_vec->obj().append(SimpleObject::create());
         visitor.reset();
         m_vec->trace(&visitor);
         EXPECT_EQ(2u, visitor.count());
     }
+
+    Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
 }
 
 } // namespace blink