Bug 196948 – Regression (r244291): Broken API Test AutoLayoutRenderingProgressRelativeOrdering

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Aakash Jain]

TestWebKitAPI.WebKit.AutoLayoutRenderingProgressRelativeOrdering seems to have start failing consistently recently.

e.g.:
https://ews-build.webkit.org/#/builders/3/builds/588
https://ews-build.webkit.org/#/builders/3/builds/589
https://ews-build.webkit.org/#/builders/3/builds/590

[Ryan Haddad]

Failed
    TestWebKitAPI.WebKit.AutoLayoutRenderingProgressRelativeOrdering
        
        /Volumes/Data/worker/macOS-High-Sierra-Release-Build-EWS/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/AutoLayoutIntegration.mm:191
        Value of: didInvalidateIntrinsicContentSize
          Actual: false
        Expected: true
        
        
        /Volumes/Data/worker/macOS-High-Sierra-Release-Build-EWS/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/AutoLayoutIntegration.mm:202
        Value of: didFirstLayout
          Actual: true
        Expected: false

[Aakash Jain]

Seems like this test is catching a real failure, which was introduced today in https://trac.webkit.org/changeset/244291

EWS indicated the failure for that patch in: https://ews-build.webkit.org/#/builders/3/builds/578

[zalan]

r244291 potentially changed the order of incoming milestones. This test might be relying on the ordering (which it is not supposed to). Will look into this shortly.

[Aakash Jain]

Created attachment 367487 [details]
Patch

[Aakash Jain]

Committed r244322: <https://trac.webkit.org/changeset/244322>

[Radar WebKit Bug Importer]

<rdar://problem/49927131>

[Ryan Haddad]

1. If a change breaks a test, we should roll it out.
2. I’m going to reopen this bug because we need to track fixing the failure somewhere.

[Aakash Jain]

(In reply to Ryan Haddad from comment #7)
> 2. I’m going to reopen this bug because we need to track fixing the failure somewhere.
Agree. I wasn't planning to close it. I used webkit-patch command which automatically closed it.

[Ryan Haddad]

(In reply to Aakash Jain from comment #8)
> (In reply to Ryan Haddad from comment #7)
> > 2. I’m going to reopen this bug because we need to track fixing the failure somewhere.
> Agree. I wasn't planning to close it. I used webkit-patch command which
> automatically closed it.
No problem! Just didn't want it to slip through the cracks.

[Ryan Haddad]

Alan, do we have a fix for this or should we just roll out the breaking change?

[zalan]

(In reply to Ryan Haddad from comment #10)
> Alan, do we have a fix for this or should we just roll out the breaking
> change?
We do have a fix for this and will most likely be landing it today.

[zalan]

Created attachment 367685 [details]
Patch

[zalan]

WIP ^^

[zalan]

Created attachment 367715 [details]
Patch

[zalan]

Created attachment 367732 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 367732 [details]
Patch

Clearing flags on attachment: 367732

Committed r244434: <https://trac.webkit.org/changeset/244434>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Aakash Jain]

> Committed r244434: <https://trac.webkit.org/changeset/244434>
Seems like it broke another API Test: WebKit.AutoLayoutIntegration
https://ews-build.webkit.org/#/builders/3/builds/743

[zalan]

(In reply to Aakash Jain from comment #18)
> > Committed r244434: <https://trac.webkit.org/changeset/244434>
> Seems like it broke another API Test: WebKit.AutoLayoutIntegration
> https://ews-build.webkit.org/#/builders/3/builds/743
:( most likely that test needs fixing too.

[Aakash Jain]

> :( most likely that test needs fixing too.
ok. Test failures makes EWS slow (since EWS has to re-run the tests, and then compile clean tree and re-run treats on clean tree, to ensure that the failures are not flaky or pre-existing).
Can we either fix this test quickly or roll out the commit?

[WebKit Commit Bot]

Re-opened since this is blocked by bug 197089

[Alexey Proskuryakov]

What is the current status here? Has the fix been rolled back for breaking another test, so we are back to having AutoLayoutRenderingProgressRelativeOrdering fail?

[zalan]

(In reply to Alexey Proskuryakov from comment #22)
> What is the current status here? Has the fix been rolled back for breaking
> another test, so we are back to having
> AutoLayoutRenderingProgressRelativeOrdering fail?
Correct.

[zalan]

Created attachment 368187 [details]
Patch

[Tim Horton]

Comment on attachment 368187 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=368187&action=review

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:6244
> +        // Ensure we never send DidFirstLayout milestone withouth updating the intrinsic size.

withouth

[zalan]

Created attachment 368191 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 368191 [details]
Patch

Clearing flags on attachment: 368191

Committed r244633: <https://trac.webkit.org/changeset/244633>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.