Bug 230242

Summary: Implement the borderBoxSize/contentBoxSize parts of ResizeObserver
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: DOMAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, cdumez, changseok, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, kondapallykalyan, pdr, ryuan.choi, sam, sergio, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar, WebExposed, WPTImpact
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=219005
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch none

Simon Fraser (smfr)
Reported 2021-09-13 21:00:50 PDT
Implement the borderBoxSize/contentBoxSize parts of ResizeObserver
Attachments
Patch (58.81 KB, patch)
2021-09-13 21:06 PDT, Simon Fraser (smfr)
no flags
Patch (57.86 KB, patch)
2021-09-13 22:09 PDT, Simon Fraser (smfr)
ews-feeder: commit-queue-
Patch (58.85 KB, patch)
2021-09-13 22:18 PDT, Simon Fraser (smfr)
no flags
Patch (63.04 KB, patch)
2021-09-14 09:49 PDT, Simon Fraser (smfr)
no flags
Patch (63.82 KB, patch)
2021-09-14 13:26 PDT, Simon Fraser (smfr)
no flags
Simon Fraser (smfr)
Comment 1 2021-09-13 21:06:10 PDT
Simon Fraser (smfr)
Comment 2 2021-09-13 22:09:44 PDT
Simon Fraser (smfr)
Comment 3 2021-09-13 22:18:52 PDT
Simon Fraser (smfr)
Comment 4 2021-09-14 09:49:12 PDT
zalan
Comment 5 2021-09-14 11:49:54 PDT
Comment on attachment 438152 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438152&action=review > Source/WebCore/rendering/RenderBox.h:141 > + LayoutSize logicalSize() const { return style().isHorizontalWritingMode() ? m_frameRect.size() : m_frameRect.size().transposedSize(); } any reason not relying on the existing logical functions e.g return { logicalWidth(), logicalHeight() };
Sam Weinig
Comment 6 2021-09-14 11:57:59 PDT
Comment on attachment 438152 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438152&action=review DOM stuff looks good. > Source/WebCore/page/ResizeObservation.cpp:106 > +bool ResizeObservation::elementSizeChanged(BoxSizes& currentSize) const I would return either a pair or a bespoke struct here instead of using an out parameter. > Source/WebCore/page/ResizeObserverBoxOptions.idl:26 > +// https://wicg.github.io/ResizeObserver/ I would update this link to the more specific https://wicg.github.io/ResizeObserver/#enumdef-resizeobserverboxoptions or https://drafts.csswg.org/resize-observer/#enumdef-resizeobserverboxoptions. (I wish there were a way to point to a particular version, like based on the git sha), so that if the spec changes, we know which one our implementation was based on, but I don't think there is a way to do that. > Source/WebCore/page/ResizeObserverBoxOptions.idl:30 > + "content-box" I would put "device-pixel-content-box" commented out here with a bugzilla link to a bug about it. > Source/WebCore/page/ResizeObserverEntry.h:68 > + // The spec is designed to allow mulitple boxes for multicol scenarios, but for now these vectors only ever contain one entry. > + Vector<Ref<ResizeObserverSize>> m_borderBoxSizes; > + Vector<Ref<ResizeObserverSize>> m_contentBoxSizes; One day, we should really convert these types of immutable vectors that don't ever change size once constructed to use something more efficient like FixedVector (and we should fix FixedVector to not depend on RefCountedArray). > Source/WebCore/page/ResizeObserverOptions.idl:26 > +// https://wicg.github.io/ResizeObserver/ Again, I would use https://drafts.csswg.org/resize-observer/#dictdef-resizeobserveroptions > Source/WebCore/page/ResizeObserverSize.idl:26 > +// https://wicg.github.io/ResizeObserver/ Again, https://drafts.csswg.org/resize-observer/#resizeobserversize.
Simon Fraser (smfr)
Comment 7 2021-09-14 13:26:47 PDT
EWS
Comment 8 2021-09-14 23:17:04 PDT
Committed r282441 (241694@main): <https://commits.webkit.org/241694@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 438161 [details].
Radar WebKit Bug Importer
Comment 9 2021-09-14 23:18:16 PDT
Note You need to log in before you can comment on or make changes to this bug.