Skip to content

GitLab

T
tangled
Commit e1127317 authored by rune@opera.com's avatar rune@opera.com
Browse files
Options

Fix crash when removing stylesheets from shadow tree. 

The added sheets passed into StyleSheetInvalidationAnalysis may either be
added or removed. When they are removed style elements from a shadow tree,
the style element may no longer have a containingShadowRoot if the element
has already been removed. That caused a crash when the containingShadowRoot
was accessed without a nullptr check.

Instead of deducing the scope from the style element, pass the stylesheet
collection TreeScope to the analysis object. The m_scopingNodes, which is
probably a left-over from <style scoped> has been removed.

Now, the shadow tree root node, or outermost shadow host if ::content
rules makes that necessary, is always marked sub-tree dirty. The previous
code-path could trigger id/class invalidation of separate elements when
the stylesheet contents had multiple owner nodes, but that was most likely
wrong if it could ever happen.

BUG=522860

Review URL: https://codereview.chromium.org/1313443003

git-svn-id: svn://svn.chromium.org/blink/trunk@201058 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 10584b73
Hide whitespace changes
Inline Side-by-side
Showing with 109 additions and 34 deletions
third_party/WebKit/LayoutTests/fast/css/invalidation/shadow-add-sheet-content-expected.txt 0 → 100644
View file @ e1127317
Insert a style element into a shadow tree affecting a distributed node.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS getComputedStyle(outerSpan).color is "rgb(255, 0, 0)"
PASS internals.updateStyleAndReturnAffectedElementCount() is 9
PASS getComputedStyle(outerSpan).color is "rgb(0, 128, 0)"
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/css/invalidation/shadow-add-sheet-content.html 0 → 100644
View file @ e1127317
<!DOCTYPE html>
<script src="../../../resources/js-test.js"></script>
<style>
#outerHost { color: red }
</style>
<div>
<div id="outerHost">
<span id="outerSpan"></span>
</div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
</div>
<script>
description("Insert a style element into a shadow tree affecting a distributed node.");
var outerRoot = outerHost.createShadowRoot();
outerRoot.innerHTML = "<div id='host1'><content/></div>";
var host1 = outerRoot.querySelector("#host1");
var root1 = host1.createShadowRoot();
root1.innerHTML = "<div id='host2'><content/></div>";
var host2 = root1.querySelector("#host2");
var root2 = host2.createShadowRoot();
root2.innerHTML = "<content/>";
shouldBeEqualToString("getComputedStyle(outerSpan).color", "rgb(255, 0, 0)");
var sheet = document.createElement("style");
sheet.appendChild(document.createTextNode("::content #outerSpan { color: green }"));
root2.appendChild(sheet);
if (window.internals)
shouldBe("internals.updateStyleAndReturnAffectedElementCount()", "9");
shouldBeEqualToString("getComputedStyle(outerSpan).color", "rgb(0, 128, 0)");
</script>
third_party/WebKit/LayoutTests/fast/dom/shadow/remove-styles-in-shadow-crash-3.html 0 → 100644
View file @ e1127317
<!DOCTYPE html>
<style></style>
<script>
if (window.testRunner)
testRunner.dumpAsText();
onload = function() {
var link = document.createElement("link");
link.setAttribute("rel", "stylesheet");
link.setAttribute("href", "file:notfound.css");
document.head.appendChild(link);
var root = host.createShadowRoot();
root.innerHTML = "<span><style></style><style></style>";
root.innerHTML = "";
}
</script>
<p>PASS if no crash</p>
<div id="host"></div>
third_party/WebKit/Source/core/css/invalidation/StyleSheetInvalidationAnalysis.cpp
View file @ e1127317
...@@ -32,13 +32,14 @@ ...@@ -32,13 +32,14 @@
#include "core/dom/ContainerNode.h" #include "core/dom/ContainerNode.h"
#include "core/dom/Document.h" #include "core/dom/Document.h"
#include "core/dom/ElementTraversal.h" #include "core/dom/ElementTraversal.h"
#include "core/dom/TreeScope.h"
#include "core/dom/shadow/ShadowRoot.h" #include "core/dom/shadow/ShadowRoot.h"
#include "core/html/HTMLStyleElement.h" #include "core/html/HTMLStyleElement.h"
namespace blink { namespace blink {
StyleSheetInvalidationAnalysis::StyleSheetInvalidationAnalysis(const WillBeHeapVector<RawPtrWillBeMember<StyleSheetContents>>& sheets) StyleSheetInvalidationAnalysis::StyleSheetInvalidationAnalysis(const TreeScope& treeScope, const WillBeHeapVector<RawPtrWillBeMember<StyleSheetContents>>& sheets)
: m_dirtiesAllStyle(false) : m_treeScope(treeScope)
{ {
for (unsigned i = 0; i < sheets.size() && !m_dirtiesAllStyle; ++i) for (unsigned i = 0; i < sheets.size() && !m_dirtiesAllStyle; ++i)
analyzeStyleSheet(sheets[i]); analyzeStyleSheet(sheets[i]);
...@@ -92,21 +93,6 @@ static bool hasDistributedRule(StyleSheetContents* styleSheetContents) ...@@ -92,21 +93,6 @@ static bool hasDistributedRule(StyleSheetContents* styleSheetContents)
return false; return false;
} }
static Node* determineScopingNodeForStyleInShadow(HTMLStyleElement* ownerElement, StyleSheetContents* styleSheetContents)
{
ASSERT(ownerElement && ownerElement->isInShadowTree());
if (hasDistributedRule(styleSheetContents)) {
ContainerNode* scope = ownerElement;
do {
scope = scope->containingShadowRoot()->shadowHost();
} while (scope->isInShadowTree());
return scope;
}
return ownerElement->containingShadowRoot()->shadowHost();
}
static bool ruleAdditionMightRequireDocumentStyleRecalc(StyleRuleBase* rule) static bool ruleAdditionMightRequireDocumentStyleRecalc(StyleRuleBase* rule)
{ {
// This funciton is conservative. We only return false when we know that // This funciton is conservative. We only return false when we know that
...@@ -152,12 +138,11 @@ void StyleSheetInvalidationAnalysis::analyzeStyleSheet(StyleSheetContents* style ...@@ -152,12 +138,11 @@ void StyleSheetInvalidationAnalysis::analyzeStyleSheet(StyleSheetContents* style
if (m_dirtiesAllStyle) if (m_dirtiesAllStyle)
return; return;
} }
if (styleSheetContents->hasSingleOwnerNode()) {
Node* ownerNode = styleSheetContents->singleOwnerNode(); if (m_treeScope.rootNode().isShadowRoot()) {
if (isHTMLStyleElement(ownerNode) && toHTMLStyleElement(*ownerNode).isInShadowTree()) { if (hasDistributedRule(styleSheetContents))
m_scopingNodes.append(determineScopingNodeForStyleInShadow(toHTMLStyleElement(ownerNode), styleSheetContents)); m_hasDistributedRules = true;
return; return;
}
} }
const WillBeHeapVector<RefPtrWillBeMember<StyleRuleBase>>& rules = styleSheetContents->childRules(); const WillBeHeapVector<RefPtrWillBeMember<StyleRuleBase>>& rules = styleSheetContents->childRules();
...@@ -192,18 +177,29 @@ static bool elementMatchesSelectorScopes(const Element* element, const HashSet<S ...@@ -192,18 +177,29 @@ static bool elementMatchesSelectorScopes(const Element* element, const HashSet<S
return false; return false;
} }
void StyleSheetInvalidationAnalysis::invalidateStyle(Document& document) static ContainerNode* outermostShadowHost(const ShadowRoot& root)
{
ContainerNode* host = root.host();
while (host->isInShadowTree())
host = host->containingShadowRoot()->host();
return host;
}
void StyleSheetInvalidationAnalysis::invalidateStyle()
{ {
ASSERT(!m_dirtiesAllStyle); ASSERT(!m_dirtiesAllStyle);
if (!m_scopingNodes.isEmpty()) { if (m_treeScope.rootNode().isShadowRoot()) {
for (unsigned i = 0; i < m_scopingNodes.size(); ++i) ContainerNode* invalidationRoot = &m_treeScope.rootNode();
m_scopingNodes.at(i)->setNeedsStyleRecalc(SubtreeStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::StyleSheetChange)); if (m_hasDistributedRules)
invalidationRoot = outermostShadowHost(*toShadowRoot(invalidationRoot));
invalidationRoot->setNeedsStyleRecalc(SubtreeStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::StyleSheetChange));
return;
} }
if (m_idScopes.isEmpty() && m_classScopes.isEmpty()) if (m_idScopes.isEmpty() && m_classScopes.isEmpty())
return; return;
Element* element = ElementTraversal::firstWithin(document); Element* element = ElementTraversal::firstWithin(m_treeScope.document());
while (element) { while (element) {
if (elementMatchesSelectorScopes(element, m_idScopes, m_classScopes)) { if (elementMatchesSelectorScopes(element, m_idScopes, m_classScopes)) {
element->setNeedsStyleRecalc(SubtreeStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::StyleSheetChange)); element->setNeedsStyleRecalc(SubtreeStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::StyleSheetChange));
......
third_party/WebKit/Source/core/css/invalidation/StyleSheetInvalidationAnalysis.h
View file @ e1127317
...@@ -36,23 +36,26 @@ namespace blink { ...@@ -36,23 +36,26 @@ namespace blink {
class Document; class Document;
class Node; class Node;
class StyleSheetContents; class StyleSheetContents;
class TreeScope;
class StyleSheetInvalidationAnalysis { class StyleSheetInvalidationAnalysis {
STACK_ALLOCATED(); STACK_ALLOCATED();
public: public:
StyleSheetInvalidationAnalysis(const WillBeHeapVector<RawPtrWillBeMember<StyleSheetContents>>&); StyleSheetInvalidationAnalysis(const TreeScope&, const WillBeHeapVector<RawPtrWillBeMember<StyleSheetContents>>&);
bool dirtiesAllStyle() const { return m_dirtiesAllStyle; } bool dirtiesAllStyle() const { return m_dirtiesAllStyle; }
void invalidateStyle(Document&); void invalidateStyle();
private: private:
void analyzeStyleSheet(StyleSheetContents*); void analyzeStyleSheet(StyleSheetContents*);
bool m_dirtiesAllStyle; const TreeScope& m_treeScope;
HashSet<StringImpl*> m_idScopes; HashSet<StringImpl*> m_idScopes;
HashSet<StringImpl*> m_classScopes; HashSet<StringImpl*> m_classScopes;
WillBeHeapVector<RawPtrWillBeMember<Node>, 8> m_scopingNodes;
bool m_dirtiesAllStyle = false;
bool m_hasDistributedRules = false;
}; };
} }
......
third_party/WebKit/Source/core/dom/TreeScopeStyleSheetCollection.cpp
View file @ e1127317
...@@ -158,10 +158,10 @@ void TreeScopeStyleSheetCollection::analyzeStyleSheetChange(StyleResolverUpdateM ...@@ -158,10 +158,10 @@ void TreeScopeStyleSheetCollection::analyzeStyleSheetChange(StyleResolverUpdateM
// If we are already parsing the body and so may have significant amount of elements, put some effort into trying to avoid style recalcs. // If we are already parsing the body and so may have significant amount of elements, put some effort into trying to avoid style recalcs.
if (!document().body() || document().hasNodesWithPlaceholderStyle()) if (!document().body() || document().hasNodesWithPlaceholderStyle())
return; return;
StyleSheetInvalidationAnalysis invalidationAnalysis(addedSheets); StyleSheetInvalidationAnalysis invalidationAnalysis(*m_treeScope, addedSheets);
if (invalidationAnalysis.dirtiesAllStyle()) if (invalidationAnalysis.dirtiesAllStyle())
return; return;
invalidationAnalysis.invalidateStyle(document()); invalidationAnalysis.invalidateStyle();
change.requiresFullStyleRecalc = false; change.requiresFullStyleRecalc = false;
return; return;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment