Refactor adding and removing named and IDed elements

Source/core/dom/Element.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Peter Kelly (pmk@post.com)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012, 2013 Apple Inc. All rights reserved.
  *           (C) 2007 Eric Seidel (eric@webkit.org)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 1198 matching lines @@
 bool Element::isInert() const
 {
     const Element* dialog = document()->activeModalDialog();
     if (dialog && !containsIncludingShadowDOM(dialog) && !dialog->containsIncludingShadowDOM(this))
         return true;
     return document()->ownerElement() && document()->ownerElement()->isInert();
 }
 
 Node::InsertionNotificationRequest Element::insertedInto(ContainerNode* insertionPoint)
 {
+    bool wasInDocument = inDocument();
     // need to do superclass processing first so inDocument() is true
     // by the time we reach updateId
     ContainerNode::insertedInto(insertionPoint);
+    ASSERT(!wasInDocument || inDocument());

[adamk, 2013/08/09 00:59:11]

How useful is this ASSERT?

 
     if (containsFullScreenElement() && parentElement() && !parentElement()->containsFullScreenElement())
         setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(true);
 
     if (Element* before = pseudoElement(BEFORE))
         before->insertedInto(insertionPoint);
 
     if (Element* after = pseudoElement(AFTER))
         after->insertedInto(insertionPoint);
 
     if (!insertionPoint->isInTreeScope())
         return InsertionDone;
 
     if (hasRareData())
         elementRareData()->clearClassListValueForQuirksMode();
 
     if (isUpgradedCustomElement() && inDocument())
         CustomElement::didEnterDocument(this, document());
 
-    TreeScope* scope = insertionPoint->treeScope();
-    if (scope != treeScope())
+    TreeScope* newScope = insertionPoint->treeScope();
+    if (newScope != treeScope())
         return InsertionDone;
 
+    HTMLDocument* newDocument = !wasInDocument && inDocument() && !isInShadowTree() && newScope->documentScope()->isHTMLDocument() ? toHTMLDocument(newScope->documentScope()) : 0;

[adamk, 2013/08/09 00:59:11]

Gah, this looks weird to me. Isn't newScope->documentScope() just document(),
since we've now guaranteed that newScope == treeScope()?

Also, if we're inDocument() and !isInShadowTree(), the !wasInDocument check is
redundant.

+
     const AtomicString& idValue = getIdAttribute();
-    if (!idValue.isNull())
-        updateId(scope, nullAtom, idValue);
+    if (!idValue.isNull()) {
+        updateIdForTreeScope(newScope, nullAtom, idValue);
+        if (newDocument)
+            updateIdForDocument(newDocument, nullAtom, idValue);
+    }
 
     const AtomicString& nameValue = getNameAttribute();
-    if (!nameValue.isNull())
-        updateName(nullAtom, nameValue);
+    if (!nameValue.isNull()) {
+        updateNameForTreeScope(newScope, nullAtom, nameValue);
+        if (newDocument)
+            updateNameForDocument(newDocument, nullAtom, nameValue);
+    }
 
     if (hasTagName(labelTag)) {
-        if (scope->shouldCacheLabelsByForAttribute())
-            updateLabel(scope, nullAtom, fastGetAttribute(forAttr));
+        if (newScope->shouldCacheLabelsByForAttribute())
+            updateLabel(newScope, nullAtom, fastGetAttribute(forAttr));
     }
 
     if (parentElement() && parentElement()->isInCanvasSubtree())
         setIsInCanvasSubtree(true);
 
     return InsertionDone;
 }
 
 void Element::removedFrom(ContainerNode* insertionPoint)
 {
@@ skipping 11 matching lines @@
 
     if (containsFullScreenElement())
         setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
 
     if (document()->page())
         document()->page()->pointerLockController()->elementRemoved(this);
 
     setSavedLayerScrollOffset(IntSize());
 
     if (insertionPoint->isInTreeScope() && treeScope() == document()) {
+        TreeScope* oldScope = insertionPoint->treeScope();
+        HTMLDocument* oldDocument = inDocument() && !isInShadowTree() && oldScope->documentScope()->isHTMLDocument() ? toHTMLDocument(oldScope->documentScope()) : 0;

[adamk, 2013/08/09 00:59:11]

Eww, this is one of those places where inDocument() means "wasInDocument"...got
me confused. We have a wasInDocument bool above, can you please use it here? And
maybe add a bool wasInShadowTree up there?

+
         const AtomicString& idValue = getIdAttribute();
-        if (!idValue.isNull())
-            updateId(insertionPoint->treeScope(), idValue, nullAtom);
+        if (!idValue.isNull()) {
+            updateIdForTreeScope(oldScope, idValue, nullAtom);
+            if (oldDocument)
+                updateIdForDocument(oldDocument, idValue, nullAtom);
+        }
 
         const AtomicString& nameValue = getNameAttribute();
-        if (!nameValue.isNull())
-            updateName(nameValue, nullAtom);
+        if (!nameValue.isNull()) {
+            updateNameForTreeScope(oldScope, nameValue, nullAtom);
+            if (oldDocument)
+                updateNameForDocument(oldDocument, nameValue, nullAtom);
+        }
 
         if (hasTagName(labelTag)) {
-            TreeScope* treeScope = insertionPoint->treeScope();
-            if (treeScope->shouldCacheLabelsByForAttribute())
-                updateLabel(treeScope, fastGetAttribute(forAttr), nullAtom);
+            if (oldScope->shouldCacheLabelsByForAttribute())
+                updateLabel(oldScope, fastGetAttribute(forAttr), nullAtom);
         }
     }
 
     ContainerNode::removedFrom(insertionPoint);
+
     if (wasInDocument) {
         if (hasPendingResources())
             document()->accessSVGExtensions()->removeElementFromPendingResources(this);
 
         if (isUpgradedCustomElement())
             CustomElement::didLeaveDocument(this, insertionPoint->document());
     }
 
     if (hasRareData())
         elementRareData()->setIsInCanvasSubtree(false);
@@ skipping 1440 matching lines @@
 }
 #endif
 
 #ifdef DUMP_NODE_STATISTICS
 bool Element::hasNamedNodeMap() const
 {
     return hasRareData() && elementRareData()->attributeMap();
 }
 #endif
 
-inline void Element::updateName(const AtomicString& oldName, const AtomicString& newName)
+void Element::updateName(const AtomicString& oldName, const AtomicString& newName)
 {
-    if (!inDocument() || isInShadowTree())
+    if (!isInTreeScope())
         return;
 
     if (oldName == newName)
         return;
 
+    updateNameForTreeScope(treeScope(), oldName, newName);
+
+    if (!inDocument() || isInShadowTree())
+        return;
+
+    Document* htmlDocument = document();
+    if (!htmlDocument->isHTMLDocument())
+        return;
+
+    updateNameForDocument(toHTMLDocument(htmlDocument), oldName, newName);
+}
+
+void Element::updateNameForTreeScope(TreeScope* scope, const AtomicString& oldName, const AtomicString& newName)
+{
+    ASSERT(isInTreeScope());
+    ASSERT(oldName != newName);
+
+    if (!oldName.isEmpty())
+        scope->removeElementByName(oldName, this);
+    if (!newName.isEmpty())
+        scope->addElementByName(newName, this);
+}
+
+void Element::updateNameForDocument(HTMLDocument* document, const AtomicString& oldName, const AtomicString& newName)
+{
+    ASSERT(inDocument() && !isInShadowTree());
+    ASSERT(oldName != newName);
+
     if (shouldRegisterAsNamedItem())
         updateNamedItemRegistration(oldName, newName);
 }
 
-inline void Element::updateId(const AtomicString& oldId, const AtomicString& newId)
+void Element::updateId(const AtomicString& oldId, const AtomicString& newId)
 {
     if (!isInTreeScope())
         return;
 
     if (oldId == newId)
         return;
 
-    updateId(treeScope(), oldId, newId);
+    updateIdForTreeScope(treeScope(), oldId, newId);
+
+    if (!inDocument() || isInShadowTree())
+        return;
+
+    Document* htmlDocument = document();
+    if (!htmlDocument->isHTMLDocument())
+        return;
+
+    updateIdForDocument(toHTMLDocument(htmlDocument), oldId, newId);
 }
 
-inline void Element::updateId(TreeScope* scope, const AtomicString& oldId, const AtomicString& newId)
+void Element::updateIdForTreeScope(TreeScope* scope, const AtomicString& oldId, const AtomicString& newId)
 {
     ASSERT(isInTreeScope());
     ASSERT(oldId != newId);
 
     if (!oldId.isEmpty())
         scope->removeElementById(oldId, this);
     if (!newId.isEmpty())
         scope->addElementById(newId, this);
+}
+
+void Element::updateIdForDocument(HTMLDocument* document, const AtomicString& oldId, const AtomicString& newId)
+{
+    ASSERT(inDocument() && !isInShadowTree());
+    ASSERT(oldId != newId);
 
     if (shouldRegisterAsExtraNamedItem())
         updateExtraNamedItemRegistration(oldId, newId);
 }
 
 void Element::updateLabel(TreeScope* scope, const AtomicString& oldForAttributeValue, const AtomicString& newForAttributeValue)
 {
     ASSERT(hasTagName(labelTag));
 
     if (!inDocument())
@@ skipping 70 matching lines @@
     if (oldDocument->inQuirksMode() != document()->inQuirksMode()) {
         if (hasID())
             setIdAttribute(getIdAttribute());
         if (hasClass())
             setAttribute(HTMLNames::classAttr, getClassAttribute());
     }
 }
 
 void Element::updateNamedItemRegistration(const AtomicString& oldName, const AtomicString& newName)
 {
-    if (!document()->isHTMLDocument())
-        return;
+    ASSERT(document()->isHTMLDocument());
 
     if (!oldName.isEmpty())
         toHTMLDocument(document())->removeNamedItem(oldName);
 
     if (!newName.isEmpty())
         toHTMLDocument(document())->addNamedItem(newName);
 }
 
 void Element::updateExtraNamedItemRegistration(const AtomicString& oldId, const AtomicString& newId)
 {
-    if (!document()->isHTMLDocument())
-        return;
+    ASSERT(document()->isHTMLDocument());
 
     if (!oldId.isEmpty())
         toHTMLDocument(document())->removeExtraNamedItem(oldId);
 
     if (!newId.isEmpty())
         toHTMLDocument(document())->addExtraNamedItem(newId);
 }
 
 PassRefPtr<HTMLCollection> Element::ensureCachedHTMLCollection(CollectionType type)
 {
@@ skipping 117 matching lines @@
 {
     if (hasSyntheticAttrChildNodes())
         detachAllAttrNodesFromElement();
 
     other.synchronizeAllAttributes();
     if (!other.m_elementData) {
         m_elementData.clear();
         return;
     }
 
+    // We can't update window and document's named item maps since the presence of image and object elements depend on other attributes and children.
+    // Fortunately, those named item maps are only updated when this element is in the document, which should never be the case.
+    ASSERT(!inDocument());
+
     const AtomicString& oldID = getIdAttribute();
     const AtomicString& newID = other.getIdAttribute();
 
     if (!oldID.isNull() || !newID.isNull())
         updateId(oldID, newID);
 
     const AtomicString& oldName = getNameAttribute();
     const AtomicString& newName = other.getNameAttribute();
 
     if (!oldName.isNull() || !newName.isNull())
@@ skipping 596 matching lines @@
     return 0;
 }
 
 Attribute* UniqueElementData::attributeItem(unsigned index)
 {
     ASSERT_WITH_SECURITY_IMPLICATION(index < length());
     return &m_attributeVector.at(index);
 }
 
 } // namespace WebCore