Crash accessing font fact rule parent

Chris Evans · Chris Evans · commit 9f364ee87abf · 2011-11-09T04:17:42.000Z
https://bugs.webkit.org/show_bug.cgi?id=71860 Reviewed by Adam Barth. Source/WebCore: Test: fast/css/css-fontface-rule-crash.html * css/CSSFontFaceRule.cpp: (WebCore::CSSFontFaceRule::~CSSFontFaceRule): tell our child rule when we are going away. LayoutTests: * fast/css/css-fontface-rule-crash-expected.txt: Added. * fast/css/css-fontface-rule-crash.html: Added. * resources/gc.js: Added. Add a re-usable best-of-breed gc(). Canonical link: https://commits.webkit.org/88216@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@99649 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2011-11-08  Chris Evans  <cevans@google.com>
+
+        Crash accessing font fact rule parent
+        https://bugs.webkit.org/show_bug.cgi?id=71860
+
+        Reviewed by Adam Barth.
+
+        * fast/css/css-fontface-rule-crash-expected.txt: Added.
+        * fast/css/css-fontface-rule-crash.html: Added.
+        * resources/gc.js: Added. Add a re-usable best-of-breed gc().
+
 2011-11-08  Brent Fulgham  <bfulgham@webkit.org>
 
         [WinCairo] Unreviewed skiplist update to match main Windows
diff --git a/LayoutTests/fast/css/css-fontface-rule-crash-expected.txt b/LayoutTests/fast/css/css-fontface-rule-crash-expected.txt
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/fast/css/css-fontface-rule-crash.html b/LayoutTests/fast/css/css-fontface-rule-crash.html
@@ -0,0 +1,37 @@
+<html>
+<head>
+<script src="../../resources/gc.js"></script>
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+function load()
+{
+    style = document.createElement('style');
+    style.textContent = '@font-face { font-family: "A"; }';
+    document.head.appendChild(style);
+    rulestyle = document.styleSheets[0].cssRules[0].style;
+    document.head.removeChild(style);
+    style = null;
+    setTimeout(crash, 0);
+}
+
+function crash()
+{
+    gc();
+    obj = rulestyle.parentRule;
+    // If the gc() actually successfully reaps everything it can, then obj
+    // will end up null (post-fix). gc() is not guaranteed to reap the font-face
+    // rule, however, particularly in the browser context.
+    if (obj)
+        obj = obj.foo;
+    document.body.innerText = 'PASS';
+    if (window.layoutTestController)
+        layoutTestController.notifyDone()
+}
+</script>
+</head>
+<body onload="load()"></body>
+</html>
diff --git a/LayoutTests/resources/gc.js b/LayoutTests/resources/gc.js
@@ -0,0 +1,20 @@
+// If there is no window.gc() already defined, define one using the best
+// method we can find.
+// The slow fallback should not hit in the actual test environment.
+if (!window.gc)
+{
+    window.gc = function()
+    {
+        if (window.GCController)
+            return GCController.collect();
+        function gcRec(n) {
+            if (n < 1)
+                return {};
+            var temp = {i: "ab" + i + (i / 100000)};
+            temp += "foo";
+            gcRec(n-1);
+        }
+        for (var i = 0; i < 10000; i++)
+            gcRec(10);
+    }
+}
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,15 @@
+2011-11-08  Chris Evans  <cevans@google.com>
+
+        Crash accessing font fact rule parent
+        https://bugs.webkit.org/show_bug.cgi?id=71860
+
+        Reviewed by Adam Barth.
+
+        Test: fast/css/css-fontface-rule-crash.html
+
+        * css/CSSFontFaceRule.cpp:
+        (WebCore::CSSFontFaceRule::~CSSFontFaceRule): tell our child rule when we are going away.
+
 2011-11-08  Adam Klein  <adamk@chromium.org>
 
         Use a typedef for ExceptionCode in all header files instead of including ExceptionCode.h
diff --git a/Source/WebCore/css/CSSFontFaceRule.cpp b/Source/WebCore/css/CSSFontFaceRule.cpp
@@ -33,6 +33,8 @@ CSSFontFaceRule::CSSFontFaceRule(CSSStyleSheet* parent)
 
 CSSFontFaceRule::~CSSFontFaceRule()
 {
+    if (m_style)
+        m_style->setParentRule(0);
 }
 
 String CSSFontFaceRule::cssText() const

Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,8 @@ CSSFontFaceRule::CSSFontFaceRule(CSSStyleSheet* parent)`
`33`	`33`
`34`	`34`	`CSSFontFaceRule::~CSSFontFaceRule()`
`35`	`35`	`{`
	`36`	`+ if (m_style)`
	`37`	`+ m_style->setParentRule(0);`
`36`	`38`	`}`
`37`	`39`
`38`	`40`	`String CSSFontFaceRule::cssText() const`