[webkit-reviews] review granted: [Bug 157743] Implement ResizeObserver : [Attachment 365042] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 18 14:04:41 PDT 2019

Simon Fraser (smfr) <simon.fraser at apple.com> has granted cathiechen
<cathiechen at igalia.com>'s request for review:
Bug 157743: Implement ResizeObserver

Attachment 365042: Patch


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

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

Please run the tests with --world-leaks to check whether you've introduced any
new Document leaks.

> Source/WebCore/page/Page.cpp:1308
> +	      

Why do you need to store m_documentsNeedingResizeObservationCheck in a member
variable? You could just return it.

> Source/WebCore/page/Page.cpp:1337
> +    // We need layout the whole frame tree here. Because ResizeObserver
could observe element in other frame,

Is that in the spec? https://github.com/w3c/csswg-drafts/issues/3703 hasn't
been commented on.

> Source/WebCore/page/Page.cpp:1340
> +	   mainFrame().view()->updateLayoutAndStyleIfNeededRecursive();

What if this layout...

> Source/WebCore/page/Page.cpp:1343
> +    for (size_t depth = document->gatherResizeObservations(0); depth !=
ResizeObserver::maxElementDepth(); depth =
document->gatherResizeObservations(depth)) {

Nulled out document

> Source/WebCore/page/ResizeObservation.h:46
> +    LayoutSize computeObservationSize() const;

Maybe computeObservedSize

More information about the webkit-reviews mailing list