Bug 173129 – Move TreeScope::adoptIfNeeded to Node and rename it to setTreeScopeRecursively

• Component: DOM
• Status: RESOLVED FIXED

[Ryosuke Niwa]

We should move TreeScope::adoptIfNeeded to Nice and rename it to setTreeScopeRecursively.
This basically brings back the code prior to https://trac.webkit.org/changeset/104528.

[Ryosuke Niwa]

Created attachment 312383 [details]
Cleanup

[Ryosuke Niwa]

Created attachment 312384 [details]
Fixed a typo

[Radar WebKit Bug Importer]

<rdar://problem/32667282>

[Ryosuke Niwa]

Created attachment 312389 [details]
Fixed relase builds

[Ryosuke Niwa]

Created attachment 312394 [details]
Fixed release builds for real

[Antti Koivisto]

Comment on attachment 312394 [details]
Fixed release builds for real

View in context: https://bugs.webkit.org/attachment.cgi?id=312394&action=review

> Source/WebCore/dom/Node.cpp:1926
> +#if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
> +class DidMoveToNewDocumentAssertionScope {

Maybe we should add NodeAssertions.h or similar at some point for this sort of things? The main files are pretty bloated already.

> Source/WebCore/dom/Node.cpp:2019
> +        if (!is<Element>(*node))
> +            continue;

It might make sense to add 'element' local right after this check and use it instead of 'node'. The following code will read better and may even get less branchy.

[Ryosuke Niwa]

(In reply to Antti Koivisto from comment #6)
> Comment on attachment 312394 [details]
> Fixed release builds for real
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=312394&action=review
> 
> > Source/WebCore/dom/Node.cpp:1926
> > +#if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
> > +class DidMoveToNewDocumentAssertionScope {
> 
> Maybe we should add NodeAssertions.h or similar at some point for this sort
> of things? The main files are pretty bloated already.

That sounds like a good idea. There's already NoEventDispatchAssertion.h so maybe we could rename hat to NodeAssertions.h and move all these assertions there. Although this particular one is local to Node.cpp so we probably don't need to put into the header.

> > Source/WebCore/dom/Node.cpp:2019
> > +        if (!is<Element>(*node))
> > +            continue;
> 
> It might make sense to add 'element' local right after this check and use it
> instead of 'node'. The following code will read better and may even get less
> branchy.

Fixed that.

[Ryosuke Niwa]

Committed r217972: <http://trac.webkit.org/changeset/217972>