Skip to content

Commit

Permalink
Crash with ruby and continuations
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=271167
rdar://124432235

Reviewed by Ryosuke Niwa.

* LayoutTests/fast/ruby/ruby-continuation-crash-expected.txt: Added.
* LayoutTests/fast/ruby/ruby-continuation-crash.html: Added.
* Source/WebCore/rendering/updating/RenderTreeBuilder.cpp:
(WebCore::RenderTreeBuilder::destroyAndCleanUpAnonymousWrappers):

Improve robustness by using WeakPtr for destroyRoot and null checking before calling destroy(*destroyRoot)
as it might get deleted by an earlier destroy() call.

Canonical link: https://commits.webkit.org/272448.763@safari-7618-branch
  • Loading branch information
anttijk committed Mar 19, 2024
1 parent 6ce91a5 commit df9629a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 8 deletions.
1 change: 1 addition & 0 deletions LayoutTests/fast/ruby/ruby-continuation-crash-expected.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This test passes if it doesn't crash.
12 changes: 12 additions & 0 deletions LayoutTests/fast/ruby/ruby-continuation-crash.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<style>
#abs_pos {
position: absolute;
}
</style>
<ruby id=abs_pos><ruby id=inner_ruby></ruby><span><div>This test passes if it doesn't crash.</div></span></ruby>
<script>
if (window.testRunner)
testRunner.dumpAsText();
document.body.offsetHeight;
inner_ruby.remove();
</script>
19 changes: 11 additions & 8 deletions Source/WebCore/rendering/updating/RenderTreeBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -870,45 +870,48 @@ void RenderTreeBuilder::destroyAndCleanUpAnonymousWrappers(RenderObject& rendere
return *destroyRoot;
};

auto& destroyRoot = destroyRootIncludingAnonymous();
WeakPtr destroyRoot = destroyRootIncludingAnonymous();

auto clearFloatsAndOutOfFlowPositionedObjects = [&] {
// Remove floats and out-of-flow positioned objects from their containing block before detaching
// the renderer from the tree. It includes all the anonymous block descendants that we are about
// to destroy as well as part of the cleanup process below.
auto* destroyRootElement = dynamicDowncast<RenderElement>(destroyRoot);
WeakPtr destroyRootElement = dynamicDowncast<RenderElement>(destroyRoot.get());
if (!destroyRootElement)
return;
for (auto& descendant : descendantsOfType<RenderBox>(*destroyRootElement)) {
if (descendant.isFloatingOrOutOfFlowPositioned())
descendant.removeFloatingOrPositionedChildFromBlockLists();
}
if (CheckedPtr box = dynamicDowncast<RenderBox>(destroyRoot); box && box->isFloatingOrOutOfFlowPositioned())
if (CheckedPtr box = dynamicDowncast<RenderBox>(destroyRoot.get()); box && box->isFloatingOrOutOfFlowPositioned())
box->removeFloatingOrPositionedChildFromBlockLists();
};
clearFloatsAndOutOfFlowPositionedObjects();

auto collapseAndDestroyAnonymousSiblings = [&] {
// FIXME: Probably need to handle other table parts here as well.
if (CheckedPtr cell = dynamicDowncast<RenderTableCell>(destroyRoot)) {
if (CheckedPtr cell = dynamicDowncast<RenderTableCell>(destroyRoot.get())) {
tableBuilder().collapseAndDestroyAnonymousSiblingCells(*cell);
return;
}

if (CheckedPtr row = dynamicDowncast<RenderTableRow>(destroyRoot)) {
if (CheckedPtr row = dynamicDowncast<RenderTableRow>(destroyRoot.get())) {
tableBuilder().collapseAndDestroyAnonymousSiblingRows(*row);
return;
}
};
collapseAndDestroyAnonymousSiblings();

// FIXME: Do not try to collapse/cleanup the anonymous wrappers inside destroy (see webkit.org/b/186746).
WeakPtr destroyRootParent = *destroyRoot.parent();
if (&rendererToDestroy != &destroyRoot) {
WeakPtr destroyRootParent = destroyRoot->parent();
if (&rendererToDestroy != destroyRoot.get()) {
// Destroy the child renderer first, before we start tearing down the anonymous wrapper ancestor chain.
destroy(rendererToDestroy);
}
destroy(destroyRoot);

if (destroyRoot)
destroy(*destroyRoot);

if (!destroyRootParent)
return;
removeAnonymousWrappersForInlineChildrenIfNeeded(*destroyRootParent);
Expand Down

0 comments on commit df9629a

Please sign in to comment.