https://bugs.webkit.org/show_bug.cgi?id=149364
Reviewed by Antti Koivisto.
Since the tree scope is set to that of Document's inside removeBetween when a node is removed from a shadow tree,
oldScope != &treeScope() was already true inside Element::removedFrom. This can introduce an inconsistency in
DocumentOrderedMap which could result in a crash. Fixed the bug by checking it against document(), which is the
behavior we had prior to r150187.
Also added a consistency check in DocumentOrderedMap to catch bugs like this.
No new tests. New assertions fail in existing tests without this fix.
* dom/DocumentOrderedMap.cpp:
(WebCore::DocumentOrderedMap::add):
(WebCore::DocumentOrderedMap::remove):
(WebCore::DocumentOrderedMap::get):
* dom/DocumentOrderedMap.h:
* dom/Element.cpp:
(WebCore::Element::removedFrom):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@190007
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2015-09-18 Ryosuke Niwa <rniwa@webkit.org>
+
+ REGRESSION(r150187): updateIdForTreeScope may not be called inside shadow trees
+ https://bugs.webkit.org/show_bug.cgi?id=149364
+
+ Reviewed by Antti Koivisto.
+
+ Since the tree scope is set to that of Document's inside removeBetween when a node is removed from a shadow tree,
+ oldScope != &treeScope() was already true inside Element::removedFrom. This can introduce an inconsistency in
+ DocumentOrderedMap which could result in a crash. Fixed the bug by checking it against document(), which is the
+ behavior we had prior to r150187.
+
+ Also added a consistency check in DocumentOrderedMap to catch bugs like this.
+
+ No new tests. New assertions fail in existing tests without this fix.
+
+ * dom/DocumentOrderedMap.cpp:
+ (WebCore::DocumentOrderedMap::add):
+ (WebCore::DocumentOrderedMap::remove):
+ (WebCore::DocumentOrderedMap::get):
+ * dom/DocumentOrderedMap.h:
+ * dom/Element.cpp:
+ (WebCore::Element::removedFrom):
+
2015-09-18 Antti Koivisto <antti@apple.com>
Don't create renderers for children of shadow host
UNUSED_PARAM(treeScope);
ASSERT_WITH_SECURITY_IMPLICATION(element.isInTreeScope());
ASSERT_WITH_SECURITY_IMPLICATION(treeScope.rootNode().containsIncludingShadowDOM(&element));
+
if (!element.isInTreeScope())
return;
Map::AddResult addResult = m_map.add(&key, MapEntry(&element));
+ MapEntry& entry = addResult.iterator->value;
+
+#ifndef NDEBUG
+ ASSERT_WITH_SECURITY_IMPLICATION(!entry.registeredElements.contains(&element));
+ entry.registeredElements.add(&element);
+#endif
+
if (addResult.isNewEntry)
return;
- MapEntry& entry = addResult.iterator->value;
ASSERT_WITH_SECURITY_IMPLICATION(entry.count);
entry.element = nullptr;
entry.count++;
{
m_map.checkConsistency();
auto it = m_map.find(&key);
+
ASSERT_WITH_SECURITY_IMPLICATION(it != m_map.end());
if (it == m_map.end())
return;
- MapEntry& entry = it->value;
+ MapEntry& entry = it->value;
+ ASSERT_WITH_SECURITY_IMPLICATION(entry.registeredElements.remove(&element));
ASSERT_WITH_SECURITY_IMPLICATION(entry.count);
if (entry.count == 1) {
ASSERT_WITH_SECURITY_IMPLICATION(!entry.element || entry.element == &element);
if (entry.element) {
ASSERT_WITH_SECURITY_IMPLICATION(entry.element->isInTreeScope());
ASSERT_WITH_SECURITY_IMPLICATION(&entry.element->treeScope() == &scope);
+ ASSERT_WITH_SECURITY_IMPLICATION(entry.registeredElements.contains(entry.element));
return entry.element;
}
entry.element = &element;
ASSERT_WITH_SECURITY_IMPLICATION(element.isInTreeScope());
ASSERT_WITH_SECURITY_IMPLICATION(&element.treeScope() == &scope);
+ ASSERT_WITH_SECURITY_IMPLICATION(entry.registeredElements.contains(entry.element));
return &element;
}
ASSERT_NOT_REACHED();
#include <wtf/HashCountedSet.h>
#include <wtf/HashMap.h>
+#include <wtf/HashSet.h>
#include <wtf/Vector.h>
#include <wtf/text/AtomicStringImpl.h>
Element* element;
unsigned count;
Vector<Element*> orderedList;
+#ifndef NDEBUG
+ HashSet<Element*> registeredElements;
+#endif
};
typedef HashMap<const AtomicStringImpl*, MapEntry> Map;
if (insertionPoint.isInTreeScope()) {
TreeScope* oldScope = &insertionPoint.treeScope();
HTMLDocument* oldDocument = inDocument() && is<HTMLDocument>(oldScope->documentScope()) ? &downcast<HTMLDocument>(oldScope->documentScope()) : nullptr;
- if (oldScope != &treeScope() || !isInTreeScope())
+ if (!isInTreeScope() || &treeScope() != &document())
oldScope = nullptr;
const AtomicString& idValue = getIdAttribute();