Refactor adding and removing named and IDed elements

Source/core/dom/Element.cpp

Index: Source/core/dom/Element.cpp
diff --git a/Source/core/dom/Element.cpp b/Source/core/dom/Element.cpp
index eac6701bb21300997e9e7664766516463f8ec1e5..fb140aa7121841e15f0e4c28faafb9f052580128 100644
--- a/Source/core/dom/Element.cpp
+++ b/Source/core/dom/Element.cpp
@@ -1238,21 +1238,28 @@ Node::InsertionNotificationRequest Element::insertedInto(ContainerNode* insertio
     if (isUpgradedCustomElement() && inDocument())
         CustomElement::didEnterDocument(this, document());
 
-    TreeScope* scope = insertionPoint->treeScope();
-    if (scope != treeScope())
+    if (insertionPoint->treeScope() != treeScope())
         return InsertionDone;
 
+    HTMLDocument* newDocument = inDocument() && !isInShadowTree() && document()->isHTMLDocument() ? toHTMLDocument(document()) : 0;
+
     const AtomicString& idValue = getIdAttribute();
-    if (!idValue.isNull())
-        updateId(scope, nullAtom, idValue);
+    if (!idValue.isNull()) {
+        updateIdForTreeScope(treeScope(), nullAtom, idValue);
+        if (newDocument)
+            updateIdForDocument(newDocument, nullAtom, idValue);
+    }
 
     const AtomicString& nameValue = getNameAttribute();
-    if (!nameValue.isNull())
-        updateName(nullAtom, nameValue);
+    if (!nameValue.isNull()) {
+        updateNameForTreeScope(treeScope(), nullAtom, nameValue);
+        if (newDocument)
+            updateNameForDocument(newDocument, nullAtom, nameValue);
+    }
 
     if (hasTagName(labelTag)) {
-        if (scope->shouldCacheLabelsByForAttribute())
-            updateLabel(scope, nullAtom, fastGetAttribute(forAttr));
+        if (treeScope()->shouldCacheLabelsByForAttribute())
+            updateLabel(treeScope(), nullAtom, fastGetAttribute(forAttr));
     }
 
     if (parentElement() && parentElement()->isInCanvasSubtree())
@@ -1264,6 +1271,7 @@ Node::InsertionNotificationRequest Element::insertedInto(ContainerNode* insertio
 void Element::removedFrom(ContainerNode* insertionPoint)
 {
     bool wasInDocument = insertionPoint->inDocument();
+    bool wasInShadowTree = isInShadowTree(); // Of course, we might still be in a shadow tree...
 
     if (Element* before = pseudoElement(BEFORE))
         before->removedFrom(insertionPoint);
@@ -1284,22 +1292,31 @@ void Element::removedFrom(ContainerNode* insertionPoint)
     setSavedLayerScrollOffset(IntSize());
 
     if (insertionPoint->isInTreeScope() && treeScope() == document()) {
+        TreeScope* oldScope = insertionPoint->treeScope();
+        HTMLDocument* oldDocument = wasInDocument && !wasInShadowTree && oldScope->documentScope()->isHTMLDocument() ? toHTMLDocument(oldScope->documentScope()) : 0;
+
         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);
@@ -2760,19 +2777,47 @@ bool Element::hasNamedNodeMap() const
 }
 #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;
@@ -2780,10 +2825,19 @@ inline void Element::updateId(const AtomicString& oldId, const AtomicString& new
     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);
@@ -2792,6 +2846,12 @@ inline void Element::updateId(TreeScope* scope, const AtomicString& oldId, const
         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);
@@ -2882,8 +2942,7 @@ void Element::didMoveToNewDocument(Document* oldDocument)
 
 void Element::updateNamedItemRegistration(const AtomicString& oldName, const AtomicString& newName)
 {
-    if (!document()->isHTMLDocument())
-        return;
+    ASSERT(document()->isHTMLDocument());
 
     if (!oldName.isEmpty())
         toHTMLDocument(document())->removeNamedItem(oldName);
@@ -2894,8 +2953,7 @@ void Element::updateNamedItemRegistration(const AtomicString& oldName, const Ato
 
 void Element::updateExtraNamedItemRegistration(const AtomicString& oldId, const AtomicString& newId)
 {
-    if (!document()->isHTMLDocument())
-        return;
+    ASSERT(document()->isHTMLDocument());
 
     if (!oldId.isEmpty())
         toHTMLDocument(document())->removeExtraNamedItem(oldId);
@@ -3033,6 +3091,10 @@ void Element::cloneAttributesFromElement(const Element& other)
         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();