[webkit-reviews] review denied: [Bug 65595] <iframe> inside shadow tree should be invisible from the host document. : [Attachment 128159] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied MORITA Hajime
<morrita at google.com>'s request for review:
Bug 65595: <iframe> inside shadow tree should be invisible from the host
document.
https://bugs.webkit.org/show_bug.cgi?id=65595

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=128159&action=review


> Source/WebCore/ChangeLog:8
> +	   This change introduced FrameTree::documentChild() and

"introduces"

> Source/WebCore/ChangeLog:10
> +	   frame lookup. Using these, now the named accessor and the indexed

remove "now" here ...

> Source/WebCore/ChangeLog:11
> +	   acceccor on Document, and Window.length is TreeScope aware. They

and replace "is" with "are now".

> Source/WebCore/page/Frame.cpp:1128
> +bool Frame::isScopedBy(TreeScope* scope) const

isScopedby -> inScope

> Source/WebCore/page/FrameTree.h:57
>	   Frame* previousSibling() const { return m_previousSibling; }
>	   Frame* firstChild() const { return m_firstChild.get(); }
>	   Frame* lastChild() const { return m_lastChild; }

Are these guys still being used?

> Source/WebCore/page/FrameTree.h:78
> +	   Frame* documentChild(unsigned index) const;

I think the name is a bit unintuitive. It requires you to know that Document is
a TreeScope.

Also, the purpose of scoped* functions is limited to just supplying results for
document* functions. Do we need the complete set of the two? Can we collapse
them into one set, called "scopedChild"?