Bug 184893 – [iOS] API test ScrollViewInsetTests.InnerHeightWithLargeTopContentInset is a flaky failure

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ryan Haddad]

API test ScrollViewInsetTests.InnerHeightWithLargeTopContentInset is a flaky failure on iOS Simulator

FAIL ScrollViewInsetTests.InnerHeightWithLargeTopContentInset

/Volumes/Data/slave/ios-simulator-11-release/build/Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm:86
Value of: [[webView objectByEvaluatingJavaScript:@"innerHeight"] floatValue]
  Actual: 100
Expected: viewHeight
Which is: 500

https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20%28Tests%29/builds/4516

[Wenson Hsieh]

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.

[Radar WebKit Bug Importer]

<rdar://problem/39747271>

[Chris Dumez]

Created attachment 341623 [details]
Patch

[Wenson Hsieh]

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:.

[Chris Dumez]

(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.

[Chris Dumez]

(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.

[Chris Dumez]

Created attachment 341664 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 341664 [details]
Patch

Clearing flags on attachment: 341664

Committed r232351: <https://trac.webkit.org/changeset/232351>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.