Summary: | [iOS] API test ScrollViewInsetTests.InnerHeightWithLargeTopContentInset is a flaky failure | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryan Haddad <ryanhaddad> | ||||||
Component: | New Bugs | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, cdumez, commit-queue, thorton, webkit-bug-importer, wenson_hsieh | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=183648 | ||||||||
Attachments: |
|
Description
Ryan Haddad
2018-04-23 11:24:05 PDT
On our internal iOS bots (those of which are not on iOS 11.3), this test seems to be passing all the time. In the past 2 weeks, this test was run on that build train 500+ times with no failures. I wonder if something's different about our OpenSource bots...I'll try running these tests with an iOS 11.3 SDK and see if I can reproduce. Created attachment 341623 [details]
Patch
Comment on attachment 341623 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341623&action=review > Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm:81 > + int tries = 0; Do we really need a max number of tries here? I think letting it just time out isn't a bad thing, per se... Also, perhaps this would just work if we waited until the next *visible content rect* update instead of the next remote layer tree commit, since visible content rect update is the mechanism by which we account for scroll view content insets. It's possible we don't even need the "wait-until-done" loop! There's WKWebView SPI to do this: see -_doAfterNextVisibleContentRectUpdate:. (In reply to Wenson Hsieh from comment #4) > Comment on attachment 341623 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=341623&action=review > > > Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm:81 > > + int tries = 0; > > Do we really need a max number of tries here? I think letting it just time > out isn't a bad thing, per se... > I have been told the opposite several times. We prefer quick failures over time outs. > Also, perhaps this would just work if we waited until the next *visible > content rect* update instead of the next remote layer tree commit, since > visible content rect update is the mechanism by which we account for scroll > view content insets. It's possible we don't even need the "wait-until-done" > loop! There's WKWebView SPI to do this: see > -_doAfterNextVisibleContentRectUpdate:. Ok i will try tomorrow. (In reply to Chris Dumez from comment #5) > (In reply to Wenson Hsieh from comment #4) > > Comment on attachment 341623 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=341623&action=review > > > > > Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm:81 > > > + int tries = 0; > > > > Do we really need a max number of tries here? I think letting it just time > > out isn't a bad thing, per se... > > > > I have been told the opposite several times. We prefer quick failures over > time outs. > > > Also, perhaps this would just work if we waited until the next *visible > > content rect* update instead of the next remote layer tree commit, since > > visible content rect update is the mechanism by which we account for scroll > > view content insets. It's possible we don't even need the "wait-until-done" > > loop! There's WKWebView SPI to do this: see > > -_doAfterNextVisibleContentRectUpdate:. > > Ok i will try tomorrow. _doAfterNextVisibleContentRectUpdate does not fix the flakiness. Created attachment 341664 [details]
Patch
Comment on attachment 341664 [details] Patch Clearing flags on attachment: 341664 Committed r232351: <https://trac.webkit.org/changeset/232351> All reviewed patches have been landed. Closing bug. |