Skip to content
Permalink
Browse files
DocumentOrderedMap::add should release assert that tree scopes match
https://bugs.webkit.org/show_bug.cgi?id=178708

Reviewed by Antti Koivisto.

Assert that the tree scope of element matches the given tree scope instead of asserting that
element is in tree scope, and replaced the use of RELEASE_ASSERT by the newly added
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION to clarify the semantics of these assertions.

Also removed now redudnant early exits which would never execute due to release assertions.

* dom/DocumentOrderedMap.cpp:
(WebCore::DocumentOrderedMap::add):
(WebCore::DocumentOrderedMap::remove):
(WebCore::DocumentOrderedMap::get const):


Canonical link: https://commits.webkit.org/194882@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223886 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rniwa committed Oct 24, 2017
1 parent 288ccf2 commit 9327764210e2a6c0252d5ae61c68b1acfc8592ac
Showing with 30 additions and 18 deletions.
  1. +18 −0 Source/WebCore/ChangeLog
  2. +12 −18 Source/WebCore/dom/DocumentOrderedMap.cpp
@@ -1,3 +1,21 @@
2017-10-24 Ryosuke Niwa <rniwa@webkit.org>

DocumentOrderedMap::add should release assert that tree scopes match
https://bugs.webkit.org/show_bug.cgi?id=178708

Reviewed by Antti Koivisto.

Assert that the tree scope of element matches the given tree scope instead of asserting that
element is in tree scope, and replaced the use of RELEASE_ASSERT by the newly added
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION to clarify the semantics of these assertions.

Also removed now redudnant early exits which would never execute due to release assertions.

* dom/DocumentOrderedMap.cpp:
(WebCore::DocumentOrderedMap::add):
(WebCore::DocumentOrderedMap::remove):
(WebCore::DocumentOrderedMap::get const):

2017-10-24 Michael Catanzaro <mcatanzaro@igalia.com>

-Wsubobject-linkage warning in InspectorIndexedDBAgent.cpp
@@ -48,8 +48,7 @@ void DocumentOrderedMap::clear()

void DocumentOrderedMap::add(const AtomicStringImpl& key, Element& element, const TreeScope& treeScope)
{
UNUSED_PARAM(treeScope);
RELEASE_ASSERT(element.isInTreeScope());
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(&element.treeScope() == &treeScope);
ASSERT_WITH_SECURITY_IMPLICATION(treeScope.rootNode().containsIncludingShadowDOM(&element));

if (!element.isInTreeScope())
@@ -67,7 +66,7 @@ void DocumentOrderedMap::add(const AtomicStringImpl& key, Element& element, cons
if (addResult.isNewEntry)
return;

RELEASE_ASSERT(entry.count);
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(entry.count);
entry.element = nullptr;
entry.count++;
entry.orderedList.clear();
@@ -78,15 +77,13 @@ void DocumentOrderedMap::remove(const AtomicStringImpl& key, Element& element)
m_map.checkConsistency();
auto it = m_map.find(&key);

RELEASE_ASSERT(it != m_map.end());
if (it == m_map.end())
return;
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(it != m_map.end());

MapEntry& entry = it->value;
ASSERT_WITH_SECURITY_IMPLICATION(entry.registeredElements.remove(&element));
RELEASE_ASSERT(entry.count);
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(entry.count);
if (entry.count == 1) {
RELEASE_ASSERT(!entry.element || entry.element == &element);
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!entry.element || entry.element == &element);
m_map.remove(it);
} else {
if (entry.element == &element)
@@ -108,19 +105,18 @@ inline Element* DocumentOrderedMap::get(const AtomicStringImpl& key, const TreeS
MapEntry& entry = it->value;
ASSERT(entry.count);
if (entry.element) {
RELEASE_ASSERT(entry.element->isInTreeScope());
RELEASE_ASSERT(&entry.element->treeScope() == &scope);
ASSERT_WITH_SECURITY_IMPLICATION(entry.registeredElements.contains(entry.element));
return entry.element;
auto& element = *entry.element;
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(&element.treeScope() == &scope);
ASSERT_WITH_SECURITY_IMPLICATION(entry.registeredElements.contains(&element));
return &element;
}

// We know there's at least one node that matches; iterate to find the first one.
for (auto& element : descendantsOfType<Element>(scope.rootNode())) {
if (!keyMatches(key, element))
continue;
entry.element = &element;
RELEASE_ASSERT(element.isInTreeScope());
RELEASE_ASSERT(&element.treeScope() == &scope);
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(&element.treeScope() == &scope);
ASSERT_WITH_SECURITY_IMPLICATION(entry.registeredElements.contains(entry.element));
return &element;
}
@@ -187,9 +183,7 @@ const Vector<Element*>* DocumentOrderedMap::getAllElementsById(const AtomicStrin
return nullptr;

MapEntry& entry = it->value;
RELEASE_ASSERT(entry.count);
if (!entry.count)
return nullptr;
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(entry.count);

if (entry.orderedList.isEmpty()) {
entry.orderedList.reserveCapacity(entry.count);
@@ -202,7 +196,7 @@ const Vector<Element*>* DocumentOrderedMap::getAllElementsById(const AtomicStrin
continue;
entry.orderedList.append(&element);
}
RELEASE_ASSERT(entry.orderedList.size() == entry.count);
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(entry.orderedList.size() == entry.count);
}

return &entry.orderedList;

0 comments on commit 9327764

Please sign in to comment.