haikuwebkit/LayoutTests/performance-api/performance-observer-no-doc...

52 lines
1.6 KiB
HTML
Raw Permalink Normal View History

JSPerformanceObserverCallback creates a GC strongly-referenced Function that is never cleaned up https://bugs.webkit.org/show_bug.cgi?id=186873 <rdar://problem/41271574> Reviewed by Simon Fraser. Source/WebCore: Add [IsWeakCallback] to PerformanceObserverCallback interface so that the generated JSPerformanceObserverCallback uses a JSC::Weak instead of a JSC::Strong to store the js function. To keep the function alive, add [JSCustomMarkFunction] to PerformanceObserver interface and have its visitAdditionalChildren() visit the callback's js function. Finally, because we want the callback to still be called even if the JS does not keep the PerformanceObserver wrapper alive, add [CustomIsReachable] to PerformanceObserver interface and have its isReachableFromOpaqueRoots() return true if the observer is registered (i.e. it may need to call the callback in the future). I have confirmed locally, that the Performance / PerformanceObserver / Document objects properly get destroyed if I navigate away from a page that had a performance observer and trigger a memory pressure warning. Also, `notifyutil -p com.apple.WebKit.showAllDocuments` no longer shows the old document. Tests: performance-api/performance-observer-callback-after-gc.html performance-api/performance-observer-no-document-leak.html * Sources.txt: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSPerformanceObserverCustom.cpp: Added. (WebCore::JSPerformanceObserver::visitAdditionalChildren): (WebCore::JSPerformanceObserverOwner::isReachableFromOpaqueRoots): * bindings/js/ScriptController.cpp: * page/PerformanceObserver.cpp: (WebCore::PerformanceObserver::disassociate): * page/PerformanceObserver.h: (WebCore::PerformanceObserver::isRegistered const): (WebCore::PerformanceObserver::callback): * page/PerformanceObserver.idl: * page/PerformanceObserverCallback.h: * page/PerformanceObserverCallback.idl: LayoutTests: * performance-api/performance-observer-callback-after-gc-expected.txt: Added. * performance-api/performance-observer-callback-after-gc.html: Added. Add layout test to make sure that a performance observer's callback still gets called, even if the JS does not keep the performance observer alive. * performance-api/performance-observer-no-document-leak-expected.txt: Added. * performance-api/performance-observer-no-document-leak.html: Added. * performance-api/resources/performance-observer-no-document-leak-frame.html: Added. Add layout test coverage to make sure the document does not leak if PerformanceObserver was used. Canonical link: https://commits.webkit.org/202150@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233053 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2018-06-21 19:04:02 +00:00
<!DOCTYPE html>
<html>
<head>
<script src="../resources/js-test-pre.js"></script>
Code pattern in GC tests in LayoutTests is broken https://bugs.webkit.org/show_bug.cgi?id=211595 Reviewed by Saam Barati. LayoutTests have several tests which attempt to ensure that document is not leaked. But they are broken since these tests are not correctly handling conservative GC of JavaScriptCore. These tests are taking a following approach. 1. Allocate *one* iframe doing something inside it. 2. Remove iframe from the parent document to make it released. 3. Repeatedly invoke GC and test whether (1)'s document gets collected. This is not the right approach. Since JavaScriptCore has conservative GC, it is easily possible that (1)'s pointer value remains in some of conservative roots: CPU registers, stack etc. And JavaScriptCore conservative GC scans it and keeps it alive. The above approach makes the test super flaky and any unrelated changes in JavaScriptCore and C compiler can make this test failed. This is not a problem in practice. Web pages are executing various kind of code and they clobber conservative roots. However, tests in LayoutTests are too simple to keep it alive accidentally. The right approach for conservative GC is the following. 1. Allocate *many* iframes doing something inside them with loop. By creating iframes with loop, for every iteration, it is likely that the same CPU registers and stack locations are overwritten by the last created iframe reference. This dramatically reduces the possibility for GC to find these addresses in conservative roots. 2. Remove iframes from the parent document to make them released. 3. Repeatedly invoke GC and test whether *one of (1) iframes* gets collected at least. Theoretically this is still possible that completely unrelated integer value in conservative roots can point to the reference of (1). By allocating many iframes in (1) and testing one of them, we can reduce the possibility of this conflict. This patch adds testDocumentIsNotLeaked helper function to enforce this pattern. And rewrite broken tests with this helper to make it less-flaky. * highlight/highlight-world-leak.html: * http/tests/IndexedDB/collect-IDB-objects.https.html: * http/tests/media/clearkey/collect-webkit-media-session.html: * http/tests/media/media-stream/collect-media-devices.https.html: * http/tests/resources/gc.js: Added. (window.gc.gcRec): (window.gc.window.gc): (nukeArray): (async testDocumentIsNotLeaked): * intersection-observer/no-document-leak.html: * performance-api/performance-observer-no-document-leak.html: * resources/gc.js: (nukeArray): (async testDocumentIsNotLeaked): * webanimations/leak-document-with-web-animation.html: Canonical link: https://commits.webkit.org/224541@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@261391 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2020-05-08 17:14:15 +00:00
<script src="../resources/gc.js"></script>
JSPerformanceObserverCallback creates a GC strongly-referenced Function that is never cleaned up https://bugs.webkit.org/show_bug.cgi?id=186873 <rdar://problem/41271574> Reviewed by Simon Fraser. Source/WebCore: Add [IsWeakCallback] to PerformanceObserverCallback interface so that the generated JSPerformanceObserverCallback uses a JSC::Weak instead of a JSC::Strong to store the js function. To keep the function alive, add [JSCustomMarkFunction] to PerformanceObserver interface and have its visitAdditionalChildren() visit the callback's js function. Finally, because we want the callback to still be called even if the JS does not keep the PerformanceObserver wrapper alive, add [CustomIsReachable] to PerformanceObserver interface and have its isReachableFromOpaqueRoots() return true if the observer is registered (i.e. it may need to call the callback in the future). I have confirmed locally, that the Performance / PerformanceObserver / Document objects properly get destroyed if I navigate away from a page that had a performance observer and trigger a memory pressure warning. Also, `notifyutil -p com.apple.WebKit.showAllDocuments` no longer shows the old document. Tests: performance-api/performance-observer-callback-after-gc.html performance-api/performance-observer-no-document-leak.html * Sources.txt: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSPerformanceObserverCustom.cpp: Added. (WebCore::JSPerformanceObserver::visitAdditionalChildren): (WebCore::JSPerformanceObserverOwner::isReachableFromOpaqueRoots): * bindings/js/ScriptController.cpp: * page/PerformanceObserver.cpp: (WebCore::PerformanceObserver::disassociate): * page/PerformanceObserver.h: (WebCore::PerformanceObserver::isRegistered const): (WebCore::PerformanceObserver::callback): * page/PerformanceObserver.idl: * page/PerformanceObserverCallback.h: * page/PerformanceObserverCallback.idl: LayoutTests: * performance-api/performance-observer-callback-after-gc-expected.txt: Added. * performance-api/performance-observer-callback-after-gc.html: Added. Add layout test to make sure that a performance observer's callback still gets called, even if the JS does not keep the performance observer alive. * performance-api/performance-observer-no-document-leak-expected.txt: Added. * performance-api/performance-observer-no-document-leak.html: Added. * performance-api/resources/performance-observer-no-document-leak-frame.html: Added. Add layout test coverage to make sure the document does not leak if PerformanceObserver was used. Canonical link: https://commits.webkit.org/202150@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233053 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2018-06-21 19:04:02 +00:00
</head>
<body>
<script>
description("Tests that using PerformanceObserver does not cause the document to get leaked.");
window.jsTestIsAsync = true;
Code pattern in GC tests in LayoutTests is broken https://bugs.webkit.org/show_bug.cgi?id=211595 Reviewed by Saam Barati. LayoutTests have several tests which attempt to ensure that document is not leaked. But they are broken since these tests are not correctly handling conservative GC of JavaScriptCore. These tests are taking a following approach. 1. Allocate *one* iframe doing something inside it. 2. Remove iframe from the parent document to make it released. 3. Repeatedly invoke GC and test whether (1)'s document gets collected. This is not the right approach. Since JavaScriptCore has conservative GC, it is easily possible that (1)'s pointer value remains in some of conservative roots: CPU registers, stack etc. And JavaScriptCore conservative GC scans it and keeps it alive. The above approach makes the test super flaky and any unrelated changes in JavaScriptCore and C compiler can make this test failed. This is not a problem in practice. Web pages are executing various kind of code and they clobber conservative roots. However, tests in LayoutTests are too simple to keep it alive accidentally. The right approach for conservative GC is the following. 1. Allocate *many* iframes doing something inside them with loop. By creating iframes with loop, for every iteration, it is likely that the same CPU registers and stack locations are overwritten by the last created iframe reference. This dramatically reduces the possibility for GC to find these addresses in conservative roots. 2. Remove iframes from the parent document to make them released. 3. Repeatedly invoke GC and test whether *one of (1) iframes* gets collected at least. Theoretically this is still possible that completely unrelated integer value in conservative roots can point to the reference of (1). By allocating many iframes in (1) and testing one of them, we can reduce the possibility of this conflict. This patch adds testDocumentIsNotLeaked helper function to enforce this pattern. And rewrite broken tests with this helper to make it less-flaky. * highlight/highlight-world-leak.html: * http/tests/IndexedDB/collect-IDB-objects.https.html: * http/tests/media/clearkey/collect-webkit-media-session.html: * http/tests/media/media-stream/collect-media-devices.https.html: * http/tests/resources/gc.js: Added. (window.gc.gcRec): (window.gc.window.gc): (nukeArray): (async testDocumentIsNotLeaked): * intersection-observer/no-document-leak.html: * performance-api/performance-observer-no-document-leak.html: * resources/gc.js: (nukeArray): (async testDocumentIsNotLeaked): * webanimations/leak-document-with-web-animation.html: Canonical link: https://commits.webkit.org/224541@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@261391 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2020-05-08 17:14:15 +00:00
window.onload = function () {
testDocumentIsNotLeaked(
async function initAndRemove(frameCount)
{
let frames = await new Promise((resolve, reject) => {
let frames = [];
let counter = 0;
function onLoad() {
counter++;
if (counter == frameCount)
resolve(frames);
}
for (let i = 0; i < frameCount; ++i) {
let frame = document.createElement('iframe');
frame.src = "resources/performance-observer-no-document-leak-frame.html";
frame.onload = onLoad;
document.body.appendChild(frame);
frames.push(frame);
}
});
let frameIdentifiers = [];
for (let i = 0; i < frameCount; ++i) {
let frame = frames[i];
frameIdentifiers.push(internals.documentIdentifier(frame.contentDocument));
frame.remove();
JSPerformanceObserverCallback creates a GC strongly-referenced Function that is never cleaned up https://bugs.webkit.org/show_bug.cgi?id=186873 <rdar://problem/41271574> Reviewed by Simon Fraser. Source/WebCore: Add [IsWeakCallback] to PerformanceObserverCallback interface so that the generated JSPerformanceObserverCallback uses a JSC::Weak instead of a JSC::Strong to store the js function. To keep the function alive, add [JSCustomMarkFunction] to PerformanceObserver interface and have its visitAdditionalChildren() visit the callback's js function. Finally, because we want the callback to still be called even if the JS does not keep the PerformanceObserver wrapper alive, add [CustomIsReachable] to PerformanceObserver interface and have its isReachableFromOpaqueRoots() return true if the observer is registered (i.e. it may need to call the callback in the future). I have confirmed locally, that the Performance / PerformanceObserver / Document objects properly get destroyed if I navigate away from a page that had a performance observer and trigger a memory pressure warning. Also, `notifyutil -p com.apple.WebKit.showAllDocuments` no longer shows the old document. Tests: performance-api/performance-observer-callback-after-gc.html performance-api/performance-observer-no-document-leak.html * Sources.txt: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSPerformanceObserverCustom.cpp: Added. (WebCore::JSPerformanceObserver::visitAdditionalChildren): (WebCore::JSPerformanceObserverOwner::isReachableFromOpaqueRoots): * bindings/js/ScriptController.cpp: * page/PerformanceObserver.cpp: (WebCore::PerformanceObserver::disassociate): * page/PerformanceObserver.h: (WebCore::PerformanceObserver::isRegistered const): (WebCore::PerformanceObserver::callback): * page/PerformanceObserver.idl: * page/PerformanceObserverCallback.h: * page/PerformanceObserverCallback.idl: LayoutTests: * performance-api/performance-observer-callback-after-gc-expected.txt: Added. * performance-api/performance-observer-callback-after-gc.html: Added. Add layout test to make sure that a performance observer's callback still gets called, even if the JS does not keep the performance observer alive. * performance-api/performance-observer-no-document-leak-expected.txt: Added. * performance-api/performance-observer-no-document-leak.html: Added. * performance-api/resources/performance-observer-no-document-leak-frame.html: Added. Add layout test coverage to make sure the document does not leak if PerformanceObserver was used. Canonical link: https://commits.webkit.org/202150@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233053 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2018-06-21 19:04:02 +00:00
}
Code pattern in GC tests in LayoutTests is broken https://bugs.webkit.org/show_bug.cgi?id=211595 Reviewed by Saam Barati. LayoutTests have several tests which attempt to ensure that document is not leaked. But they are broken since these tests are not correctly handling conservative GC of JavaScriptCore. These tests are taking a following approach. 1. Allocate *one* iframe doing something inside it. 2. Remove iframe from the parent document to make it released. 3. Repeatedly invoke GC and test whether (1)'s document gets collected. This is not the right approach. Since JavaScriptCore has conservative GC, it is easily possible that (1)'s pointer value remains in some of conservative roots: CPU registers, stack etc. And JavaScriptCore conservative GC scans it and keeps it alive. The above approach makes the test super flaky and any unrelated changes in JavaScriptCore and C compiler can make this test failed. This is not a problem in practice. Web pages are executing various kind of code and they clobber conservative roots. However, tests in LayoutTests are too simple to keep it alive accidentally. The right approach for conservative GC is the following. 1. Allocate *many* iframes doing something inside them with loop. By creating iframes with loop, for every iteration, it is likely that the same CPU registers and stack locations are overwritten by the last created iframe reference. This dramatically reduces the possibility for GC to find these addresses in conservative roots. 2. Remove iframes from the parent document to make them released. 3. Repeatedly invoke GC and test whether *one of (1) iframes* gets collected at least. Theoretically this is still possible that completely unrelated integer value in conservative roots can point to the reference of (1). By allocating many iframes in (1) and testing one of them, we can reduce the possibility of this conflict. This patch adds testDocumentIsNotLeaked helper function to enforce this pattern. And rewrite broken tests with this helper to make it less-flaky. * highlight/highlight-world-leak.html: * http/tests/IndexedDB/collect-IDB-objects.https.html: * http/tests/media/clearkey/collect-webkit-media-session.html: * http/tests/media/media-stream/collect-media-devices.https.html: * http/tests/resources/gc.js: Added. (window.gc.gcRec): (window.gc.window.gc): (nukeArray): (async testDocumentIsNotLeaked): * intersection-observer/no-document-leak.html: * performance-api/performance-observer-no-document-leak.html: * resources/gc.js: (nukeArray): (async testDocumentIsNotLeaked): * webanimations/leak-document-with-web-animation.html: Canonical link: https://commits.webkit.org/224541@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@261391 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2020-05-08 17:14:15 +00:00
nukeArray(frames);
frames = null;
return frameIdentifiers;
}
).then(
() => testPassed("Document did not leak"),
(error) => testFailed(error.message)
).finally(finishJSTest);
};
JSPerformanceObserverCallback creates a GC strongly-referenced Function that is never cleaned up https://bugs.webkit.org/show_bug.cgi?id=186873 <rdar://problem/41271574> Reviewed by Simon Fraser. Source/WebCore: Add [IsWeakCallback] to PerformanceObserverCallback interface so that the generated JSPerformanceObserverCallback uses a JSC::Weak instead of a JSC::Strong to store the js function. To keep the function alive, add [JSCustomMarkFunction] to PerformanceObserver interface and have its visitAdditionalChildren() visit the callback's js function. Finally, because we want the callback to still be called even if the JS does not keep the PerformanceObserver wrapper alive, add [CustomIsReachable] to PerformanceObserver interface and have its isReachableFromOpaqueRoots() return true if the observer is registered (i.e. it may need to call the callback in the future). I have confirmed locally, that the Performance / PerformanceObserver / Document objects properly get destroyed if I navigate away from a page that had a performance observer and trigger a memory pressure warning. Also, `notifyutil -p com.apple.WebKit.showAllDocuments` no longer shows the old document. Tests: performance-api/performance-observer-callback-after-gc.html performance-api/performance-observer-no-document-leak.html * Sources.txt: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSPerformanceObserverCustom.cpp: Added. (WebCore::JSPerformanceObserver::visitAdditionalChildren): (WebCore::JSPerformanceObserverOwner::isReachableFromOpaqueRoots): * bindings/js/ScriptController.cpp: * page/PerformanceObserver.cpp: (WebCore::PerformanceObserver::disassociate): * page/PerformanceObserver.h: (WebCore::PerformanceObserver::isRegistered const): (WebCore::PerformanceObserver::callback): * page/PerformanceObserver.idl: * page/PerformanceObserverCallback.h: * page/PerformanceObserverCallback.idl: LayoutTests: * performance-api/performance-observer-callback-after-gc-expected.txt: Added. * performance-api/performance-observer-callback-after-gc.html: Added. Add layout test to make sure that a performance observer's callback still gets called, even if the JS does not keep the performance observer alive. * performance-api/performance-observer-no-document-leak-expected.txt: Added. * performance-api/performance-observer-no-document-leak.html: Added. * performance-api/resources/performance-observer-no-document-leak-frame.html: Added. Add layout test coverage to make sure the document does not leak if PerformanceObserver was used. Canonical link: https://commits.webkit.org/202150@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233053 268f45cc-cd09-0410-ab3c-d52691b4dbfc
2018-06-21 19:04:02 +00:00
</script>
<script src="../resources/js-test-post.js"></script>
</body>
</html>