- Revision
- 125988
- Author
- morr...@google.com
- Date
- 2012-08-19 19:56:18 -0700 (Sun, 19 Aug 2012)
Log Message
DOM mutation against including <link> shouldn't trigger pending HTML parser.
https://bugs.webkit.org/show_bug.cgi?id=93641
Reviewed by Ryosuke Niwa.
Source/WebCore:
HTMLLinkElement::removedFrom() invoked Document::removePendingSheet(), which can trigger
HTMLParser that can mutate DOM tree. DOM mutation reentrancy on like this is problematic and
should be prohibited.
This change add an variation of Document::removePendingSheet() which postpones the notification
which triggers DOM mutation, and flush such pending notifications at the end of ongoing mutation.
Test: http/tests/loading/remove-child-triggers-parser.html
* dom/ContainerNodeAlgorithms.h:
(WebCore::ChildNodeRemovalNotifier::notify): Flushed pending notifications at the end.
* dom/Document.cpp:
(WebCore::Document::Document):
(WebCore::Document::removePendingSheet): Added RemovePendingSheetNotificationType parameter.
(WebCore):
(WebCore::Document::didRemoveAllPendingStylesheet): Extracted from removePendingSheet()
* dom/Document.h:
(Document):
(WebCore::Document::setNeedsNotifyRemoveAllPendingStylesheet): A flag setter.
(WebCore::Document::notifyRemovePendingSheetIfNeeded):
(WebCore):
* html/HTMLLinkElement.cpp:
(WebCore::HTMLLinkElement::removedFrom): Switched to use "notification later" version of removePendingSheet()
(WebCore::HTMLLinkElement::removePendingSheet): Added RemovePendingSheetNotificationType parameter.
* html/HTMLLinkElement.h:
LayoutTests:
Note that the test content need to be such cryptic because HTML parser is involving the
captured bug and adding explanations can affect the behavior then mask the bug.
* http/tests/loading/remove-child-triggers-parser-expected.txt: Added.
* http/tests/loading/remove-child-triggers-parser.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (125987 => 125988)
--- trunk/LayoutTests/ChangeLog 2012-08-20 02:30:04 UTC (rev 125987)
+++ trunk/LayoutTests/ChangeLog 2012-08-20 02:56:18 UTC (rev 125988)
@@ -1,3 +1,16 @@
+2012-08-19 MORITA Hajime <morr...@google.com>
+
+ DOM mutation against including <link> shouldn't trigger pending HTML parser.
+ https://bugs.webkit.org/show_bug.cgi?id=93641
+
+ Reviewed by Ryosuke Niwa.
+
+ Note that the test content need to be such cryptic because HTML parser is involving the
+ captured bug and adding explanations can affect the behavior then mask the bug.
+
+ * http/tests/loading/remove-child-triggers-parser-expected.txt: Added.
+ * http/tests/loading/remove-child-triggers-parser.html: Added.
+
2012-08-19 KwangYong Choi <ky0.c...@samsung.com>
[EFL] Gardening for fast/forms/range/thumbslider-no-parent-slider.html
Added: trunk/LayoutTests/http/tests/loading/remove-child-triggers-parser-expected.txt (0 => 125988)
--- trunk/LayoutTests/http/tests/loading/remove-child-triggers-parser-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/loading/remove-child-triggers-parser-expected.txt 2012-08-20 02:56:18 UTC (rev 125988)
@@ -0,0 +1,6 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+main frame - didFinishLoadForFrame
+
Added: trunk/LayoutTests/http/tests/loading/remove-child-triggers-parser.html (0 => 125988)
--- trunk/LayoutTests/http/tests/loading/remove-child-triggers-parser.html (rev 0)
+++ trunk/LayoutTests/http/tests/loading/remove-child-triggers-parser.html 2012-08-20 02:56:18 UTC (rev 125988)
@@ -0,0 +1,18 @@
+<script></script>
+<!-- This test covers Bug 93641 -->
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+setTimeout(function() { var child = document.documentElement; child.parentNode.removeChild(child); }, 36);
+</script>
+<script></script>
+<span></span>
+<div></div>
+<span></span>
+<nobr>
+<span></span>
+<div>
+ <link rel="stylesheet" href=""
+ <script>;</script>
+ <nobr>
+</div>
Modified: trunk/Source/WebCore/ChangeLog (125987 => 125988)
--- trunk/Source/WebCore/ChangeLog 2012-08-20 02:30:04 UTC (rev 125987)
+++ trunk/Source/WebCore/ChangeLog 2012-08-20 02:56:18 UTC (rev 125988)
@@ -1,3 +1,36 @@
+2012-08-19 MORITA Hajime <morr...@google.com>
+
+ DOM mutation against including <link> shouldn't trigger pending HTML parser.
+ https://bugs.webkit.org/show_bug.cgi?id=93641
+
+ Reviewed by Ryosuke Niwa.
+
+ HTMLLinkElement::removedFrom() invoked Document::removePendingSheet(), which can trigger
+ HTMLParser that can mutate DOM tree. DOM mutation reentrancy on like this is problematic and
+ should be prohibited.
+
+ This change add an variation of Document::removePendingSheet() which postpones the notification
+ which triggers DOM mutation, and flush such pending notifications at the end of ongoing mutation.
+
+ Test: http/tests/loading/remove-child-triggers-parser.html
+
+ * dom/ContainerNodeAlgorithms.h:
+ (WebCore::ChildNodeRemovalNotifier::notify): Flushed pending notifications at the end.
+ * dom/Document.cpp:
+ (WebCore::Document::Document):
+ (WebCore::Document::removePendingSheet): Added RemovePendingSheetNotificationType parameter.
+ (WebCore):
+ (WebCore::Document::didRemoveAllPendingStylesheet): Extracted from removePendingSheet()
+ * dom/Document.h:
+ (Document):
+ (WebCore::Document::setNeedsNotifyRemoveAllPendingStylesheet): A flag setter.
+ (WebCore::Document::notifyRemovePendingSheetIfNeeded):
+ (WebCore):
+ * html/HTMLLinkElement.cpp:
+ (WebCore::HTMLLinkElement::removedFrom): Switched to use "notification later" version of removePendingSheet()
+ (WebCore::HTMLLinkElement::removePendingSheet): Added RemovePendingSheetNotificationType parameter.
+ * html/HTMLLinkElement.h:
+
2012-08-19 Kentaro Hara <hara...@chromium.org>
Remove RefPtr from HTMLProgressElement::m_value
Modified: trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h (125987 => 125988)
--- trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h 2012-08-20 02:30:04 UTC (rev 125987)
+++ trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h 2012-08-20 02:56:18 UTC (rev 125988)
@@ -260,9 +260,10 @@
inline void ChildNodeRemovalNotifier::notify(Node* node)
{
- if (node->inDocument())
+ if (node->inDocument()) {
notifyNodeRemovedFromDocument(node);
- else if (node->isContainerNode())
+ node->document()->notifyRemovePendingSheetIfNeeded();
+ } else if (node->isContainerNode())
notifyNodeRemovedFromTree(toContainerNode(node));
}
Modified: trunk/Source/WebCore/dom/Document.cpp (125987 => 125988)
--- trunk/Source/WebCore/dom/Document.cpp 2012-08-20 02:30:04 UTC (rev 125987)
+++ trunk/Source/WebCore/dom/Document.cpp 2012-08-20 02:56:18 UTC (rev 125988)
@@ -553,6 +553,7 @@
m_hasDirtyStyleResolver = false;
m_pendingStylesheets = 0;
m_ignorePendingStylesheets = false;
+ m_needsNotifyRemoveAllPendingStylesheet = false;
m_hasNodesWithPlaceholderStyle = false;
m_pendingSheetLayout = NoLayoutWithPendingSheets;
@@ -3313,7 +3314,7 @@
}
// This method is called whenever a top-level stylesheet has finished loading.
-void Document::removePendingSheet()
+void Document::removePendingSheet(RemovePendingSheetNotificationType notification)
{
// Make sure we knew this sheet was pending, and that our count isn't out of sync.
ASSERT(m_pendingStylesheets > 0);
@@ -3328,6 +3329,18 @@
if (m_pendingStylesheets)
return;
+ if (notification == RemovePendingSheetNotifyLater) {
+ setNeedsNotifyRemoveAllPendingStylesheet();
+ return;
+ }
+
+ didRemoveAllPendingStylesheet();
+}
+
+void Document::didRemoveAllPendingStylesheet()
+{
+ m_needsNotifyRemoveAllPendingStylesheet = false;
+
styleResolverChanged(RecalcStyleIfNeeded);
if (ScriptableDocumentParser* parser = scriptableDocumentParser())
@@ -3337,6 +3350,7 @@
view()->scrollToFragment(m_url);
}
+
void Document::evaluateMediaQueryList()
{
if (m_mediaQueryMatcher)
Modified: trunk/Source/WebCore/dom/Document.h (125987 => 125988)
--- trunk/Source/WebCore/dom/Document.h 2012-08-20 02:30:04 UTC (rev 125987)
+++ trunk/Source/WebCore/dom/Document.h 2012-08-20 02:56:18 UTC (rev 125988)
@@ -480,8 +480,14 @@
/**
* Updates the pending sheet count and then calls updateActiveStylesheets.
*/
- void removePendingSheet();
+ enum RemovePendingSheetNotificationType {
+ RemovePendingSheetNotifyImmediately,
+ RemovePendingSheetNotifyLater
+ };
+ void removePendingSheet(RemovePendingSheetNotificationType = RemovePendingSheetNotifyImmediately);
+ void notifyRemovePendingSheetIfNeeded();
+
/**
* This method returns true if all top-level stylesheets have loaded (including
* any @imports that they may be loading).
@@ -1230,6 +1236,8 @@
void collectActiveStylesheets(Vector<RefPtr<StyleSheet> >&);
bool testAddedStylesheetRequiresStyleRecalc(StyleSheetContents*);
void analyzeStylesheetChange(StyleResolverUpdateFlag, const Vector<RefPtr<StyleSheet> >& newStylesheets, bool& requiresStyleResolverReset, bool& requiresFullStyleRecalc);
+ void didRemoveAllPendingStylesheet();
+ void setNeedsNotifyRemoveAllPendingStylesheet() { m_needsNotifyRemoveAllPendingStylesheet = true; }
void seamlessParentUpdatedStylesheets();
void notifySeamlessChildDocumentsOfStylesheetUpdate() const;
@@ -1307,7 +1315,8 @@
// But sometimes you need to ignore pending stylesheet count to
// force an immediate layout when requested by JS.
- bool m_ignorePendingStylesheets;
+ bool m_ignorePendingStylesheets : 1;
+ bool m_needsNotifyRemoveAllPendingStylesheet : 1;
// If we do ignore the pending stylesheet count, then we need to add a boolean
// to track that this happened so that we can do a full repaint when the stylesheets
@@ -1561,6 +1570,12 @@
#endif
};
+inline void Document::notifyRemovePendingSheetIfNeeded()
+{
+ if (m_needsNotifyRemoveAllPendingStylesheet)
+ didRemoveAllPendingStylesheet();
+}
+
// Put these methods here, because they require the Document definition, but we really want to inline them.
inline bool Node::isDocumentNode() const
Modified: trunk/Source/WebCore/html/HTMLLinkElement.cpp (125987 => 125988)
--- trunk/Source/WebCore/html/HTMLLinkElement.cpp 2012-08-20 02:30:04 UTC (rev 125987)
+++ trunk/Source/WebCore/html/HTMLLinkElement.cpp 2012-08-20 02:56:18 UTC (rev 125988)
@@ -278,7 +278,7 @@
clearSheet();
if (styleSheetIsLoading())
- removePendingSheet();
+ removePendingSheet(RemovePendingSheetNotifyLater);
if (document()->renderer())
document()->styleResolverChanged(DeferRecalcStyle);
@@ -458,7 +458,7 @@
document()->addPendingSheet();
}
-void HTMLLinkElement::removePendingSheet()
+void HTMLLinkElement::removePendingSheet(RemovePendingSheetNotificationType notification)
{
PendingSheetType type = m_pendingSheetType;
m_pendingSheetType = None;
@@ -470,7 +470,11 @@
document()->styleResolverChanged(RecalcStyleImmediately);
return;
}
- document()->removePendingSheet();
+
+ document()->removePendingSheet(
+ notification == RemovePendingSheetNotifyImmediately
+ ? Document::RemovePendingSheetNotifyImmediately
+ : Document::RemovePendingSheetNotifyLater);
}
DOMSettableTokenList* HTMLLinkElement::sizes() const
Modified: trunk/Source/WebCore/html/HTMLLinkElement.h (125987 => 125988)
--- trunk/Source/WebCore/html/HTMLLinkElement.h 2012-08-20 02:30:04 UTC (rev 125987)
+++ trunk/Source/WebCore/html/HTMLLinkElement.h 2012-08-20 02:56:18 UTC (rev 125988)
@@ -105,8 +105,14 @@
enum PendingSheetType { None, NonBlocking, Blocking };
void addPendingSheet(PendingSheetType);
- void removePendingSheet();
+ enum RemovePendingSheetNotificationType {
+ RemovePendingSheetNotifyImmediately,
+ RemovePendingSheetNotifyLater
+ };
+
+ void removePendingSheet(RemovePendingSheetNotificationType = RemovePendingSheetNotifyImmediately);
+
#if ENABLE(MICRODATA)
virtual String itemValueText() const OVERRIDE;
virtual void setItemValueText(const String&, ExceptionCode&) OVERRIDE;
