More in fast/table:

NEEDBUG fast/table/backgr_border-table-collapsed-border.html [ Crash ]
NEEDBUG fast/table/backgr_border-table-quirks-collapsed-border.html [ Crash ]
NEEDBUG fast/table/backgr_layers-show-collapsed-border.html [ Crash ]
NEEDBUG fast/table/backgr_simple-table-cell-collapsed-border.html [ Crash ]

atotic pointed out that

fast/table/click-near-anonymous-table.html
external/wpt/pointerevents/pointerevent_touch-action-table-test_touch-manual.html

also crash after https://chromium-review.googlesource.com/c/chromium/src/+/1054002/14/third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc landed yesterday.

LayoutNGMixin::NodeAtPoint uses fragments for hit testing when the Mixin has a PaintFragment set.
https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/ng/la
yout_ng_mixin.cc?type=cs&sq=package:chromium&g=0&l=230

Paint does something similar but has a further check in NGBoxFragmentPainter::Paint that uses position info from LayoutBox if !PhysicalFragment().IsPlacedByLayoutNG(), which checks that the parent is also LayoutNG.
https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc?type=cs&q=NGBoxFragmentPainter::Paint&g=0&l=80

Potential solutions:

(1) override LayoutNGTableCaption::SetPaintFragment to do nothing so that legacy hit testing is used
 - this would also make legacy painting be used instead of NGBlockFlowPainter

(2) Augment the `PaintFragment() ? fragments : legacy` check in LayoutNGMixin<Base>::NodeAtPoint to also use legacy if !PhysicalFragment().IsPlacedByLayoutNG():
`(PaintFragment() && PhysicalFragment().IsPlacedByLayoutNG()) ? fragments : legacy`

(3) Make legacy table layout set the caption fragment's offset the same time it sets caption.SetLogicalLocation.
https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/layout_table.cc?q=file:layout_table++lang:c%2B%2B&sq=package:chromium&dr&l=471

Or, equivalently, set the offset from LayoutTableCaption in an overridden SetLogicalLocation method. Oh, LayoutBox::SetLogicalLocation isn't virtual so that's out.


+atotic, who recommended (3), to see if he still recommends it in light of the (1) or (2) hacks.

(3) Doesnt seem hard, it looks like just repackaging the positioning logic at the end of LayoutNGBlockFlow::UpdateBlockLayout.


Aside:
LayoutTableCell sets a fragment position that is known to be wrong, so I don't know why its hit testing tests didn't go from pass to fail when https://chromium-review.googlesource.com/c/chromium/src/+/1054002/14/third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc landed.

Thanks for presenting all the options. #2 is my favorite. There are probably other places in Legacy where Offset will not be set, and that will handle them all.

What surprised me was that PhysicalFragment was not placed after UpdateLayout(). Might want to add a comment to PhysicalFragment::Offset declaration:

"Offset is not guaranteed to be set after Layout for all fragments.
Fragments that are IsPlacedByLayoutNG() are guaranteed to have offset.
Use GetLayoutObject()->Location for fragments !IsPlacedByLayoutNG()"

also notify LayoutNG devs. I think "Offset is always valid after layout" is a common assumption.

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

commit f964dc1f567b431d77fd4fd8669f16b61557fd47
Author: Aleks Totic <atotic@chromium.org>
Date: Thu May 17 19:30:53 2018

[LayoutNG] Fix NodeAtPoint offset crashes

Fragments not placed by LayoutNG are not guaranteed to have an Offset().
Use Location instead.

Bug:  843512 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Ie43f2e0ab8f01258db8ea91fc71c1466abd11457
Reviewed-on: https://chromium-review.googlesource.com/1064597
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559640}
[modify] https://crrev.com/f964dc1f567b431d77fd4fd8669f16b61557fd47/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/f964dc1f567b431d77fd4fd8669f16b61557fd47/third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc