Search correctly for inline ancestors without a sibling when dirtying lineboxes

third_party/WebKit/Source/core/layout/line/LineBoxList.cpp

Index: third_party/WebKit/Source/core/layout/line/LineBoxList.cpp
diff --git a/third_party/WebKit/Source/core/layout/line/LineBoxList.cpp b/third_party/WebKit/Source/core/layout/line/LineBoxList.cpp
index 8cdb236b02d08de27c8c6f7479faa0570dff9945..43085b849537bf6e536b9917ed9b2d933829e23c 100644
--- a/third_party/WebKit/Source/core/layout/line/LineBoxList.cpp
+++ b/third_party/WebKit/Source/core/layout/line/LineBoxList.cpp
@@ -289,16 +289,16 @@ void LineBoxList::DirtyLinesFromChangedChild(LineLayoutItem container,
 
   // Try to figure out which line box we belong in. First try to find a previous
   // line box by examining our siblings. If we are a float inside an inline then
-  // check the siblings of our inline parent. If we didn't find a line box, then
-  // use our parent's first line box.
+  // check our nearest inline ancestor with siblings. If we didn't find a line
+  // box, then use our parent's first line box.
   RootInlineBox* box = nullptr;
   LineLayoutItem curr = child.PreviousSibling();
-  if (child.IsFloating() && !curr) {
-    LineLayoutInline outer_inline;
-    for (LineLayoutItem parent = child.Parent();
-         parent && parent.IsLayoutInline() && !parent.PreviousSibling();
-         parent = parent.Parent())
-      outer_inline = LineLayoutInline(parent);
+  LineLayoutItem parent = child.Parent();

[mstensho (USE GERRIT), 2017/04/19 11:37:24]

|parent| is only needed if child.IsFloating(), so I think it would be better to
move it inside some scope (not that it's so much cheaper, but it makes it easier
to read). Something like this:

if (child.IsFloating() && !curr)  {
  LineLayoutItem parent = child.Parent();
  if (parent && parent.IsLayoutInline()) {
     outer_inline = parent;
     while ...
     if (outer_inline) ...
  }
}

... or just omit the declaration of |parent| completely. Writing
"child.Parent()" a couple of times extra doesn't sound so harmful. Do we really
need the parent NULL-check, BTW? I think I'd like the program to just go ahead
and die of the child doesn't have a parent. :)

+  if (child.IsFloating() && !curr && parent && parent.IsLayoutInline()) {
+    LineLayoutItem outer_inline = parent;
+    while (outer_inline && outer_inline.Parent().IsLayoutInline() &&
+           !outer_inline.PreviousSibling())

[mstensho (USE GERRIT), 2017/04/19 11:37:24]

Could you check !outer_inline.PreviousSibling() BEFORE
outer_inline.Parent().IsLayoutInline()? I had a hard time reading this, and
besides it doesn't matter what the parent is if outer_inline has siblings.

+      outer_inline = outer_inline.Parent();
     if (outer_inline)
       curr = outer_inline.PreviousSibling();
   }