Skip to content

Conversation

@fantasai
Copy link
Contributor

@fantasai fantasai commented Oct 16, 2025

dd50ed7

Clean up RenderBox overflow methods
https://bugs.webkit.org/show_bug.cgi?id=300835
rdar://162722242

Reviewed by Alan Baradlay.

The various computeOverflow/addOverflow methods are kinda tangled up. Clean it up.
- Shifts method declarations to the correct superclass/subclass level
  (adjusting names as necessary), and in the right section of the .h file.
- Simplifies parameters and sets us up for additional options in the future.
- Encapsulates memory management of m_overflow.

* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computeOverflow):
(WebCore::RenderBlock::addOverflowFromOutOfFlowBoxes):
(WebCore::RenderBlock::addVisualOverflowFromTheme):
(WebCore::RenderBlock::simplifiedLayout):
(WebCore::RenderBlock::addOverflowFromChildren): Deleted.
(WebCore::RenderBlock::addOverflowFromBlockChildren): Deleted.
* Source/WebCore/rendering/RenderBlock.h:
(WebCore::RenderBlock::computeOverflow):
(WebCore::RenderBlock::addOverflowFromInlineChildren): Deleted.
* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::computeOverflow):
(WebCore::RenderBlockFlow::addOverflowFromInFlowChildren):
* Source/WebCore/rendering/RenderBlockFlow.h:
(WebCore::RenderBlockFlow::computeOverflow):
(WebCore::RenderBlockFlow::addOverflowFromInFlowChildren):
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::addOverflowFromInFlowChildren):
(WebCore::RenderBox::addOverflowFromContainedBox):
(WebCore::RenderBox::addOverflowWithRendererOffset):
(WebCore::RenderBox::addVisualOverflow):
(WebCore::RenderBox::percentageLogicalHeightIsResolvable const):
(WebCore::RenderBox::hasUnsplittableScrollingOverflow const):
(WebCore::RenderBox::isUnsplittableForPagination const):
(WebCore::RenderBox::addOverflowFromInFlowChildOrAbsolutePositionedDescendant): Deleted.
* Source/WebCore/rendering/RenderBox.h:
* Source/WebCore/rendering/RenderGrid.cpp:
(WebCore::RenderGrid::contentOverflowRect const):
(WebCore::RenderGrid::computeOverflow): Deleted.
* Source/WebCore/rendering/RenderGrid.h:
* Source/WebCore/rendering/RenderMultiColumnSet.cpp:
(WebCore::RenderMultiColumnSet::addOverflowFromInFlowChildren):
(WebCore::RenderMultiColumnSet::addOverflowFromChildren): Deleted.
* Source/WebCore/rendering/RenderMultiColumnSet.h:
* Source/WebCore/rendering/RenderTable.cpp:
(WebCore::RenderTable::addOverflowFromInFlowChildren):
(WebCore::RenderTable::addOverflowFromChildren): Deleted.
* Source/WebCore/rendering/RenderTable.h:
(WebCore::RenderTable::addOverflowFromInFlowChildren):
* Source/WebCore/rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::computeOverflowFromCells):
* Source/WebCore/rendering/svg/RenderSVGBlock.cpp:
(WebCore::RenderSVGBlock::computeOverflow):
* Source/WebCore/rendering/svg/RenderSVGBlock.h:

Canonical link: https://commits.webkit.org/301714@main

d857985

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win ✅ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 🧪 win-tests loading 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@fantasai fantasai self-assigned this Oct 16, 2025
@fantasai fantasai added the Layout and Rendering For bugs with layout and rendering of Web pages. label Oct 16, 2025
Copy link
Member

@nt1m nt1m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new tests (OOPS!). will probably trip up merge-queue

RecomputeFloats = 1 << 0,
};
virtual void addOverflowFromInFlowChildren(OptionSet<ComputeOverflowOptions> = { });
void addOverflowFromChild(const RenderBox& child, OptionSet<ComputeOverflowOptions> = { });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alan points out these are not always children, so need to rename this to something else that's more correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to addOverflowFromContainedBox

@fantasai fantasai force-pushed the add-overflow-refactor branch from ab5373a to d857985 Compare October 17, 2025 04:17
@fantasai fantasai added the merge-queue Applied to send a pull request to merge-queue label Oct 17, 2025
https://bugs.webkit.org/show_bug.cgi?id=300835
rdar://162722242

Reviewed by Alan Baradlay.

The various computeOverflow/addOverflow methods are kinda tangled up. Clean it up.
- Shifts method declarations to the correct superclass/subclass level
  (adjusting names as necessary), and in the right section of the .h file.
- Simplifies parameters and sets us up for additional options in the future.
- Encapsulates memory management of m_overflow.

* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computeOverflow):
(WebCore::RenderBlock::addOverflowFromOutOfFlowBoxes):
(WebCore::RenderBlock::addVisualOverflowFromTheme):
(WebCore::RenderBlock::simplifiedLayout):
(WebCore::RenderBlock::addOverflowFromChildren): Deleted.
(WebCore::RenderBlock::addOverflowFromBlockChildren): Deleted.
* Source/WebCore/rendering/RenderBlock.h:
(WebCore::RenderBlock::computeOverflow):
(WebCore::RenderBlock::addOverflowFromInlineChildren): Deleted.
* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::computeOverflow):
(WebCore::RenderBlockFlow::addOverflowFromInFlowChildren):
* Source/WebCore/rendering/RenderBlockFlow.h:
(WebCore::RenderBlockFlow::computeOverflow):
(WebCore::RenderBlockFlow::addOverflowFromInFlowChildren):
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::addOverflowFromInFlowChildren):
(WebCore::RenderBox::addOverflowFromContainedBox):
(WebCore::RenderBox::addOverflowWithRendererOffset):
(WebCore::RenderBox::addVisualOverflow):
(WebCore::RenderBox::percentageLogicalHeightIsResolvable const):
(WebCore::RenderBox::hasUnsplittableScrollingOverflow const):
(WebCore::RenderBox::isUnsplittableForPagination const):
(WebCore::RenderBox::addOverflowFromInFlowChildOrAbsolutePositionedDescendant): Deleted.
* Source/WebCore/rendering/RenderBox.h:
* Source/WebCore/rendering/RenderGrid.cpp:
(WebCore::RenderGrid::contentOverflowRect const):
(WebCore::RenderGrid::computeOverflow): Deleted.
* Source/WebCore/rendering/RenderGrid.h:
* Source/WebCore/rendering/RenderMultiColumnSet.cpp:
(WebCore::RenderMultiColumnSet::addOverflowFromInFlowChildren):
(WebCore::RenderMultiColumnSet::addOverflowFromChildren): Deleted.
* Source/WebCore/rendering/RenderMultiColumnSet.h:
* Source/WebCore/rendering/RenderTable.cpp:
(WebCore::RenderTable::addOverflowFromInFlowChildren):
(WebCore::RenderTable::addOverflowFromChildren): Deleted.
* Source/WebCore/rendering/RenderTable.h:
(WebCore::RenderTable::addOverflowFromInFlowChildren):
* Source/WebCore/rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::computeOverflowFromCells):
* Source/WebCore/rendering/svg/RenderSVGBlock.cpp:
(WebCore::RenderSVGBlock::computeOverflow):
* Source/WebCore/rendering/svg/RenderSVGBlock.h:

Canonical link: https://commits.webkit.org/301714@main
@webkit-commit-queue
Copy link
Collaborator

Committed 301714@main (dd50ed7): https://commits.webkit.org/301714@main

Reviewed commits have been landed. Closing PR #52441 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit dd50ed7 into WebKit:main Oct 17, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Layout and Rendering For bugs with layout and rendering of Web pages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants