Handle some failing DocumentOrderedMap ID lookups across tree removals.

third_party/WebKit/Source/core/dom/DocumentOrderedMap.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 32 matching lines @@
 
 PassOwnPtrWillBeRawPtr<DocumentOrderedMap> DocumentOrderedMap::create()
 {
     return adoptPtrWillBeNoop(new DocumentOrderedMap);
 }
 
 DocumentOrderedMap::DocumentOrderedMap()
 {
 }
 
-DocumentOrderedMap::~DocumentOrderedMap()
+DEFINE_EMPTY_DESTRUCTOR_WILL_BE_REMOVED(DocumentOrderedMap);
+
+#if ENABLE(ASSERT)
+void DocumentOrderedMap::enterTreeRemoveScope()

[esprehn, 2016/01/05 07:48:33]

no need for the methods, put the class in DocumentOrderedMap.cpp and
increment/decrement the static int

[sof, 2016/01/05 07:53:37]

That's what ps#1 did, but then I got worried about concurrent updates to
multiple Documents. That's not a valid concern?

 {
+    m_treeRemovalScopeLevel++;
 }
 
+void DocumentOrderedMap::leaveTreeRemoveScope()
+{
+    ASSERT(m_treeRemovalScopeLevel);
+    m_treeRemovalScopeLevel--;
+}
+#endif
+
 inline bool keyMatchesId(const AtomicString& key, const Element& element)
 {
     return element.getIdAttribute() == key;
 }
 
 inline bool keyMatchesMapName(const AtomicString& key, const Element& element)
 {
     return isHTMLMapElement(element) && toHTMLMapElement(element).getName() == key;
 }
 
@@ skipping 40 matching lines @@
     } else {
         if (entry->element == element) {
             ASSERT(entry->orderedList.isEmpty() || entry->orderedList.first() == element);
             entry->element = entry->orderedList.size() > 1 ? entry->orderedList[1] : nullptr;
         }
         entry->count--;
         entry->orderedList.clear();
     }
 }
 
-#if ENABLE(ASSERT)
-void DocumentOrderedMap::willRemoveId(const AtomicString& key)
-{
-    ASSERT(m_removingId.isNull() || key.isNull());
-    m_removingId = key;
-}
-#endif
-
 template<bool keyMatches(const AtomicString&, const Element&)>
 inline Element* DocumentOrderedMap::get(const AtomicString& key, const TreeScope* scope) const
 {
     ASSERT(key);
     ASSERT(scope);
 
     MapEntry* entry = m_map.get(key);
     if (!entry)
         return 0;
 
     ASSERT(entry->count);
     if (entry->element)
         return entry->element;
 
     // Iterate to find the node that matches. Nothing will match iff an element
     // with children having duplicate IDs is being removed -- the tree traversal
-    // will be over an updated tree not having that element. In all other cases,
+    // will be over an updated tree not having that subtree. In all other cases,
     // a match is expected.
-    //
-    // Such calls to get()/getElementById() while handling element removals will
-    // legitimately happen when e.g., adjusting form ID associations. Quietly
-    // allow those lookups to (expectedly) fail by having the tree scope removal
-    // register the element ID it is in the process of removing.
     for (Element& element : ElementTraversal::startsAfter(scope->rootNode())) {
         if (!keyMatches(key, element))
             continue;
         entry->element = &element;
         return &element;
     }
-    ASSERT(key == m_removingId);
+    // As get()/getElementById() can legitimately be called while handling element
+    // removals, allow failure iff we're in the scope of node removals.
+    ASSERT(m_treeRemovalScopeLevel);
     return 0;
 }
 
 Element* DocumentOrderedMap::getElementById(const AtomicString& key, const TreeScope* scope) const
 {
     return get<keyMatchesId>(key, scope);
 }
 
 const WillBeHeapVector<RawPtrWillBeMember<Element>>& DocumentOrderedMap::getAllElementsById(const AtomicString& key, const TreeScope* scope) const
 {
@@ skipping 47 matching lines @@
 
 DEFINE_TRACE(DocumentOrderedMap::MapEntry)
 {
     visitor->trace(element);
 #if ENABLE(OILPAN)
     visitor->trace(orderedList);
 #endif
 }
 
 } // namespace blink