- Revision
- 88601
- Author
- [email protected]
- Date
- 2011-06-11 09:37:59 -0700 (Sat, 11 Jun 2011)
Log Message
2011-06-10 Abhishek Arya <[email protected]>
Reviewed by Simon Fraser.
Null parent element sheet pointers in CSSMutableStyleDeclaration consumers
when removed from document, set them when reinserted into document.
https://bugs.webkit.org/show_bug.cgi?id=62230
When a HTMLBodyElement, StyledElement are removed from document,
we didn't clear out the parent pointers from their link, style declarations.
These parent pointers pointed to the document's element sheet which will
get removed when document is getting destroyed. It does make sense to
clear out parent pointers when we are getting removed from document and
readd them when we get inserted again.
Tests: fast/dom/body-link-decl-parent-crash.html
fast/dom/styled-inline-style-decl-parent-crash.html
* dom/StyledElement.cpp:
(WebCore::StyledElement::insertedIntoDocument):
(WebCore::StyledElement::removedFromDocument):
* dom/StyledElement.h:
* html/HTMLBodyElement.cpp:
(WebCore::HTMLBodyElement::parseMappedAttribute):
(WebCore::HTMLBodyElement::insertedIntoDocument):
(WebCore::HTMLBodyElement::removedFromDocument):
(WebCore::HTMLBodyElement::didMoveToNewOwnerDocument):
* html/HTMLBodyElement.h:
2011-06-10 Abhishek Arya <[email protected]>
Reviewed by Simon Fraser.
Tests that accessing the parent element sheet of an inline style, link
declaration of styled, body elements which are removed from document,
does not result in crash.
https://bugs.webkit.org/show_bug.cgi?id=62230
* fast/dom/body-link-decl-parent-crash-expected.txt: Added.
* fast/dom/body-link-decl-parent-crash.html: Added.
* fast/dom/styled-inline-style-decl-parent-crash-expected.txt: Added.
* fast/dom/styled-inline-style-decl-parent-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (88600 => 88601)
--- trunk/LayoutTests/ChangeLog 2011-06-11 06:45:02 UTC (rev 88600)
+++ trunk/LayoutTests/ChangeLog 2011-06-11 16:37:59 UTC (rev 88601)
@@ -1,3 +1,17 @@
+2011-06-10 Abhishek Arya <[email protected]>
+
+ Reviewed by Simon Fraser.
+
+ Tests that accessing the parent element sheet of an inline style, link
+ declaration of styled, body elements which are removed from document,
+ does not result in crash.
+ https://bugs.webkit.org/show_bug.cgi?id=62230
+
+ * fast/dom/body-link-decl-parent-crash-expected.txt: Added.
+ * fast/dom/body-link-decl-parent-crash.html: Added.
+ * fast/dom/styled-inline-style-decl-parent-crash-expected.txt: Added.
+ * fast/dom/styled-inline-style-decl-parent-crash.html: Added.
+
2011-06-10 Ryosuke Niwa <[email protected]>
Skip inspector/profiler/cpu-profiler-profiling-without-inspector.html and
Added: trunk/LayoutTests/fast/dom/body-link-decl-parent-crash-expected.txt (0 => 88601)
--- trunk/LayoutTests/fast/dom/body-link-decl-parent-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/body-link-decl-parent-crash-expected.txt 2011-06-11 16:37:59 UTC (rev 88601)
@@ -0,0 +1,5 @@
+Test passes if it does not crash.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/body-link-decl-parent-crash.html (0 => 88601)
--- trunk/LayoutTests/fast/dom/body-link-decl-parent-crash.html (rev 0)
+++ trunk/LayoutTests/fast/dom/body-link-decl-parent-crash.html 2011-06-11 16:37:59 UTC (rev 88601)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+Test passes if it does not crash.
+<div id="console"></div>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+iframe1 = document.createElement('iframe');
+document.body.appendChild(iframe1);
+document1 = iframe1.contentDocument.implementation.createHTMLDocument("document");
+var body1 = document1.body;
+document1.alinkColor = "blue";
+document1.body = document1.createElement('body');
+delete document1;
+gc();
+body1.vLink = 1;
+
+var successfullyParsed = true;
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/dom/styled-inline-style-decl-parent-crash-expected.txt (0 => 88601)
--- trunk/LayoutTests/fast/dom/styled-inline-style-decl-parent-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/styled-inline-style-decl-parent-crash-expected.txt 2011-06-11 16:37:59 UTC (rev 88601)
@@ -0,0 +1,5 @@
+Test passes if it does not crash.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/styled-inline-style-decl-parent-crash.html (0 => 88601)
--- trunk/LayoutTests/fast/dom/styled-inline-style-decl-parent-crash.html (rev 0)
+++ trunk/LayoutTests/fast/dom/styled-inline-style-decl-parent-crash.html 2011-06-11 16:37:59 UTC (rev 88601)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+Test passes if it does not crash.
+<div id="console"></div>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+iframe1 = document.createElement('iframe');
+document.body.appendChild(iframe1);
+document1 = iframe1.contentDocument.implementation.createHTMLDocument("document");
+var div1 = document1.createElement('div');
+document1.body.appendChild(div1);
+div1.style.color = "blue";
+document1.body.removeChild(div1);
+delete document1;
+gc();
+div1.style.color = "red";
+
+var successfullyParsed = true;
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (88600 => 88601)
--- trunk/Source/WebCore/ChangeLog 2011-06-11 06:45:02 UTC (rev 88600)
+++ trunk/Source/WebCore/ChangeLog 2011-06-11 16:37:59 UTC (rev 88601)
@@ -1,3 +1,32 @@
+2011-06-10 Abhishek Arya <[email protected]>
+
+ Reviewed by Simon Fraser.
+
+ Null parent element sheet pointers in CSSMutableStyleDeclaration consumers
+ when removed from document, set them when reinserted into document.
+ https://bugs.webkit.org/show_bug.cgi?id=62230
+
+ When a HTMLBodyElement, StyledElement are removed from document,
+ we didn't clear out the parent pointers from their link, style declarations.
+ These parent pointers pointed to the document's element sheet which will
+ get removed when document is getting destroyed. It does make sense to
+ clear out parent pointers when we are getting removed from document and
+ readd them when we get inserted again.
+
+ Tests: fast/dom/body-link-decl-parent-crash.html
+ fast/dom/styled-inline-style-decl-parent-crash.html
+
+ * dom/StyledElement.cpp:
+ (WebCore::StyledElement::insertedIntoDocument):
+ (WebCore::StyledElement::removedFromDocument):
+ * dom/StyledElement.h:
+ * html/HTMLBodyElement.cpp:
+ (WebCore::HTMLBodyElement::parseMappedAttribute):
+ (WebCore::HTMLBodyElement::insertedIntoDocument):
+ (WebCore::HTMLBodyElement::removedFromDocument):
+ (WebCore::HTMLBodyElement::didMoveToNewOwnerDocument):
+ * html/HTMLBodyElement.h:
+
2011-06-10 Adam Barth <[email protected]>
Remove bogus ASSERTs. These ASSERTs used to be correct before I
Modified: trunk/Source/WebCore/dom/StyledElement.cpp (88600 => 88601)
--- trunk/Source/WebCore/dom/StyledElement.cpp 2011-06-11 06:45:02 UTC (rev 88600)
+++ trunk/Source/WebCore/dom/StyledElement.cpp 2011-06-11 16:37:59 UTC (rev 88601)
@@ -439,7 +439,22 @@
style->addSubresourceStyleURLs(urls);
}
+void StyledElement::insertedIntoDocument()
+{
+ Element::insertedIntoDocument();
+ if (m_inlineStyleDecl)
+ m_inlineStyleDecl->setParent(document()->elementSheet());
+}
+
+void StyledElement::removedFromDocument()
+{
+ if (m_inlineStyleDecl)
+ m_inlineStyleDecl->setParent(0);
+
+ Element::removedFromDocument();
+}
+
void StyledElement::didMoveToNewOwnerDocument()
{
if (m_inlineStyleDecl)
Modified: trunk/Source/WebCore/dom/StyledElement.h (88600 => 88601)
--- trunk/Source/WebCore/dom/StyledElement.h 2011-06-11 06:45:02 UTC (rev 88600)
+++ trunk/Source/WebCore/dom/StyledElement.h 2011-06-11 16:37:59 UTC (rev 88601)
@@ -84,6 +84,8 @@
// svgAttributeChanged (called when element.className.baseValue is set)
void classAttributeChanged(const AtomicString& newClassString);
+ virtual void insertedIntoDocument();
+ virtual void removedFromDocument();
virtual void didMoveToNewOwnerDocument();
private:
Modified: trunk/Source/WebCore/html/HTMLBodyElement.cpp (88600 => 88601)
--- trunk/Source/WebCore/html/HTMLBodyElement.cpp 2011-06-11 06:45:02 UTC (rev 88600)
+++ trunk/Source/WebCore/html/HTMLBodyElement.cpp 2011-06-11 16:37:59 UTC (rev 88601)
@@ -116,6 +116,13 @@
} else if (attr->name() == vlinkAttr ||
attr->name() == alinkAttr ||
attr->name() == linkAttr) {
+ // This tells us that we are removed from document. If our document is later destroyed
+ // (not deleted since we hold a guardRef), our stylesheet list will be null causing a crash
+ // later in document()->styleSelector(). So, we bail out early because we shouldn't be
+ // modifying anything in that document. See webkit bug 62230.
+ if (m_linkDecl && !m_linkDecl->parent())
+ return;
+
if (attr->isNull()) {
if (attr->name() == linkAttr)
document()->resetLinkColor();
@@ -202,8 +209,27 @@
if (document() && document()->page())
document()->page()->updateViewportArguments();
+
+ if (m_linkDecl)
+ m_linkDecl->setParent(document()->elementSheet());
}
+void HTMLBodyElement::removedFromDocument()
+{
+ if (m_linkDecl)
+ m_linkDecl->setParent(0);
+
+ HTMLElement::removedFromDocument();
+}
+
+void HTMLBodyElement::didMoveToNewOwnerDocument()
+{
+ if (m_linkDecl)
+ m_linkDecl->setParent(document()->elementSheet());
+
+ HTMLElement::didMoveToNewOwnerDocument();
+}
+
bool HTMLBodyElement::isURLAttribute(Attribute *attr) const
{
return attr->name() == backgroundAttr;
@@ -345,16 +371,4 @@
addSubresourceURL(urls, document()->completeURL(getAttribute(backgroundAttr)));
}
-void HTMLBodyElement::didMoveToNewOwnerDocument()
-{
- // When moving body elements between documents, we should have to reset the parent sheet for any
- // link style declarations. If we don't we might crash later.
- // In practice I can't reproduce this theoretical problem.
- // webarchive/adopt-attribute-styled-body-webarchive.html tries to make sure this crash won't surface.
- if (m_linkDecl)
- m_linkDecl->setParent(document()->elementSheet());
-
- HTMLElement::didMoveToNewOwnerDocument();
-}
-
} // namespace WebCore
Modified: trunk/Source/WebCore/html/HTMLBodyElement.h (88600 => 88601)
--- trunk/Source/WebCore/html/HTMLBodyElement.h 2011-06-11 06:45:02 UTC (rev 88600)
+++ trunk/Source/WebCore/html/HTMLBodyElement.h 2011-06-11 16:37:59 UTC (rev 88601)
@@ -74,6 +74,8 @@
virtual void parseMappedAttribute(Attribute*);
virtual void insertedIntoDocument();
+ virtual void removedFromDocument();
+ virtual void didMoveToNewOwnerDocument();
void createLinkDecl();
@@ -91,8 +93,6 @@
virtual int scrollWidth() const;
virtual void addSubresourceAttributeURLs(ListHashSet<KURL>&) const;
-
- virtual void didMoveToNewOwnerDocument();
RefPtr<CSSMutableStyleDeclaration> m_linkDecl;
};
