Title: [124631] trunk
- Revision
- 124631
- Author
- [email protected]
- Date
- 2012-08-03 11:34:27 -0700 (Fri, 03 Aug 2012)
Log Message
Crash when a clip path referencing a clip path changes documents
https://bugs.webkit.org/show_bug.cgi?id=93023
Reviewed by Dirk Schulze.
Source/WebCore:
The SVGClipPathElement is set to not need pending resource handling,
when in fact it can have pending resources. The result is a crash when
the element is moved to a new document (which deletes all resources
and leaves them pending) and then immediately deleted (which asserts
that there are no pending resources). There is code to remove pending
resources upon deletion and removal from the DOM, but it was not
executing for clips because of the aforementioned code claiming that
clips don't require such handling.
The assertion that there be no pending resources is necessary to
prevent caches of pending resources from trying to access the deleted
element.
This change removes the check for needsPendingResourceHandling in
SVGStyledElement upon deletion and removal from the DOM. Pending resources
will always be checked in such cases to ensure we do not introduce
security issues.
Test: svg/custom/clip-path-document-change-assert.html
* svg/SVGStyledElement.cpp:
(WebCore::SVGStyledElement::~SVGStyledElement): Removed needsPendingResourceHandling in the conditional to clean up resources.
(WebCore::SVGStyledElement::removedFrom): Removed needsPendingResourceHandling in the conditional to clean up resources.
LayoutTests:
Test that asserts in debug DRT without this change. Any attempt to
delete a clip that references another clip after changing the document
results in a crash.
* svg/custom/clip-path-document-change-assert-expected.txt: Added.
* svg/custom/clip-path-document-change-assert.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (124630 => 124631)
--- trunk/LayoutTests/ChangeLog 2012-08-03 18:23:12 UTC (rev 124630)
+++ trunk/LayoutTests/ChangeLog 2012-08-03 18:34:27 UTC (rev 124631)
@@ -1,3 +1,17 @@
+2012-08-03 Stephen Chenney <[email protected]>
+
+ Crash when a clip path referencing a clip path changes documents
+ https://bugs.webkit.org/show_bug.cgi?id=93023
+
+ Reviewed by Dirk Schulze.
+
+ Test that asserts in debug DRT without this change. Any attempt to
+ delete a clip that references another clip after changing the document
+ results in a crash.
+
+ * svg/custom/clip-path-document-change-assert-expected.txt: Added.
+ * svg/custom/clip-path-document-change-assert.html: Added.
+
2012-07-20 Jon Lee <[email protected]>
Crash in Notification when setting a non-object as an event listener (91881)
Added: trunk/LayoutTests/svg/custom/clip-path-document-change-assert-expected.txt (0 => 124631)
--- trunk/LayoutTests/svg/custom/clip-path-document-change-assert-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/clip-path-document-change-assert-expected.txt 2012-08-03 18:34:27 UTC (rev 124631)
@@ -0,0 +1 @@
+PASS
Added: trunk/LayoutTests/svg/custom/clip-path-document-change-assert.html (0 => 124631)
--- trunk/LayoutTests/svg/custom/clip-path-document-change-assert.html (rev 0)
+++ trunk/LayoutTests/svg/custom/clip-path-document-change-assert.html 2012-08-03 18:34:27 UTC (rev 124631)
@@ -0,0 +1,39 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+ <head>
+ </head>
+ <body id="bodyRoot">
+ <svg xmlns="http://www.w3.org/2000/svg">
+ <defs>
+ <clipPath id="clipClip">
+ <rect>
+ </rect>
+ </clipPath>
+ <clipPath clip-path="url(#clipClip)">
+ <circle>
+ </circle>
+ </clipPath>
+ </defs>
+ <rect>
+ </rect>
+ </svg>
+ </body>
+ <script>
+ if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+ }
+
+ document.addEventListener("DOMContentLoaded", initCrash, false);
+
+ function initCrash() {
+ var bodyRoot = document.getElementById("bodyRoot");
+ try { document.implementation.createDocument("", "", null).adoptNode(bodyRoot); } catch(e) {}
+ try { bodyRoot.textContent = "" } catch(e) {}
+ document.documentElement.innerHTML = "PASS";
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+ </script>
+</html>
+
Modified: trunk/Source/WebCore/ChangeLog (124630 => 124631)
--- trunk/Source/WebCore/ChangeLog 2012-08-03 18:23:12 UTC (rev 124630)
+++ trunk/Source/WebCore/ChangeLog 2012-08-03 18:34:27 UTC (rev 124631)
@@ -1,3 +1,34 @@
+2012-08-03 Stephen Chenney <[email protected]>
+
+ Crash when a clip path referencing a clip path changes documents
+ https://bugs.webkit.org/show_bug.cgi?id=93023
+
+ Reviewed by Dirk Schulze.
+
+ The SVGClipPathElement is set to not need pending resource handling,
+ when in fact it can have pending resources. The result is a crash when
+ the element is moved to a new document (which deletes all resources
+ and leaves them pending) and then immediately deleted (which asserts
+ that there are no pending resources). There is code to remove pending
+ resources upon deletion and removal from the DOM, but it was not
+ executing for clips because of the aforementioned code claiming that
+ clips don't require such handling.
+
+ The assertion that there be no pending resources is necessary to
+ prevent caches of pending resources from trying to access the deleted
+ element.
+
+ This change removes the check for needsPendingResourceHandling in
+ SVGStyledElement upon deletion and removal from the DOM. Pending resources
+ will always be checked in such cases to ensure we do not introduce
+ security issues.
+
+ Test: svg/custom/clip-path-document-change-assert.html
+
+ * svg/SVGStyledElement.cpp:
+ (WebCore::SVGStyledElement::~SVGStyledElement): Removed needsPendingResourceHandling in the conditional to clean up resources.
+ (WebCore::SVGStyledElement::removedFrom): Removed needsPendingResourceHandling in the conditional to clean up resources.
+
2012-08-03 Kentaro Hara <[email protected]>
[V8] Remove unused methods in V8Proxy
Modified: trunk/Source/WebCore/svg/SVGStyledElement.cpp (124630 => 124631)
--- trunk/Source/WebCore/svg/SVGStyledElement.cpp 2012-08-03 18:23:12 UTC (rev 124630)
+++ trunk/Source/WebCore/svg/SVGStyledElement.cpp 2012-08-03 18:34:27 UTC (rev 124631)
@@ -76,7 +76,7 @@
SVGStyledElement::~SVGStyledElement()
{
- if (needsPendingResourceHandling() && hasPendingResources() && document())
+ if (hasPendingResources() && document())
document()->accessSVGExtensions()->removeElementFromPendingResources(this);
ASSERT(!hasPendingResources());
@@ -389,7 +389,7 @@
SVGElement::removedFrom(rootParent);
SVGElementInstance::invalidateAllInstancesOfElement(this);
Document* document = this->document();
- if (!rootParent->inDocument() || !needsPendingResourceHandling() || !document)
+ if (!rootParent->inDocument() || !document)
return;
document->accessSVGExtensions()->removeElementFromPendingResources(this);
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
