[webkit-reviews] review denied: [Bug 78596] <shadow> should be rendered correctly : [Attachment 130553] Patch

[bugzilla-daemon at webkit.org]

Hajime Morrita <morrita at google.com> has denied Shinya Kawanaka
<shinyak at chromium.org>'s request for review:
Bug 78596: <shadow> should be rendered correctly
https://bugs.webkit.org/show_bug.cgi?id=78596

Attachment 130553: Patch
https://bugs.webkit.org/attachment.cgi?id=130553&action=review

------- Additional Comments from Hajime Morrita <morrita at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=130553&action=review


> Source/WebCore/html/shadow/HTMLShadowElement.cpp:64
> +    if (ShadowRoot* root = toShadowRoot(shadowTreeRootNode()))

Why not treeScope()? shadowTreeRootNode() traverses down the tree and better be
avoided if possible.

> Source/WebCore/html/shadow/InsertionPoint.cpp:52
>      if (ShadowRoot* root = toShadowRoot(shadowTreeRootNode())) {

Ditto.

> Source/WebCore/html/shadow/InsertionPoint.h:71
> +    ShadowRoot* shadowRootAssignedFrom;

I don't think we need this.

> Source/WebCore/html/shadow/InsertionPoint.h:76
>      if (!node || node->isContentElement())

It looks Node::isContentElement() is no longer need and can be morphed to
isInsertionPoint().
It also even don't need to stay in Node. can be pulled down to HTMLElement.