I just remembered one more thing. If you do not want to generate
empty fragments, we could do a hack. Something like this:

NGBoxFragmentPainter::PaintLineBoxChildren
if (line->IsEmpty() && paint_phase == kOutline) {
  find all LayoutInlines that generated no fragments inside box_fragment_->GetLayoutObject()
  for each:
    InlinePainter(LayoutInline).Paint() // if possible to use legacy code.
}
if we cannot use InlinePainter, create new method PaintEmptyInline that just collects descendant outlines from LayoutInline, and paints them.

One more thing: I've talked to fantasai about this, and she said that if no fragments are generated, we need to make sure that getClientRects() still returns something that matches legacy. You might already handle this with CulledInlineRects, but I do not understand it much.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12e458fa85e147a866fd0704908edcba16677bcd

commit 12e458fa85e147a866fd0704908edcba16677bcd
Author: Koji Ishii <kojii@chromium.org>
Date: Fri Sep 28 09:05:30 2018

[LayoutNG] Match AddSelfOutlineRects to legacy

For AddSelfOutlineRects to reach to LayoutInline that has
a continuation, this patch splits the logic in
AddSelfOutlineRects so that it can be called recursively.

In doing so, the logic flow and functions are organized to
match to the one in LayoutObject tree.

Bug: 889721
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I79e00ea7c37c94bf06e564a69fe7fc5da4f72349
Reviewed-on: https://chromium-review.googlesource.com/1246881
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595028}
[modify] https://crrev.com/12e458fa85e147a866fd0704908edcba16677bcd/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/12e458fa85e147a866fd0704908edcba16677bcd/third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.cc
[modify] https://crrev.com/12e458fa85e147a866fd0704908edcba16677bcd/third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h
[modify] https://crrev.com/12e458fa85e147a866fd0704908edcba16677bcd/third_party/blink/renderer/core/layout/ng/ng_physical_container_fragment.cc
[modify] https://crrev.com/12e458fa85e147a866fd0704908edcba16677bcd/third_party/blink/renderer/core/layout/ng/ng_physical_container_fragment.h
[modify] https://crrev.com/12e458fa85e147a866fd0704908edcba16677bcd/third_party/blink/renderer/core/layout/ng/ng_physical_fragment.cc

Tried a few possible solutions to always generate box fragments even if the line is "empty", but it fails quite a lot of tests.

https://chromium-review.googlesource.com/c/chromium/src/+/1251142
https://test-results.appspot.com/data/layout_results/linux_layout_tests_layout_ng/10155/layout-test-results/results.html

Still thinking how to solve this...

Great to see that code. I'll also take a look what's going on. Briefly looking at your 43 failures:
33 text failures looks like DumpTree diffs, to be expected with new fragments.

There are some positioning failures.

Outline failures are interestng. Looks like too much outline is being drawn...

Some of the failures suggests that there are cases where not generating fragments is the right thing to do.

Given it's all about empty lines, maybe it's easier to special-case in outline rect?

> Given it's all about empty lines, maybe it's easier to special-case in outline rect?

The problem is not collecting rects, the problem is that because LayoutInline with outlines did not generate any fragments, it does not get painted at all.
Legacy does not have this problem, because they are traversing Layout tree.
If LayoutInline does not generate any fragments, there is nothing to be painted
if we are painting fragment tree.

PaintLineBoxChildren is the only hack I can think of. It traverses LayoutObjects if line is empty.

Reviewed the failures more. I'm still leaning to special-case in outline code than to pursue the fix in #4. Many of these failures indicate it is correct not to generate fragments in these cases. We could change line breaker behavior and avoid these cases in different ways, but which is correct is unclear, and it'll be larger changes.

I will try to see how easy/hard special-casing in outline code is.

Oh, I think I understand what you're saying in #7. So if we were to special-case, we will need to special-case both in AddOutlineRects and in PaintLineBoxChildren? It makes sense then.

AddOutlineRects should work, it just calls Legacy for Children and Continuations.

PaintLineBoxChildren is the problem.

> AddOutlineRects should work

There's one failure I was tracking, and it looks like it was the only one. Please see https://chromium-review.googlesource.com/c/chromium/src/+/1252484

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73afe6fee5cecdb11a08a29dac26282efe88a30e

commit 73afe6fee5cecdb11a08a29dac26282efe88a30e
Author: Koji Ishii <kojii@chromium.org>
Date: Tue Oct 02 01:50:17 2018

[LayoutNG] Fix AddOutlineRects to include continuation when the first line is empty

This patch fixes AddOutlineRects to include continuation
when the first line is empty.

In such case, NGInlineLayoutAlgorithm suppresses box
fragments, and thus we cannot reach to the continuation by
traversing fragment tree.

This patch detects such case and traverse the first
LayoutInline instead.

Bug: 889721
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I0e3bbd895501231b76775b074b35310037a3e113
Reviewed-on: https://chromium-review.googlesource.com/1252484
Reviewed-by: Aleks Totic <atotic@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595693}
[modify] https://crrev.com/73afe6fee5cecdb11a08a29dac26282efe88a30e/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/73afe6fee5cecdb11a08a29dac26282efe88a30e/third_party/blink/renderer/core/layout/ng/ng_physical_container_fragment.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b57995bd484f196c0b8249909c4b4c5ae0eef395

commit b57995bd484f196c0b8249909c4b4c5ae0eef395
Author: Aleks Totic <atotic@chromium.org>
Date: Tue Oct 16 04:40:22 2018

[LayoutNG] Label failing tests with correct bug number

Labeling all the failures caused by #889721, "no fragment, no outline" as such.

I've also triaged rest of outline failures. I'd like to rebaseline, these
are minor inline layout pixel differences:

paint/invalidation/outline/focus-layers.html
paint/invalidation/outline/focus-ring-on-inline-continuation-move.html
paint/invalidation/outline/inline-outline-repaint-2.html
paint/invalidation/outline/outline-containing-image-in-non-standard-mode.html

I've also investigated
paint/invalidation/outline/inline-focus.html
This seems to be a real case of too much invalidation.

Bug: 889721
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Iba888ea769e54433ca1272d475baefd078bf6a12
Reviewed-on: https://chromium-review.googlesource.com/c/1281915
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599850}
[modify] https://crrev.com/b57995bd484f196c0b8249909c4b4c5ae0eef395/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66167fa7eb8f355479f1f487e47be3ec41d08c35

commit 66167fa7eb8f355479f1f487e47be3ec41d08c35
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Thu Nov 29 14:05:56 2018

[LayoutNG] Rebaseline paint/ tests.

Only internal data type differences and such things.

TBR=atotic@chromium.org,kojii@chromium.org

Bug: 889721, 835484
Change-Id: I1b46bb16b5ab39da52ad7b20241441f7a99d0fa2
Reviewed-on: https://chromium-review.googlesource.com/c/1354925
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612180}
[modify] https://crrev.com/66167fa7eb8f355479f1f487e47be3ec41d08c35/third_party/blink/web_tests/FlagExpectations/enable-blink-features=LayoutNG
[add] https://crrev.com/66167fa7eb8f355479f1f487e47be3ec41d08c35/third_party/blink/web_tests/flag-specific/enable-blink-features=LayoutNG/paint/invalidation/clip/clip-with-layout-delta-expected.txt
[add] https://crrev.com/66167fa7eb8f355479f1f487e47be3ec41d08c35/third_party/blink/web_tests/flag-specific/enable-blink-features=LayoutNG/paint/invalidation/crbug-371640-4-expected.txt
[add] https://crrev.com/66167fa7eb8f355479f1f487e47be3ec41d08c35/third_party/blink/web_tests/flag-specific/enable-blink-features=LayoutNG/paint/invalidation/crbug-371640-expected.txt
[add] https://crrev.com/66167fa7eb8f355479f1f487e47be3ec41d08c35/third_party/blink/web_tests/flag-specific/enable-blink-features=LayoutNG/paint/invalidation/outline/focus-continuations-expected.txt
[add] https://crrev.com/66167fa7eb8f355479f1f487e47be3ec41d08c35/third_party/blink/web_tests/flag-specific/enable-blink-features=LayoutNG/paint/invalidation/outline/focus-enable-continuations-expected.txt
[add] https://crrev.com/66167fa7eb8f355479f1f487e47be3ec41d08c35/third_party/blink/web_tests/flag-specific/enable-blink-features=LayoutNG/paint/invalidation/outline/focus-ring-on-continuation-move-expected.txt