Status of LayoutNGPaintFragments for inline children

[Koji Ishii]

Hi all, I'd like to update the status of LayoutNGPaintFragments for inline children.

We've been thinking this flag to be short-lived, and enabled by default when LayoutNG is enabled, but too many failures hesitated us to do so. Currently, if enabled on layout_ng bots, it'll fail ~10,000 additional tests.

Good news is that we're making great progress:

• DrawingRecorder crash fix is under review, and this is supposed to fix ~7,000.
• yosin@ and xiaochenghu@ are working on ContainerNode::GetUpperLeftCorner() crashes. This is supposed to fix ~300.
• They are also working on NGInlineNode::ComputeOffsetMappingIfNeeded() crashes. This is supposed to fix ~20.
• Images are placed incorrectly, breaks a lot of ref-tests. Two fixes needed, when combined, supposed to fix ~300.
• A good number of image tests need rebaselines due to crbug.com/787315. Exact numbers not clear.

I'm hoping the number of additional failures will go down to 1,000-2,000 soon, and at that point, I'd like us to enable LayoutNGPaintFragments by default whenever LayoutNG is enabled.

If you're locally debugging with LayoutNGPaintFragments is enabled but troubled by too many crashes, please take the WIP listed above locally and most of them should be gone. I'm hoping to land these fixes in a week or so and enable LayoutNGPaintFragments in early December.

[Yoichi Osato]

That means LayoutTests/FlagExpectation/enable-blink-features=LayoutNG reflects LayoutNGPaintFragment and layout_ng_paint in VirtualTestSuites gone?

2017年11月21日(火) 15:26 Koji Ishii <ko...@chromium.org>:

--
You received this message because you are subscribed to the Google Groups "layout-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to layout-dev+...@chromium.org.
To post to this group, send email to layou...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/layout-dev/CACQRE%2BSCi6G5_SOCN_YKrXUHnoSu_xmv5F2JVs%2BzjNo%2BmF9iSQ%40mail.gmail.com.

[Koji Ishii]

Yes, I think we can remove the flag itself, and all its related stuff.

2017-11-22 11:26 GMT+09:00 Yoichi Osato <yoi...@chromium.org>:

That means LayoutTests/FlagExpectation/enable-blink-features=LayoutNG reflects LayoutNGPaintFragment and layout_ng_paint in VirtualTestSuites gone?

2017年11月21日(火) 15:26 Koji Ishii <ko...@chromium.org>:

Hi all, I'd like to update the status of LayoutNGPaintFragments for inline children.

We've been thinking this flag to be short-lived, and enabled by default when LayoutNG is enabled, but too many failures hesitated us to do so. Currently, if enabled on layout_ng bots, it'll fail ~10,000 additional tests.

Good news is that we're making great progress:

• DrawingRecorder crash fix is under review, and this is supposed to fix ~7,000.
• yosin@ and xiaochenghu@ are working on ContainerNode::GetUpperLeftCorner() crashes. This is supposed to fix ~300.
• They are also working on NGInlineNode::ComputeOffsetMappingIfNeeded() crashes. This is supposed to fix ~20.
• Images are placed incorrectly, breaks a lot of ref-tests. Two fixes needed, when combined, supposed to fix ~300.
• A good number of image tests need rebaselines due to crbug.com/787315. Exact numbers not clear.

I'm hoping the number of additional failures will go down to 1,000-2,000 soon, and at that point, I'd like us to enable LayoutNGPaintFragments by default whenever LayoutNG is enabled.

If you're locally debugging with LayoutNGPaintFragments is enabled but troubled by too many crashes, please take the WIP listed above locally and most of them should be gone. I'm hoping to land these fixes in a week or so and enable LayoutNGPaintFragments in early December.

--
You received this message because you are subscribed to the Google Groups "layout-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to layout-dev+unsubscribe@chromium.org.

To post to this group, send email to layou...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/layout-dev/CACQRE%2BSCi6G5_SOCN_YKrXUHnoSu_xmv5F2JVs%2BzjNo%2BmF9iSQ%40mail.gmail.com.

--

You received this message because you are subscribed to the Google Groups "layout-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to layout-dev+unsubscribe@chromium.org.

To post to this group, send email to layou...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/layout-dev/CAAEV3pk__K1cSQy%2B9L-zzh0H7kE0zyf4rZL1q%2BTRtwCSjXNgGg%40mail.gmail.com.

[Koji Ishii]

Hi, here's status updates.

The DrawingRecorder crash fix landed thanks to Chris. With a few more CLs under reviews (1, 2, 3), the number of additional failures is now down to 1,805. Reviewing results, I think we're at the level that enabling LayoutNGPaintFragments on ng bots would not trouble layout-devs. It'd help us if you could review this try bot results and see whether tests that may block you are included or not.

With the feedback and when Emil is back, I hope to discuss the plan to enable it.

[Koji Ishii]

Updates again.

We discussed this yesterday and agreed to enable LayoutNGPaintFragments flag by default. I also discussed with Chris and Emil that we should enable Slimming Paint v175 when working on LayoutNG. That means 3 flags should be used:

  --enable-blink-features=LayoutNG

  --enable-blink-features=LayoutNGPaintFragments

  --enable-slimming-paint-v175

This could be shorten to 2 flags:

  --enable-blink-features=LayoutNG,LayoutNGPaintFragments

  --enable-slimming-paint-v175

but when running layout tests locally, this will not pick up "FlagExpectations/enable-blink-features=LayoutNG" as far as I tested.

I'm working on enabling these flags on ng bots, and also to convert "virtual/layout_ng" tests to use these flags.

Currently, ~2000 tests still fail. We once went down to ~1500, but some fixes regress other tests. When the former CL lands, ng bots will be a bit unstable, and it'll probably take a few maintenance cycles. I hope it's short though, appreciate your patience in advance.

I know some of causes and working on fixes. I hope the number should go down once we've got bots to prevent unexpected regressions, but if anyone has any tests that should have priority to fix, please let me know.

[Aleks Totic]

That is a lot of new flags. For those of us switching from plain LayoutNG, could you give us a short usage intro:

- what is broken with new flags? 

- what new kinds of failures should not be surprising? I've noticed that some text just does not show up.

- what is slimming paint v175? What do I need to know when I am stepping in the debugger?

Aleks

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/layout-dev/CACQRE%2BRFtFOXb7puEz6mU1cKvJ%2B4fJRAcY9zeV-yk861cUkVkQ%40mail.gmail.com.

[Chris Harrelson]

BTW you can make LayoutNG imply SlimmingPaintV175 in runtime_enabled_features.json5 and not have to remember commandline constraints.

On Thu, Dec 7, 2017 at 1:07 PM, Aleks Totic <ato...@google.com> wrote:

That is a lot of new flags. For those of us switching from plain LayoutNG, could you give us a short usage intro:

- what is broken with new flags? 

- what new kinds of failures should not be surprising? I've noticed that some text just does not show up.

- what is slimming paint v175? What do I need to know when I am stepping in the debugger?

You don't have to know anything. We're almost done with burning down failing tests for it. If a test fails for a mysterious reason in paint, best thing to do is to raise it with a paint team member.

[Koji Ishii]

Yeah, if you prefer modifying your repo rather than writing a shell script, please refer to this CL (uses runtime_enabled_features.json5 but without SlimmingPaintV175, works for both content shell and tests) or this CL (only for layout tests.)

The change to run_webkit_tests.py is needed to run layout test so that it picks up FlagExpectations/enable-blink-features=LayoutNG.

[Koji Ishii]

Both the layout_ng bots change and the virtual/layout_ng change landed, thank you all for the support to move this forward!

I've ran 2 cycles of updating FlagExpectations, ToT has ~10 flaky failures and ~100 passes (probably from fixes landed recently.) I'm currently running the 3rd cycle, hopefully it should be stable by the morning in US time.

I'll also doing cleanup for virtual/layout_ng. Recent CLs fixed severals, and some are just about rebaselines due to slight difference (improvements!) in text rendering.

We probably have more flaky expectations for a while, because we don't have the historical results to deflake. This means we have more flaky expectations (i.e.., [ Failure Pass ]) than necessary. This would take ~1 week to stabilize.

Thank you again, all!

[Chris Harrelson]

On Thu, Dec 7, 2017 at 8:53 PM, Koji Ishii <ko...@chromium.org> wrote:

Yeah, if you prefer modifying your repo rather than writing a shell script, please refer to this CL (uses runtime_enabled_features.json5 but without SlimmingPaintV175, works for both content shell and tests) or this CL (only for layout tests.)

I didn't mean that. I meant that you can commit a change to that file to make the flags turn on automatically. For example, SPv2 implies SPv175 already.

[Koji Ishii]

I didn't know "implied_by", looks easier, thank you!