[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
https://bugs.webkit.org/show_bug.cgi?id=157743
Attachment 365042: Patch
https://bugs.webkit.org/attachment.cgi?id=365042&action=review
--- Comment #49 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 365042
--> https://bugs.webkit.org/attachment.cgi?id=365042
Patch
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
> +
m_documentsNeedingResizeObservationCheck.append(makeWeakPtr(*doc));
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