Fix test crashes by doing an additional GC on cleanup.

Fix test crashes by doing an additional GC on cleanup.

R=ager@chromium.org, erik.corry@gmail.com, haraken@chromium.org, oilpan-reviews@chromium.org, tkent@chromium.org, vegorov@chromium.org, zerny@chromium.org
BUG=
NOTRY=true

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168014

[haraken, 2014-02-27 11:30:31]

https://codereview.chromium.org/181823004/diff/1/Source/heap/ThreadState.cpp
File Source/heap/ThreadState.cpp (right):

https://codereview.chromium.org/181823004/diff/1/Source/heap/ThreadState.cpp#...
Source/heap/ThreadState.cpp:301: // FIXME: oilpan: get rid of the additional GC.

Just help me understand: Why is this a FIXME? I guess that we need to call
multiple GCs to break persistent chains.

https://codereview.chromium.org/181823004/diff/1/Source/heap/ThreadState.cpp#...
Source/heap/ThreadState.cpp:303:
Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);

Are two GCs enough?

[Vyacheslav Egorov (Chromium), 2014-02-27 11:32:25]

LGTM!

https://codereview.chromium.org/181823004/diff/1/Source/heap/ThreadState.cpp
File Source/heap/ThreadState.cpp (right):

https://codereview.chromium.org/181823004/diff/1/Source/heap/ThreadState.cpp#...
Source/heap/ThreadState.cpp:301: // FIXME: oilpan: get rid of the additional GC.
On 2014/02/27 11:30:31, haraken wrote:
> 
> Just help me understand: Why is this a FIXME? I guess that we need to call
> multiple GCs to break persistent chains.

Because in the end everything will be moved to the heap and persistents will
disappear.

https://codereview.chromium.org/181823004/diff/1/Source/heap/ThreadState.cpp#...
Source/heap/ThreadState.cpp:303:
Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);
On 2014/02/27 11:30:31, haraken wrote:
> 
> Are two GCs enough?

It is enough for the test we are trying to fix.

[wibling-chromium, 2014-02-27 11:32:31]

The CQ bit was checked by wibling@chromium.org

[commit-bot: I haz the power, 2014-02-27 11:32:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wibling@chromium.org/181823004/20001

[commit-bot: I haz the power, 2014-02-27 11:32:57]

Change committed as 168014

[haraken, 2014-02-27 11:37:19]

https://codereview.chromium.org/181823004/diff/1/Source/heap/ThreadState.cpp
File Source/heap/ThreadState.cpp (right):

https://codereview.chromium.org/181823004/diff/1/Source/heap/ThreadState.cpp#...
Source/heap/ThreadState.cpp:301: // FIXME: oilpan: get rid of the additional GC.
On 2014/02/27 11:32:26, Vyacheslav Egorov wrote:
> On 2014/02/27 11:30:31, haraken wrote:
> > 
> > Just help me understand: Why is this a FIXME? I guess that we need to call
> > multiple GCs to break persistent chains.
> 
> Because in the end everything will be moved to the heap and persistents will
> disappear.

I'm not sure how realistic it is. We'll anyway have off-heap and on-heap, and
will have a pointer from off-heap to on-heap (which is persistent).

https://codereview.chromium.org/181823004/diff/1/Source/heap/ThreadState.cpp#...
Source/heap/ThreadState.cpp:307: m_heaps[i]->assertEmpty();

As long as we assert this (this assertion is enabled in release mode), I think
we should call collectGarbage multiple times until we remove all persistent
handles.

It won't make sense that we call only one GC and assert the emptiness whereas we
still have a lot of persistent handles.