[webkit-reviews] review granted: [Bug 173129] Move TreeScope::adoptIfNeeded to Node and rename it to setTreeScopeRecursively : [Attachment 312394] Fixed release builds for real

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 173129: Move TreeScope::adoptIfNeeded to Node and rename it to
setTreeScopeRecursively
https://bugs.webkit.org/show_bug.cgi?id=173129

Attachment 312394: Fixed release builds for real

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




--- Comment #6 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 312394
  --> https://bugs.webkit.org/attachment.cgi?id=312394
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.