Merge ChildNodeInsertionNotifier and ChildNodeRemovalNotifier into ContainerNode

Merge ChildNodeInsertionNotifier and ChildNodeRemovalNotifier into ContainerNode

We don't really need these classes anymore, we can just use methods on
ContainerNode. This gets us one step closer to completely eliminating
ContainerNodeAlgorithms which now only contains one shared method related
to SVGElementInstance and cleanup code that Oilpan will eliminate.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174982

[ojan, 2014-05-28 06:05:48]

Oh man, getting rid of ContainerNodeAlgorithms would be amazing. The patch looks
fine, but you're hitting asserts on debug, so it's clearly not fine. :)

https://codereview.chromium.org/305603003/diff/20001/Source/core/dom/Containe...
File Source/core/dom/ContainerNode.cpp (right):

https://codereview.chromium.org/305603003/diff/20001/Source/core/dom/Containe...
Source/core/dom/ContainerNode.cpp:673: ScriptForbiddenScope forbidScript;
Nit: I might put this inside notifyNodeInsertedInternal. No big deal either way.
I understand you're just moving this code around, so it should be a separate
patch anyways.

[esprehn, 2014-05-28 06:53:54]

On 2014/05/28 06:05:48, ojan wrote:
> Oh man, getting rid of ContainerNodeAlgorithms would be amazing. The patch
looks
> fine, but you're hitting asserts on debug, so it's clearly not fine. :)
> 

I changed the RefPtr protects inside notifyNodeInserted to be more correct but
it exposed some real bugs where ensureUserAgentShadowRoot() is being called in
the constructor of an Element before the element itself has been adoptRef()'ed.
Then I RefPtr<Node> protect(this) which triggers an ASSERT because the Node was
never adopted.

We shouldn't be calling ensureUserAgentShadowRoot() in the constructor at all,
instead it should be called in the ::create() methods. For now I've reverted the
RefPtrs in this patch, I'll change the callers and the RefPtrs in the next
patch.

[esprehn, 2014-05-28 06:55:13]

On 2014/05/28 06:05:48, ojan wrote:
> ...
>
https://codereview.chromium.org/305603003/diff/20001/Source/core/dom/Containe...
> Source/core/dom/ContainerNode.cpp:673: ScriptForbiddenScope forbidScript;
> Nit: I might put this inside notifyNodeInsertedInternal. No big deal either
way.
> I understand you're just moving this code around, so it should be a separate
> patch anyways.

Yeah, I'll happily do that in another patch.

[ojan, 2014-05-28 17:46:36]

lgtm

[commit-bot: I haz the power, 2014-05-28 17:47:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/esprehn@chromium.org/305603003/40001

[commit-bot: I haz the power, 2014-05-28 18:16:37]

Change committed as 174982