Refactor adding and removing named and IDed elements

Refactor adding and removing named and IDed elements

This CL prepares us to register named and IDed elements on the document for
quick access by the global namespace polluters. By itself, this CL shouldn't
have any visible effects.

R=adamk@chromium.org
BUG=265009
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=155846

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

https://codereview.chromium.org/22508006/diff/1/Source/core/dom/Element.cpp
File Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/22508006/diff/1/Source/core/dom/Element.cpp#n...
Source/core/dom/Element.cpp:1223: ASSERT(!wasInDocument || inDocument());
How useful is this ASSERT?

https://codereview.chromium.org/22508006/diff/1/Source/core/dom/Element.cpp#n...
Source/core/dom/Element.cpp:1247: HTMLDocument* newDocument = !wasInDocument &&
inDocument() && !isInShadowTree() && newScope->documentScope()->isHTMLDocument()
? toHTMLDocument(newScope->documentScope()) : 0;
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.

https://codereview.chromium.org/22508006/diff/1/Source/core/dom/Element.cpp#n...
Source/core/dom/Element.cpp:1298: HTMLDocument* oldDocument = inDocument() &&
!isInShadowTree() && oldScope->documentScope()->isHTMLDocument() ?
toHTMLDocument(oldScope->documentScope()) : 0;
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?

https://codereview.chromium.org/22508006/diff/1/Source/core/editing/ReplaceNo...
File Source/core/editing/ReplaceNodeWithSpanCommand.cpp (right):

https://codereview.chromium.org/22508006/diff/1/Source/core/editing/ReplaceNo...
Source/core/editing/ReplaceNodeWithSpanCommand.cpp:57:
newNode->cloneDataFromElement(*nodeToReplace);
Ick. Not happy that we don't understand this, but cloneDataFromElement() looks
pretty evil.

[abarth-chromium, 2013-08-09 01:10:53]

Done.

[adamk, 2013-08-09 01:11:29]

lgtm

[abarth-chromium, 2013-08-09 01:12:15]

Thanks!

[commit-bot: I haz the power, 2013-08-09 06:33:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/22508006/7001

[abarth-chromium, 2013-08-09 16:03:23]

Committed patchset #2 manually as r155846 (presubmit successful).