Remove SVGUseElement helper subtreeContainsDisallowedElement

Remove SVGUseElement helper subtreeContainsDisallowedElement

The related helper removeDisallowedElementsFromSubtree() already walks
the same subtree and checks with the same predicate[1], so letting the
removing function do all the work should not be a problem.

Also change isDisallowedElement to take a const Node&, and tighten the
type of the subtree root passed to removeDisallowedElementsFromSubtree
(it's always either a SVGSVGElement or a SVGGElement.)
Move the lengthy - and somewhat outdated - comment above
removeDisallowedElementsFromSubtree to just above its definition.
(We aim to align the current behavior to it though, so keeping it
around unchanged.)

[1] subtreeContainsDisallowedElement() was walking the Nodes of the
    tree, while removeDisallowedElementsFromSubtree() walks the
    Elements. Thus they did not look at the exact same set of nodes.
    Since the removal took place on the smaller set though there should
    be no change in behavior.
    Previously we could end up walking the entire subtree looking for
    something to remove (in removeDisallowedElementsFromSubtree)
    eventhough we wouldn't find it (like for example a COMMENT node.)

BUG=589682
Committed: https://crrev.com/1b7b24793aceb615c9d54ae73c0cbbd7b9480622
Cr-Commit-Position: refs/heads/master@{#378993}

[fs, 2016-03-02 16:41:33]

fs@opera.com changed reviewers:
+ pdr@chromium.org

[Stephen Chennney, 2016-03-02 17:40:03]

schenney@chromium.org changed reviewers:
+ schenney@chromium.org

[Stephen Chennney, 2016-03-02 17:40:03]

LGTM. I'm assuming we don't regress performance here, which we would do if it's
much slower to do the element traversal vs the node traversal in the common case
of no disallowed elements. I guess we'll find out.

[fs, 2016-03-02 17:58:22]

On 2016/03/02 at 17:40:03, schenney wrote:
> LGTM. I'm assuming we don't regress performance here, which we would do if
it's much slower to do the element traversal vs the node traversal in the common
case of no disallowed elements. I guess we'll find out.

The major difference is a check of the Node IsElementFlag, so I'm not too
worried, but yeah we'll see =)

[pdr., 2016-03-02 21:01:10]

On 2016/03/02 at 17:58:22, fs wrote:
> On 2016/03/02 at 17:40:03, schenney wrote:
> > LGTM. I'm assuming we don't regress performance here, which we would do if
it's much slower to do the element traversal vs the node traversal in the common
case of no disallowed elements. I guess we'll find out.
> 
> The major difference is a check of the Node IsElementFlag, so I'm not too
worried, but yeah we'll see =)

LGTM! This is looking so much better. Thank you for giving this area some TLC

[fs, 2016-03-03 10:22:48]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2016-03-03 10:22:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1755153002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1755153002/1

[commit-bot: I haz the power, 2016-03-03 10:27:51]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-03 10:29:12]

Description was changed from

==========
Remove SVGUseElement helper subtreeContainsDisallowedElement

The related helper removeDisallowedElementsFromSubtree() already walks
the same subtree and checks with the same predicate[1], so letting the
removing function do all the work should not be a problem.

Also change isDisallowedElement to take a const Node&, and tighten the
type of the subtree root passed to removeDisallowedElementsFromSubtree
(it's always either a SVGSVGElement or a SVGGElement.)
Move the lengthy - and somewhat outdated - comment above
removeDisallowedElementsFromSubtree to just above its definition.
(We aim to align the current behavior to it though, so keeping it
around unchanged.)

[1] subtreeContainsDisallowedElement() was walking the Nodes of the
    tree, while removeDisallowedElementsFromSubtree() walks the
    Elements. Thus they did not look at the exact same set of nodes.
    Since the removal took place on the smaller set though there should
    be no change in behavior.
    Previously we could end up walking the entire subtree looking for
    something to remove (in removeDisallowedElementsFromSubtree)
    eventhough we wouldn't find it (like for example a COMMENT node.)

BUG=589682
==========

to

==========
Remove SVGUseElement helper subtreeContainsDisallowedElement

The related helper removeDisallowedElementsFromSubtree() already walks
the same subtree and checks with the same predicate[1], so letting the
removing function do all the work should not be a problem.

Also change isDisallowedElement to take a const Node&, and tighten the
type of the subtree root passed to removeDisallowedElementsFromSubtree
(it's always either a SVGSVGElement or a SVGGElement.)
Move the lengthy - and somewhat outdated - comment above
removeDisallowedElementsFromSubtree to just above its definition.
(We aim to align the current behavior to it though, so keeping it
around unchanged.)

[1] subtreeContainsDisallowedElement() was walking the Nodes of the
    tree, while removeDisallowedElementsFromSubtree() walks the
    Elements. Thus they did not look at the exact same set of nodes.
    Since the removal took place on the smaller set though there should
    be no change in behavior.
    Previously we could end up walking the entire subtree looking for
    something to remove (in removeDisallowedElementsFromSubtree)
    eventhough we wouldn't find it (like for example a COMMENT node.)

BUG=589682

Committed: https://crrev.com/1b7b24793aceb615c9d54ae73c0cbbd7b9480622
Cr-Commit-Position: refs/heads/master@{#378993}
==========

[commit-bot: I haz the power, 2016-03-03 10:29:12]

Patchset 1 (id:??) landed as
https://crrev.com/1b7b24793aceb615c9d54ae73c0cbbd7b9480622
Cr-Commit-Position: refs/heads/master@{#378993}