Skip to content

Commit ba2060b

Browse files
rrenoemutavchi
rreno
authored and
emutavchi
committed
HTMLCanvasElement is orphaned causing a HTMLDocument leak on YouTube video pages
https://bugs.webkit.org/show_bug.cgi?id=256262 rdar://108845985 Reviewed by Darin Adler. CanvasRenderingContext2DBase has a State stack for setting up drawing state. Two of the fields - strokeStyle and fillStyle - each hold CanvasStyle objects. These objects may be set with colors or images or gradients. In the case of a CanvasGradient, the gradient has a strong reference to the context which creates a reference cycle. CanvasRenderingContext2DBase -> State -> CanvasStyle -> CanvasGradient -> CanvasRenderingContext(2DBase) What makes this cycle dangerous is the CanvasRenderingContext will increase the ref count of an HTMLCanvasElement which, being a Node, will increment the referencingNodeCount of a Document object. So if a gradient is set on the context's fillStyle or strokeStyle we can cause a Document leak if the state is never cleared like on YouTube video pages. This patch changes the CanvasGradient object to hold a weak reference to the CanvasRenderingContext instead of a strong reference which breaks the cycle. * LayoutTests/fast/canvas Add tests to verify the now-decoupled lifetimes of gradient and context do not cause crashes. * LayoutTests/fast/canvas/canvas-gradient-can-outlive-context-expected.txt: Added. * LayoutTests/fast/canvas/canvas-gradient-can-outlive-context.html: Added. * LayoutTests/fast/canvas/canvas-state-stack-gradient-expected.txt: Added. * LayoutTests/fast/canvas/canvas-state-stack-gradient.html: Added. * LayoutTests/http/tests/canvas Add a test to verify that setting the style of a 2D canvas context does not leak Documents. * LayoutTests/http/tests/canvas/ctx.2d-canvas-style-no-document-leak-expected.txt: Added. * LayoutTests/http/tests/canvas/ctx.2d-canvas-style-no-document-leak.html: Added. * LayoutTests/http/tests/canvas/resources/background.png: Added. * LayoutTests/http/tests/canvas/resources/ctx.2d-fillStyle-color.html: Added. * LayoutTests/http/tests/canvas/resources/ctx.2d-fillStyle-gradient.html: Added. * LayoutTests/http/tests/canvas/resources/ctx.2d-fillStyle-pattern.html: Added. * Source/WebCore/html/canvas/CanvasGradient.cpp: (WebCore::CanvasGradient::addColorStop): * Source/WebCore/html/canvas/CanvasGradient.h: * Source/WebCore/html/canvas/CanvasStyle.cpp: (WebCore::parseColor): Since the gradient can outlive the context that created it, we need to be able to parse colors without having a CSSParserContext reference. This overload calls CSSParser::parseColorWithoutContext in that case. * Source/WebCore/html/canvas/CanvasStyle.h: (WebCore::CanvasStyle::canvasGradient const): Canonical link: https://commits.webkit.org/263774@main
1 parent 59d63a5 commit ba2060b

14 files changed

Lines changed: 285 additions & 2 deletions

‎LayoutTests/fast/canvas/canvas-gradient-can-outlive-context-expected.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Test that a gradient can outlive a context. Test passes if it doesn't crash.
2+
3+
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
4+
5+
6+
PASS successfullyParsed is true
7+
8+
TEST COMPLETE
9+

‎LayoutTests/fast/canvas/canvas-gradient-can-outlive-context.html‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<!DOCTYPE html>
2+
<body>
3+
<script src='../../resources/js-test.js'></script>
4+
<script>
5+
description("Test that a gradient can outlive a context. Test passes if it doesn't crash.");
6+
jsTestIsAsync = true;
7+
var gradient;
8+
window.onload = function() {
9+
let context = document.createElement('canvas').getContext('2d');
10+
gradient = context.createLinearGradient(0, 0, 100, 100);
11+
gradient.addColorStop(0, 'red');
12+
gradient.addColorStop(0.25, 'white');
13+
gradient.addColorStop(0.67, 'white');
14+
gradient.addColorStop(1, 'aquamarine');
15+
context.scale(2,2);
16+
context.beginPath();
17+
context.rect(0,0,75,75);
18+
context.clip();
19+
context.fillStyle = gradient;
20+
context.fillRect(0,0,30,30);
21+
context.fillRect(50,50,30,30);
22+
context.save();
23+
context.scale(0.5,0.5);
24+
context.beginPath();
25+
context.rect(0,0,5,5);
26+
context.clip();
27+
requestAnimationFrame(() => forceGCThenModifyGradient());
28+
};
29+
30+
function forceGCThenModifyGradient() {
31+
count = 0;
32+
handle = setInterval(() => {
33+
gc();
34+
count++;
35+
if (count > 500) {
36+
clearInterval(handle);
37+
modifyGradient();
38+
}
39+
}, 10);
40+
}
41+
42+
// Create a new context but use the old gradient.
43+
function modifyGradient() {
44+
let context = document.createElement('canvas').getContext('2d');
45+
gradient.addColorStop(0.75, 'aquamarine');
46+
gradient.addColorStop(0.8, 'white');
47+
gradient.addColorStop(0.9, 'orange');
48+
if (window.testRunner)
49+
finishJSTest("PASS");
50+
}
51+
</script>
52+
</body>

‎LayoutTests/fast/canvas/canvas-state-stack-gradient-expected.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Test a gradient stored on the state stack does not get garbage collected. Test passes if it does not crash.
2+
3+
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
4+
5+
6+
PASS successfullyParsed is true
7+
8+
TEST COMPLETE
9+

‎LayoutTests/fast/canvas/canvas-state-stack-gradient.html‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<!DOCTYPE html>
2+
<body>
3+
<script src='../../resources/js-test.js'></script>
4+
<script>
5+
description("Test a gradient stored on the state stack does not get garbage collected. Test passes if it does not crash.");
6+
jsTestIsAsync = true;
7+
window.onload = function() {
8+
let context = document.getElementById('c').getContext('2d');
9+
if (true) {
10+
let gradient = context.createLinearGradient(0, 0, 75, 75);
11+
gradient.addColorStop(0, 'red');
12+
gradient.addColorStop(0.33, 'white');
13+
gradient.addColorStop(0.67, 'white');
14+
gradient.addColorStop(1, 'blue');
15+
context.scale(2,2);
16+
context.beginPath();
17+
context.rect(0,0,75,75);
18+
context.clip();
19+
context.fillStyle = gradient;
20+
context.fillRect(0,0,30,30);
21+
context.fillRect(50,50,30,30);
22+
context.save();
23+
}
24+
context.fillStyle = 'green';
25+
context.scale(0.5,0.5);
26+
context.beginPath();
27+
context.rect(0,0,5,5);
28+
context.clip();
29+
requestAnimationFrame(() => forceGC(context));
30+
};
31+
32+
function forceGC(ctx) {
33+
count = 0;
34+
handle = setInterval(() => {
35+
gc();
36+
count++;
37+
if (count > 500) {
38+
clearInterval(handle);
39+
drawSecondFrame(ctx);
40+
}
41+
}, 10);
42+
}
43+
44+
function drawSecondFrame(ctx) {
45+
ctx.restore();
46+
let gradient = ctx.fillStyle;
47+
// colors are type string, gradients and patterns are objects
48+
if (typeof gradient !== typeof {})
49+
testFailed("Should have restored a gradient from the state stack.");
50+
51+
ctx.fillRect(0,0,30,30);
52+
ctx.fillRect(75,75,30,30);
53+
if (window.testRunner) {
54+
finishJSTest("PASS");
55+
}
56+
}
57+
</script>
58+
<canvas id="c" width="300" height="300"></canvas>
59+
</body>

‎LayoutTests/http/tests/canvas/ctx.2d-canvas-style-no-document-leak-expected.txt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
Tests that canvas context fillStyle does not leak documents.
2+
3+
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
4+
5+
6+
PASS internals.isDocumentAlive(frameDocumentID) is true
7+
PASS internals.isDocumentAlive(frameDocumentID) is true
8+
PASS internals.isDocumentAlive(frameDocumentID) is true
9+
PASS The iframe document didn't leak.
10+
PASS successfullyParsed is true
11+
12+
TEST COMPLETE
13+

‎LayoutTests/http/tests/canvas/ctx.2d-canvas-style-no-document-leak.html‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<body>
4+
<iframe id="iframe"></iframe>
5+
<script src="../resources/js-test-pre.js"></script>
6+
<script>
7+
description("Tests that canvas context fillStyle does not leak documents.");
8+
9+
jsTestIsAsync = true;
10+
frameDocumentID = 0;
11+
checkCount = 0;
12+
13+
function iframeLoaded(frameDocument) {
14+
frameDocumentID = internals.documentIdentifier(frameDocument);
15+
shouldBeTrue("internals.isDocumentAlive(frameDocumentID)");
16+
17+
iframe.addEventListener("load", () => {
18+
handle = setInterval(() => {
19+
gc();
20+
if (!internals.isDocumentAlive(frameDocumentID)) {
21+
clearInterval(handle);
22+
testPassed("The iframe document didn't leak.");
23+
finishJSTest();
24+
}
25+
checkCount++;
26+
if (checkCount > 500) {
27+
clearInterval(handle);
28+
testFailed("The iframe document leaked.");
29+
finishJSTest();
30+
}
31+
}, 10);
32+
}, {once: true});
33+
34+
iframe.src = "about:blank";
35+
}
36+
37+
function test() {
38+
["color", "gradient", "pattern"].forEach(variant => {
39+
window.addEventListener("message", (message) => {
40+
if (message.data === "frameLoaded")
41+
iframeLoaded(iframe.contentWindow.document);
42+
}, {once: true});
43+
44+
iframe.src = `resources/ctx.2d-fillStyle-${variant}.html`;
45+
});
46+
}
47+
48+
onload = () => {
49+
if (!window.testRunner || !window.internals)
50+
testFailed("Test requires internals.");
51+
52+
test();
53+
};
54+
55+
</script>
56+
<script src="../resources/js-test-post.js"></script>
57+
</body>
58+
</html>

‎LayoutTests/http/tests/canvas/resources/background.png‎

86 Bytes
Loading

‎LayoutTests/http/tests/canvas/resources/ctx.2d-fillStyle-color.html‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<body>
4+
<script>
5+
onload = () => {
6+
var canvas = document.createElement('canvas');
7+
canvas.setAttribute('width', '640');
8+
canvas.setAttribute('height', '480');
9+
var ctx = canvas.getContext('2d');
10+
ctx.fillStyle = "aquamarine";
11+
ctx.fillRect(50, 50, 100, 100);
12+
13+
document.body.style.backgroundImage = `url(${canvas.toDataURL()})`;
14+
15+
parent.postMessage("frameLoaded");
16+
};
17+
</script>
18+
</body>
19+
</html>

‎LayoutTests/http/tests/canvas/resources/ctx.2d-fillStyle-gradient.html‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<body>
4+
<script>
5+
onload = () => {
6+
var canvas = document.createElement('canvas');
7+
canvas.setAttribute('width', '640');
8+
canvas.setAttribute('height', '480');
9+
var ctx = canvas.getContext('2d');
10+
var gradient = ctx.createLinearGradient(0, 0, 400, 400);
11+
12+
gradient.addColorStop(0, 'red');
13+
gradient.addColorStop(0.5, 'green');
14+
gradient.addColorStop(1, 'blue');
15+
16+
ctx.fillStyle = gradient;
17+
ctx.fillRect(50, 50, 100, 100);
18+
19+
document.body.style.backgroundImage = `url(${canvas.toDataURL()})`;
20+
21+
parent.postMessage("frameLoaded");
22+
};
23+
</script>
24+
</body>
25+
</html>

‎LayoutTests/http/tests/canvas/resources/ctx.2d-fillStyle-pattern.html‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<body>
4+
<script>
5+
onload = () => {
6+
var img = new Image();
7+
img.onload = () => {
8+
var canvas = document.createElement('canvas');
9+
canvas.setAttribute('width', '640');
10+
canvas.setAttribute('height', '480');
11+
var ctx = canvas.getContext('2d');
12+
ctx.fillStyle = ctx.createPattern(img, "repeat");
13+
ctx.fillRect(50, 50, 100, 100);
14+
15+
document.body.style.backgroundImage = `url(${canvas.toDataURL()})`;
16+
17+
parent.postMessage("frameLoaded");
18+
};
19+
img.src = "background.png";
20+
};
21+
</script>
22+
</body>
23+
</html>

0 commit comments

Comments
 (0)