Bug 190401 – [iOS Debug] Assertion failure in NSComparisonResult UIContentSizeCategoryCompareToCategory()

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Dawei Fenton (:realdawei)]

API test TestWebKitAPI.EditorStateTests.TypingAttributesTextAlignmentAbsoluteAlignmentOptions is crashing on iOS 12 [Debug]

It crashes ~50 percent of the time.

Sample output:
https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Debug%20WK2%20%28Tests%29/builds/221/steps/run-api-tests/logs/stdio

    TestWebKitAPI.EditorStateTests.TypingAttributesTextAlignmentAbsoluteAlignmentOptions
        2018-10-08 10:01:47.818 TestWebKitAPI[14259:247411] *** Assertion failure in NSComparisonResult UIContentSizeCategoryCompareToCategory(__strong UIContentSizeCategory _Nonnull, __strong UIContentSizeCategory _Nonnull)(), /BuildRoot/Library/Caches/com.apple.xbs/Sources/UIKitCore_Sim/UIKit-3698.84.15/UIContentSizeCategory.m:87
        Child process terminated with signal 11: Segmentation fault

[Ryan Haddad]

Created attachment 351892 [details]
Crash log

[Ryan Haddad]

Dawei, is this reproducible locally?

[Dawei Fenton (:realdawei)]

(In reply to Ryan Haddad from comment #2)
> Dawei, is this reproducible locally?

Attempting this now

[Dawei Fenton (:realdawei)]

This is reproducible with the following command:

While true; do run-api-tests --debug --ios-simulator TestWebKitAPI.EditorStateTests.TypingAttributesTextAlignmentAbsoluteAlignmentOptions ;done

[Wenson Hsieh]

I suspect that making TestWebKitAPI a proper UI application would fix this.

[Jonathan Bedard]

(In reply to Wenson Hsieh from comment #5)
> I suspect that making TestWebKitAPI a proper UI application would fix this.

Making it a UI application would prevent a flakey SEGFAULT? That would strike me as very strange.

Also, are we sure that's the right crashlog? The stack trace isn't pointing to that UIKit assertion error, it's a C++ string/stream error in our code.

[Ryan Haddad]

(In reply to Jonathan Bedard from comment #6)
> (In reply to Wenson Hsieh from comment #5)
> Also, are we sure that's the right crashlog? The stack trace isn't pointing
> to that UIKit assertion error, it's a C++ string/stream error in our code.

That is the one that matched up with the timestamp for the assertion failure in one of the examples I saw. I just found a more recent one and verified that it looks the same.

[Radar WebKit Bug Importer]

<rdar://problem/45195285>

[Wenson Hsieh]

(In reply to Jonathan Bedard from comment #6)
> (In reply to Wenson Hsieh from comment #5)
> > I suspect that making TestWebKitAPI a proper UI application would fix this.
> 
> Making it a UI application would prevent a flakey SEGFAULT? That would
> strike me as very strange.

Yes, I strongly suspect this is the case. We're hitting this exception because UIKit can't figure out what scale to apply to an icon image for a button in the callout bar when focusing an editable element.

UIKit attempts to determine this scale by inspecting -[UIApplication preferredContentSizeCategory], which in the case of these API tests, returns `nil`. However, in WebKitTestRunner, this returns a non-null string, so we don't hit this exception. So I think we either need to make TestWebKitAPI a UI application to address this, or pretend to make it an app by swizzling some (carefully chosen) SPI.

> 
> Also, are we sure that's the right crashlog? The stack trace isn't pointing
> to that UIKit assertion error, it's a C++ string/stream error in our code.

The crash log does not seem relevant to the exception here; I've pasted an example trace in the radar.

[Alexey Proskuryakov]

This may imply that the test itself is flaky, as it doesn't always hit the code path that asserts.

[Wenson Hsieh]

(In reply to Alexey Proskuryakov from comment #10)
> This may imply that the test itself is flaky, as it doesn't always hit the
> code path that asserts.

I think this just means that the test sometimes finishes before UIKit decides to show the callout bar.

[Ryan Haddad]

Seeing something similar with another test:

    TestWebKitAPI.WKAttachmentTests.CopyAndPasteBetweenWebViews
        2018-11-05 16:33:02.855 TestWebKitAPI[33977:6055524] *** Assertion failure in NSComparisonResult UIContentSizeCategoryCompareToCategory(__strong UIContentSizeCategory _Nonnull, __strong UIContentSizeCategory _Nonnull)(), /BuildRoot/Library/Caches/com.apple.xbs/Sources/UIKitCore_Sim/UIKit-3698.84.15/UIContentSizeCategory.m:87
        Child process terminated with signal 11: Segmentation fault

https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/759/steps/run-api-tests/logs/stdio

[Ryan Haddad]

And another :( 

    TestWebKitAPI.WKWebViewEditActions.PasteAndMatchStyle
        2018-11-14 15:03:50.448 TestWebKitAPI[57358:101711663] *** Assertion failure in NSComparisonResult UIContentSizeCategoryCompareToCategory(__strong UIContentSizeCategory _Nonnull, __strong UIContentSizeCategory _Nonnull)(), /BuildRoot/Library/Caches/com.apple.xbs/Sources/UIKitCore_Sim/UIKit-3698.84.15/UIContentSizeCategory.m:87
        Child process terminated with signal 11: Segmentation fault

[Ryan Haddad]

This is making it difficult to spot legitimate regressions, as there is almost always at least one test hitting this issue.

[Wenson Hsieh]

(In reply to Ryan Haddad from comment #14)
> This is making it difficult to spot legitimate regressions, as there is
> almost always at least one test hitting this issue.

I think that ideally, this should be fixed by making TestWebKitAPI a real UI application.

But perhaps it's worth checking in a trivial workaround for this crash in the meantime, so that API test results aren't muddled by these assertions. We could then remove the workaround after the former is done. Does that sound reasonable?

[Ryan Haddad]

(In reply to Wenson Hsieh from comment #15)
> (In reply to Ryan Haddad from comment #14)
> > This is making it difficult to spot legitimate regressions, as there is
> > almost always at least one test hitting this issue.
> 
> I think that ideally, this should be fixed by making TestWebKitAPI a real UI
> application.
> 
> But perhaps it's worth checking in a trivial workaround for this crash in
> the meantime, so that API test results aren't muddled by these assertions.
> We could then remove the workaround after the former is done. Does that
> sound reasonable?
Since I don't think moving TestWebKitAPI to a UI application is happening soon (someone correct me if I am wrong), I think it is reasonable to add a workaround.

[Jonathan Bedard]

(In reply to Ryan Haddad from comment #16)
> ...
> Since I don't think moving TestWebKitAPI to a UI application is happening
> soon (someone correct me if I am wrong), I think it is reasonable to add a
> workaround.

Unless someone other than me is moving it to a UI application, it will be May/June 2019.

[Wenson Hsieh]

Created attachment 355248 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 355248 [details]
Patch

Clearing flags on attachment: 355248

Committed r238380: <https://trac.webkit.org/changeset/238380>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.