[webkit-reviews] review denied: [Bug 208047] [intersection-observer] Accept a Document as an explicit root : [Attachment 391642] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Frédéric Wang (:fredw)
<fred.wang at free.fr>'s request for review:
Bug 208047: [intersection-observer] Accept a Document as an explicit root
https://bugs.webkit.org/show_bug.cgi?id=208047

Attachment 391642: Patch

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




--- Comment #8 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 391642
  --> https://bugs.webkit.org/attachment.cgi?id=391642
Patch

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

Let's do a patch without the flag.

> Source/JavaScriptCore/ChangeLog:6
> +	   This patch introduces a new runtime flag

I don't think we need a runtime flag.

> Source/JavaScriptCore/ChangeLog:13
> +	   Reviewed by NOBODY (OOPS!).

This line goes above the description (prepare-Changelogs does this for you).

> Source/WebCore/dom/Document.cpp:7716
> +IntersectionObserverData&
Document::ensureDocumentExplicitIntersectionObserverData()

Just intersectionObserverData

> Source/WebCore/dom/Document.cpp:7723
> +IntersectionObserverData*
Document::documentExplicitIntersectionObserverData()

intersectionObserverDataIfExists()

> Source/WebCore/dom/Document.cpp:7725
> +    return m_documentExplicitIntersectionObserverData ?
m_documentExplicitIntersectionObserverData.get() : nullptr;

Just return m_documentExplicitIntersectionObserverData;

> Source/WebCore/dom/Document.h:1830
> +    // FIXME(fwang): Make this a rare data?
> +    std::unique_ptr<IntersectionObserverData>
m_documentExplicitIntersectionObserverData;

Adding one pointer to Document is fine.

The "documentExplicit" name is confusing. Maybe just
m_intersectionObserverData? Or m_documentRootIntersectionObserverData