New issue
Advanced search Search tips

Issue 922419 link

Starred by 1 user

Issue metadata

Status: Fixed
Merged: issue 693741
Owner:
Closed: Jan 17
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[LayoutNG] All/FrameThrottlingTest.ThrottleSubtreeAtomically/2 fails

Project Member Reported by mstensho@chromium.org, Jan 16 (6 days ago)

Issue description

Comment 1 by atotic@chromium.org, Jan 16 (6 days ago)

Mergedinto: 693741
Status: Duplicate (was: Assigned)
This is ok. According to @pdr's comment in the CL:
>>>>>
 this failure is due to https://crbug.com/693741, there is a TODO in PaintPropertyTreeBuilder::NeedsScrollNode to fix this for CompositeAfterPaint. Please skip this test or otherwise mark it as failing and proceed with this patch.
<<<<<

Comment 2 by mstensho@chromium.org, Jan 16 (6 days ago)

Status: Assigned (was: Duplicate)
Then the test should be skipped (disabled) in the meantime.

Comment 3 by atotic@chromium.org, Jan 16 (6 days ago)

Is there a way to disable webkit_unit_test for NG only? Only suggestions I got were to just add code that bails out if LayoutNG is on.

Comment 4 by mstensho@chromium.org, Jan 16 (6 days ago)

Yes, that's the only way I know of as well. That sounds good to me.

Comment 5 by atotic@chromium.org, Jan 16 (6 days ago)

I'd rather not do that. 

If I just add code that bails out if NG, there will be no way to tell that test is failing in NG. How would we know it needs to be fixed?

We already have a bunch of NG failing webkit_unit_tests. Why not have another one fail?

Comment 6 by mstensho@chromium.org, Jan 16 (6 days ago)

That bunch is shrinking. I'm working on bringing it down to zero. :)
At some point it will be impossible to land with failing tests.

As far as I understand, the failures currently force the trybot to run every job both with and without the patch, to figure out if a CL introduces new failures or not. Wasted time.

See e.g. https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_layout_tests_layout_ng/13761
(that redness isn't really that pretty either)

There's no mechanism to expect failures for unit tests, so disabling them is the normal way to do it.

Comment 7 by atotic@chromium.org, Jan 17 (6 days ago)

According to @pdr: "LayoutNG can launch without fixing this bug. CompositeAfterPaint is quite a ways out."

Comment 8 by mstensho@chromium.org, Jan 17 (5 days ago)

Ok, let's disable it for NG then.
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 17 (5 days ago)

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

commit 58e1a8c55cd00d6010cc2cadd34b2481545dc582
Author: Aleks Totic <atotic@chromium.org>
Date: Thu Jan 17 07:19:40 2019

[LayoutNG] Disable webkit_unit_test ThrottleSubtreeAtomically

Bug:  922419 
Change-Id: Ibaeb2a9cf9ba5edb550fc151e77371ff0233c7e3
Reviewed-on: https://chromium-review.googlesource.com/c/1415694
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623613}
[modify] https://crrev.com/58e1a8c55cd00d6010cc2cadd34b2481545dc582/third_party/blink/renderer/core/scheduler/frame_throttling_test.cc

Comment 10 by atotic@chromium.org, Jan 17 (5 days ago)

Status: Fixed (was: Assigned)

Sign in to add a comment