Nullptr crash due to display:block ruby and continuations

https://bugs.webkit.org/show_bug.cgi?id=268770 rdar://121960530 Reviewed by Alan Baradlay. Continuations may end up splitting anonymous 'display:ruby' box inside block ruby. * LayoutTests/fast/ruby/ruby-block-continuation-crash-expected.txt: Added. * LayoutTests/fast/ruby/ruby-block-continuation-crash.html: Added. * Source/WebCore/rendering/updating/RenderTreeBuilderRuby.cpp: (WebCore::RenderTreeBuilder::Ruby::findOrCreateParentForStyleBasedRubyChild): Find the correct anonymous box from nested continuation structure. Canonical link: https://commits.webkit.org/279005@main
WebKit · May 20, 2024 · c2f9092 · c2f9092
1 parent 8718132
commit c2f9092
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 3 deletions.
diff --git a/LayoutTests/fast/ruby/ruby-block-continuation-crash-expected.txt b/LayoutTests/fast/ruby/ruby-block-continuation-crash-expected.txt
@@ -0,0 +1,3 @@
+base with
+forced
+line break annotation This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/ruby/ruby-block-continuation-crash.html b/LayoutTests/fast/ruby/ruby-block-continuation-crash.html
@@ -0,0 +1,9 @@
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+<ruby style="position: absolute">
+  <rb><span>base with <div>forced</div> line break</span></rb>
+  <rt>annotation</rt>
+</ruby>
+This test passes if it doesn't crash.
diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilderRuby.cpp b/Source/WebCore/rendering/updating/RenderTreeBuilderRuby.cpp
@@ -65,10 +65,17 @@ RenderElement& RenderTreeBuilder::Ruby::findOrCreateParentForStyleBasedRubyChild
     if (!child.isRenderText() && child.style().display() == DisplayType::Ruby && parent.style().display() == DisplayType::RubyBlock)
         return parent;
 
-    if (parent.style().display() == DisplayType::RubyBlock && parent.firstChild()) {
+    if (parent.style().display() == DisplayType::RubyBlock) {
         // See if we have an anonymous ruby box already.
-        ASSERT(parent.firstChild()->style().display() == DisplayType::Ruby);
-        return downcast<RenderElement>(*parent.firstChild());
+        // FIXME: It should be the immediate child but continuations can break this assumption.
+        for (CheckedPtr first = parent.firstChild(); first; first = first->firstChildSlow()) {
+            if (!first->isAnonymous()) {
+                ASSERT_NOT_REACHED();
+                break;
+            }
+            if (first->style().display() == DisplayType::Ruby)
+                return downcast<RenderElement>(*first);
+        }
     }
 
     if (parent.style().display() != DisplayType::Ruby) {