Skip to content

Commit

Permalink
Oilpan: introduce sticky forcedForTesting flag to ensure that a precise
Browse files Browse the repository at this point in the history
GC is performed at the next return to the event loop.

Use this in svg animation tests to be able to reliably check that
objects die when expected.

R=erik.corry@gmail.com, haraken@chromium.org, kouhei@chromium.org, oilpan-reviews@chromium.org, vegorov@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/220203005

git-svn-id: svn://svn.chromium.org/blink/trunk@170648 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
ager@chromium.org committed Apr 2, 2014
1 parent d063925 commit 3ac2149
Show file tree
Hide file tree
Showing 9 changed files with 123 additions and 62 deletions.
4 changes: 0 additions & 4 deletions third_party/WebKit/LayoutTests/OilpanExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,3 @@ crbug.com/342574 [ Mac Debug ] fast/css-generated-content/crash-selection-editin

crbug.com/350316 [ Linux Win Debug ] http/tests/eventsource/workers/eventsource-simple.html [ Crash ]
crbug.com/350316 [ Mac Debug ] http/tests/eventsource/workers/eventsource-simple.html [ Timeout ]

crbug.com/356900 svg/animations/smil-leak-element-instances-noBaseValRef.svg [ Pass Failure ]
crbug.com/356900 svg/animations/smil-leak-element-instances.svg [ Pass Failure ]
crbug.com/356900 svg/animations/smil-leak-elements.svg [ Pass Failure ]
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,26 @@
if (!window.internals) {
debug("This test only runs on \"content_shell --dump-render-tree\", as it requires existence of window.internals.");
} else {
gc();
var documentsBefore = window.internals.numberOfLiveDocuments();
testRunner.waitUntilDone();
window.jsTestIsAsync = true;
var documentsBefore;
var documentsAfter;
collectGarbage(function() {
documentsBefore = window.internals.numberOfLiveDocuments();

var frame = document.getElementById('frame');
frame.contentDocument.body.innerHTML = '<form></form>';
document.body.removeChild(frame);
frame = null;
var frame = document.getElementById('frame');
frame.contentDocument.body.innerHTML = '<form></form>';
document.body.removeChild(frame);
frame = null;

gc();
var documentsAfter = window.internals.numberOfLiveDocuments();
collectGarbage(function() {
documentsAfter = window.internals.numberOfLiveDocuments();

// -1 is from removing frame itself.
shouldBe('documentsBefore - 1', 'documentsAfter');
// -1 is from removing frame itself.
shouldBe('documentsBefore - 1', 'documentsAfter');
finishJSTest();
});
});
}
</script>
</body>
Expand Down
24 changes: 24 additions & 0 deletions third_party/WebKit/LayoutTests/resources/js-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,30 @@ function shouldHaveHadError(message)
testFailed("expectError() not called before shouldHaveHadError()");
}

// With Oilpan tests that rely on garbage collection need to go through
// the event loop in order to get precise garbage collections. Oilpan
// uses conservative stack scanning when not at the event loop and that
// can artificially keep objects alive. Therefore, tests that need to check
// that something is dead need to use this asynchronous collectGarbage
// function.
function collectGarbage(callback) {
// Perform multiple GCs to break sequences of Oilpan Persistent handles
// or RefPtrs that will keep objects alive until the next GC.
// FIXME: Oilpan: Once everything is moved to the oilpan heap we can
// reduce the number of garbage collections.
GCController.collect();
setTimeout(function() {
GCController.collect();
setTimeout(function() {
GCController.collect();
setTimeout(function() {
GCController.collect();
setTimeout(callback, 0);
}, 0);
}, 0);
}, 0);
}

function gc() {
if (typeof GCController !== "undefined")
GCController.collectAll();
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions third_party/WebKit/Source/heap/Heap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1305,6 +1305,8 @@ void Heap::prepareForGC()

void Heap::collectGarbage(ThreadState::StackState stackState, GCType gcType)
{
if (gcType == ForcedForTesting && stackState != ThreadState::NoHeapPointersOnStack)
ThreadState::current()->setForcedForTesting(true);
ThreadState::current()->clearGCRequested();
GCScope gcScope(stackState);

Expand Down
27 changes: 23 additions & 4 deletions third_party/WebKit/Source/heap/ThreadState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ ThreadState::ThreadState()
, m_atSafePoint(false)
, m_interruptors()
, m_gcRequested(false)
, m_forcePreciseGCForTesting(false)
, m_sweepRequested(0)
, m_sweepInProgress(false)
, m_noAllocationCount(0)
Expand Down Expand Up @@ -497,6 +498,26 @@ void ThreadState::clearGCRequested()
m_gcRequested = false;
}

void ThreadState::performPendingGC(StackState stackState)
{
if (stackState == NoHeapPointersOnStack && (gcRequested() || forcePreciseGCForTesting())) {
setForcedForTesting(false);
Heap::collectGarbage(NoHeapPointersOnStack);
}
}

void ThreadState::setForcedForTesting(bool value)
{
checkThread();
m_forcePreciseGCForTesting = value;
}

bool ThreadState::forcePreciseGCForTesting()
{
checkThread();
return m_forcePreciseGCForTesting;
}

bool ThreadState::isConsistentForGC()
{
for (int i = 0; i < NumberOfHeaps; i++) {
Expand Down Expand Up @@ -589,8 +610,7 @@ void ThreadState::resumeThreads()
void ThreadState::safePoint(StackState stackState)
{
checkThread();
if (stackState == NoHeapPointersOnStack && gcRequested())
Heap::collectGarbage(NoHeapPointersOnStack);
performPendingGC(stackState);
m_stackState = stackState;
s_safePointBarrier->checkAndPark(this);
m_stackState = HeapPointersOnStack;
Expand Down Expand Up @@ -628,8 +648,7 @@ void ThreadState::enterSafePoint(StackState stackState, void* scopeMarker)
scopeMarker = adjustScopeMarkerForAdressSanitizer(scopeMarker);
#endif
ASSERT(stackState == NoHeapPointersOnStack || scopeMarker);
if (stackState == NoHeapPointersOnStack && gcRequested())
Heap::collectGarbage(NoHeapPointersOnStack);
performPendingGC(stackState);
checkThread();
ASSERT(!m_atSafePoint);
m_atSafePoint = true;
Expand Down
12 changes: 12 additions & 0 deletions third_party/WebKit/Source/heap/ThreadState.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,15 @@ class HEAP_EXPORT ThreadState {
void setGCRequested();
void clearGCRequested();

// Was the last GC forced for testing? This is set when garbage collection
// is forced for testing and there are pointers on the stack. It remains
// set until a garbage collection is triggered with no pointers on the stack.
// This is used for layout tests that trigger GCs and check if objects are
// dead at a given point in time. That only reliably works when we get
// precise GCs with no conservative stack scanning.
void setForcedForTesting(bool);
bool forcePreciseGCForTesting();

bool sweepRequested();
void setSweepRequested();
void clearSweepRequested();
Expand Down Expand Up @@ -497,6 +506,8 @@ class HEAP_EXPORT ThreadState {
m_safePointScopeMarker = 0;
}

void performPendingGC(StackState);

// Finds the Blink HeapPage in this thread-specific heap
// corresponding to a given address. Return 0 if the address is
// not contained in any of the pages. This does not consider
Expand Down Expand Up @@ -540,6 +551,7 @@ class HEAP_EXPORT ThreadState {
bool m_atSafePoint;
Vector<Interruptor*> m_interruptors;
bool m_gcRequested;
bool m_forcePreciseGCForTesting;
volatile int m_sweepRequested;
bool m_sweepInProgress;
size_t m_noAllocationCount;
Expand Down

0 comments on commit 3ac2149

Please sign in to comment.