ruby: Cleanup of layout_ruby*.*

- Rename LayoutRubyRun::RubyBaseSafe() to EnsureRubyBase()
- Rename LayoutRubyRun::StaticCreateRubyRun() to Create()
- Change return types of some functions from pointers to references
- Change argument types of some functions from pointers to references
- Update comments

This CL has no behavior changes.

Change-Id: I145a159129dbe06a134d82b887e4a2d43558c889
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3640868
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Auto-Submit: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001935}

third_party/blink/renderer/core/layout/layout_ruby.cc

@@ -85,7 +85,7 @@ void LayoutRubyAsInline::AddChild(LayoutObject* child,
       run->AddChild(child, before_child);
       return;
     }
-    NOTREACHED();  // beforeChild should always have a run as parent!
+    NOTREACHED();  // before_child should always have a run as parent!
                    // Emergency fallback: fall through and just append.
   }
 
@@ -94,9 +94,9 @@ void LayoutRubyAsInline::AddChild(LayoutObject* child,
   // (The LayoutRubyRun object will handle the details)
   LayoutRubyRun* last_run = LastRubyRun(this);
   if (!last_run || last_run->HasRubyText()) {
-    last_run = LayoutRubyRun::StaticCreateRubyRun(this, *ContainingBlock());
+    last_run = &LayoutRubyRun::Create(this, *ContainingBlock());
     LayoutInline::AddChild(last_run, before_child);
-    last_run->RubyBaseSafe();
+    last_run->EnsureRubyBase();
   }
   last_run->AddChild(child);
 }
@@ -154,7 +154,7 @@ void LayoutRubyAsBlock::AddChild(LayoutObject* child,
       run->AddChild(child, before_child);
       return;
     }
-    NOTREACHED();  // beforeChild should always have a run as parent!
+    NOTREACHED();  // before_child should always have a run as parent!
                    // Emergency fallback: fall through and just append.
   }
 
@@ -163,9 +163,9 @@ void LayoutRubyAsBlock::AddChild(LayoutObject* child,
   // (The LayoutRubyRun object will handle the details)
   LayoutRubyRun* last_run = LastRubyRun(this);
   if (!last_run || last_run->HasRubyText()) {
-    last_run = LayoutRubyRun::StaticCreateRubyRun(this, *this);
+    last_run = &LayoutRubyRun::Create(this, *this);
     LayoutBlockFlow::AddChild(last_run, before_child);
-    last_run->RubyBaseSafe();
+    last_run->EnsureRubyBase();
   }
   last_run->AddChild(child);
 }

third_party/blink/renderer/core/layout/layout_ruby_base.cc

@@ -60,12 +60,11 @@ bool LayoutRubyBase::IsChildAllowed(LayoutObject* child,
   return child->IsInline();
 }
 
-void LayoutRubyBase::MoveChildren(LayoutRubyBase* to_base,
+// This function removes all children that are before (!) before_child
+// and appends them to to_base.
+void LayoutRubyBase::MoveChildren(LayoutRubyBase& to_base,
                                   LayoutObject* before_child) {
   NOT_DESTROYED();
-  // This function removes all children that are before (!) beforeChild
-  // and appends them to toBase.
-  DCHECK(to_base);
   // Callers should have handled the percent height descendant map.
   DCHECK(!HasPercentHeightDescendants());
 
@@ -79,55 +78,53 @@ void LayoutRubyBase::MoveChildren(LayoutRubyBase* to_base,
 
   SetNeedsLayoutAndIntrinsicWidthsRecalcAndFullPaintInvalidation(
       layout_invalidation_reason::kUnknown);
-  to_base->SetNeedsLayoutAndIntrinsicWidthsRecalcAndFullPaintInvalidation(
+  to_base.SetNeedsLayoutAndIntrinsicWidthsRecalcAndFullPaintInvalidation(
       layout_invalidation_reason::kUnknown);
 }
 
-void LayoutRubyBase::MoveInlineChildren(LayoutRubyBase* to_base,
+void LayoutRubyBase::MoveInlineChildren(LayoutRubyBase& to_base,
                                         LayoutObject* before_child) {
   NOT_DESTROYED();
   DCHECK(ChildrenInline());
-  DCHECK(to_base);
 
   if (!FirstChild())
     return;
 
   LayoutBlock* to_block;
-  if (to_base->ChildrenInline()) {
+  if (to_base.ChildrenInline()) {
     // The standard and easy case: move the children into the target base
-    to_block = to_base;
+    to_block = &to_base;
   } else {
     // We need to wrap the inline objects into an anonymous block.
     // If toBase has a suitable block, we re-use it, otherwise create a new one.
-    LayoutObject* last_child = to_base->LastChild();
+    LayoutObject* last_child = to_base.LastChild();
     if (last_child && last_child->IsAnonymousBlock() &&
         last_child->ChildrenInline()) {
       to_block = To<LayoutBlock>(last_child);
     } else {
-      to_block = to_base->CreateAnonymousBlock();
-      to_base->Children()->AppendChildNode(to_base, to_block);
+      to_block = to_base.CreateAnonymousBlock();
+      to_base.Children()->AppendChildNode(&to_base, to_block);
     }
   }
   // Move our inline children into the target block we determined above.
   MoveChildrenTo(to_block, FirstChild(), before_child);
 }
 
-void LayoutRubyBase::MoveBlockChildren(LayoutRubyBase* to_base,
+void LayoutRubyBase::MoveBlockChildren(LayoutRubyBase& to_base,
                                        LayoutObject* before_child) {
   NOT_DESTROYED();
   DCHECK(!ChildrenInline());
-  DCHECK(to_base);
 
   if (!FirstChild())
     return;
 
-  if (to_base->ChildrenInline())
-    to_base->MakeChildrenNonInline();
+  if (to_base.ChildrenInline())
+    to_base.MakeChildrenNonInline();
 
   // If an anonymous block would be put next to another such block, then merge
   // those.
   LayoutObject* first_child_here = FirstChild();
-  LayoutObject* last_child_there = to_base->LastChild();
+  LayoutObject* last_child_there = to_base.LastChild();
   if (first_child_here->IsAnonymousBlock() &&
       first_child_here->ChildrenInline() && last_child_there &&
       last_child_there->IsAnonymousBlock() &&
@@ -142,17 +139,17 @@ void LayoutRubyBase::MoveBlockChildren(LayoutRubyBase* to_base,
   // Move all remaining children normally. If moving all children, include our
   // float list.
   if (!before_child) {
-    bool full_remove_insert = to_base->HasLayer() || HasLayer();
+    bool full_remove_insert = to_base.HasLayer() || HasLayer();
     // TODO(kojii): |this| is |!ChildrenInline()| when we enter this function,
     // but it may turn to |ChildrenInline()| when |anon_block_here| is destroyed
     // above. Probably the correct fix is to do it earlier and switch to
     // |MoveInlineChildren()| if this happens. For the short term safe fix,
     // using |full_remove_insert| can prevent inconsistent LayoutObject tree
     // that leads to CHECK failures.
     full_remove_insert |= ChildrenInline();
-    MoveAllChildrenIncludingFloatsTo(to_base, full_remove_insert);
+    MoveAllChildrenIncludingFloatsTo(&to_base, full_remove_insert);
   } else {
-    MoveChildrenTo(to_base, FirstChild(), before_child);
+    MoveChildrenTo(&to_base, FirstChild(), before_child);
     RemoveFloatingObjectsFromDescendants();
   }
 }

third_party/blink/renderer/core/layout/layout_ruby_base.h

@@ -68,11 +68,11 @@ class LayoutRubyBase : public LayoutBlockFlow {
       LayoutUnit& logical_left,
       LayoutUnit& logical_width) const override;
 
-  void MoveChildren(LayoutRubyBase* to_base,
+  void MoveChildren(LayoutRubyBase& to_base,
                     LayoutObject* before_child = nullptr);
-  void MoveInlineChildren(LayoutRubyBase* to_base,
+  void MoveInlineChildren(LayoutRubyBase& to_base,
                           LayoutObject* before_child = nullptr);
-  void MoveBlockChildren(LayoutRubyBase* to_base,
+  void MoveBlockChildren(LayoutRubyBase& to_base,
                          LayoutObject* before_child = nullptr);
 
   friend class LayoutNGMixin<LayoutRubyBase>;

third_party/blink/renderer/core/layout/layout_ruby_run.cc

@@ -70,25 +70,21 @@ LayoutRubyText* LayoutRubyRun::RubyText() const {
   // text, layout will have to be changed to handle them properly.
   DCHECK(!child || !child->IsRubyText() ||
          !child->IsFloatingOrOutOfFlowPositioned());
-  return child && child->IsRubyText() ? static_cast<LayoutRubyText*>(child)
-                                      : nullptr;
+  return DynamicTo<LayoutRubyText>(child);
 }
 
 LayoutRubyBase* LayoutRubyRun::RubyBase() const {
   NOT_DESTROYED();
-  LayoutObject* child = LastChild();
-  return child && child->IsRubyBase() ? static_cast<LayoutRubyBase*>(child)
-                                      : nullptr;
+  return DynamicTo<LayoutRubyBase>(LastChild());
 }
 
-LayoutRubyBase* LayoutRubyRun::RubyBaseSafe() {
+LayoutRubyBase& LayoutRubyRun::EnsureRubyBase() {
   NOT_DESTROYED();
-  LayoutRubyBase* base = RubyBase();
-  if (!base) {
-    base = CreateRubyBase();
-    LayoutBlockFlow::AddChild(base);
-  }
-  return base;
+  if (auto* base = RubyBase())
+    return *base;
+  auto& new_base = CreateRubyBase();
+  LayoutBlockFlow::AddChild(&new_base);
+  return new_base;
 }
 
 bool LayoutRubyRun::IsChildAllowed(LayoutObject* child,
@@ -114,42 +110,42 @@ void LayoutRubyRun::AddChild(LayoutObject* child, LayoutObject* before_child) {
       DCHECK_EQ(before_child->Parent(), this);
       LayoutObject* ruby = Parent();
       DCHECK(ruby->IsRuby());
-      auto* new_run = StaticCreateRubyRun(ruby, *ContainingBlock());
-      ruby->AddChild(new_run, NextSibling());
-      new_run->RubyBaseSafe();
+      auto& new_run = Create(ruby, *ContainingBlock());
+      ruby->AddChild(&new_run, NextSibling());
+      new_run.EnsureRubyBase();
       // Add the new ruby text and move the old one to the new run
       // Note: Doing it in this order and not using LayoutRubyRun's methods,
       // in order to avoid automatic removal of the ruby run in case there is no
       // other child besides the old ruby text.
       LayoutBlockFlow::AddChild(child, before_child);
       LayoutBlockFlow::RemoveChild(before_child);
-      new_run->AddChild(before_child);
+      new_run.AddChild(before_child);
     } else if (RubyBase()->FirstChild()) {
       // Insertion before a ruby base object.
       // In this case we need insert a new run before the current one and split
       // the base.
       LayoutObject* ruby = Parent();
-      LayoutRubyRun* new_run = StaticCreateRubyRun(ruby, *ContainingBlock());
-      ruby->AddChild(new_run, this);
-      new_run->RubyBaseSafe();
-      new_run->AddChild(child);
+      LayoutRubyRun& new_run = Create(ruby, *ContainingBlock());
+      ruby->AddChild(&new_run, this);
+      auto& new_base = new_run.EnsureRubyBase();
+      new_run.AddChild(child);
 
       // Make sure we don't leave anything in the percentage descendant
       // map before moving the children to the new base.
       if (HasPercentHeightDescendants())
         ClearPercentHeightDescendants();
-      RubyBaseSafe()->MoveChildren(new_run->RubyBaseSafe(), before_child);
+      EnsureRubyBase().MoveChildren(new_base, before_child);
     }
   } else {
     // child is not a text -> insert it into the base
     // (append it instead if beforeChild is the ruby text)
-    LayoutRubyBase* base = RubyBaseSafe();
-    if (before_child == base)
-      before_child = base->FirstChild();
+    LayoutRubyBase& base = EnsureRubyBase();
+    if (before_child == &base)
+      before_child = base.FirstChild();
     if (before_child && before_child->IsRubyText())
       before_child = nullptr;
-    DCHECK(!before_child || before_child->IsDescendantOf(base));
-    base->AddChild(child, before_child);
+    DCHECK(!before_child || before_child->IsDescendantOf(&base));
+    base.AddChild(child, before_child);
   }
 }
 
@@ -162,12 +158,12 @@ void LayoutRubyRun::RemoveChild(LayoutObject* child) {
     LayoutObject* right_neighbour = NextSibling();
     if (base->FirstChild() && right_neighbour && right_neighbour->IsRubyRun()) {
       auto* right_run = To<LayoutRubyRun>(right_neighbour);
-      LayoutRubyBase* right_base = right_run->RubyBaseSafe();
-      if (right_base->FirstChild()) {
+      LayoutRubyBase& right_base = right_run->EnsureRubyBase();
+      if (right_base.FirstChild()) {
         // Collect all children in a single base, then swap the bases.
-        right_base->MoveChildren(base);
+        right_base.MoveChildren(*base);
         MoveChildTo(right_run, base);
-        right_run->MoveChildTo(this, right_base);
+        right_run->MoveChildTo(this, &right_base);
         DCHECK(!RubyBase()->FirstChild());
       }
     }
@@ -188,22 +184,21 @@ void LayoutRubyRun::RemoveChild(LayoutObject* child) {
   }
 }
 
-LayoutRubyBase* LayoutRubyRun::CreateRubyBase() const {
+LayoutRubyBase& LayoutRubyRun::CreateRubyBase() const {
   NOT_DESTROYED();
-  LayoutRubyBase* layout_object =
-      LayoutRubyBase::CreateAnonymous(&GetDocument(), *this);
+  auto* layout_object = LayoutRubyBase::CreateAnonymous(&GetDocument(), *this);
   scoped_refptr<ComputedStyle> new_style =
       GetDocument().GetStyleResolver().CreateAnonymousStyleWithDisplay(
           StyleRef(), EDisplay::kBlock);
   new_style->SetTextAlign(ETextAlign::kCenter);  // FIXME: use WEBKIT_CENTER?
   new_style->SetHasLineIfEmpty(true);
   layout_object->SetStyle(std::move(new_style));
-  return layout_object;
+  return *layout_object;
 }
 
-LayoutRubyRun* LayoutRubyRun::StaticCreateRubyRun(
-    const LayoutObject* parent_ruby,
-    const LayoutBlock& containing_block) {
+// static
+LayoutRubyRun& LayoutRubyRun::Create(const LayoutObject* parent_ruby,
+                                     const LayoutBlock& containing_block) {
   DCHECK(parent_ruby);
   DCHECK(parent_ruby->IsRuby());
   LayoutRubyRun* rr;
@@ -219,7 +214,7 @@ LayoutRubyRun* LayoutRubyRun::StaticCreateRubyRun(
           .CreateAnonymousStyleWithDisplay(parent_ruby->StyleRef(),
                                            EDisplay::kInlineBlock);
   rr->SetStyle(std::move(new_style));
-  return rr;
+  return *rr;
 }
 
 LayoutObject* LayoutRubyRun::LayoutSpecialExcludedChild(

third_party/blink/renderer/core/layout/layout_ruby_run.h

@@ -43,18 +43,22 @@ class LayoutNGMixin;
 
 // LayoutRubyRun are 'inline-block/table' like objects,and wrap a single pairing
 // of a ruby base with its ruby text(s).
-// See LayoutRuby.h for further comments on the structure
+// See layout_ruby.h for further comments on the structure
 
 class LayoutRubyRun : public LayoutBlockFlow {
  public:
+  // The argument must be nullptr.
+  explicit LayoutRubyRun(ContainerNode*);
+  static LayoutRubyRun& Create(const LayoutObject* parent_ruby,
+                               const LayoutBlock& containing_block);
   ~LayoutRubyRun() override;
 
   bool HasRubyText() const;
   bool HasRubyBase() const;
   LayoutRubyText* RubyText() const;
   LayoutRubyBase* RubyBase() const;
-  LayoutRubyBase*
-  RubyBaseSafe();  // creates the base if it doesn't already exist
+  // Creates the base if it doesn't already exist
+  LayoutRubyBase& EnsureRubyBase();
 
   LayoutObject* LayoutSpecialExcludedChild(bool relayout_children,
                                            SubtreeLayoutScope&) override;
@@ -71,22 +75,15 @@ class LayoutRubyRun : public LayoutBlockFlow {
                    int& start_overhang,
                    int& end_overhang) const;
 
-  static LayoutRubyRun* StaticCreateRubyRun(
-      const LayoutObject* parent_ruby,
-      const LayoutBlock& containing_block);
-
   bool CanBreakBefore(const LazyLineBreakIterator&) const;
 
   const char* GetName() const override {
     NOT_DESTROYED();
     return "LayoutRubyRun";
   }
 
-  // The argument must be nullptr.
-  explicit LayoutRubyRun(ContainerNode*);
-
  protected:
-  LayoutRubyBase* CreateRubyBase() const;
+  LayoutRubyBase& CreateRubyBase() const;
 
  private:
   bool IsOfType(LayoutObjectType type) const override {