Bug 191783

Summary: REGRESSION (r238294): TestWebKitAPI.WKNavigation.ProcessCrashDuringCallback failing on iOS
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: New BugsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, rniwa, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=189148
Attachments:
Description Flags
Allow both errors none

Description Ryan Haddad 2018-11-16 15:25:20 PST 
The following API test is failing on iOS after https://trac.webkit.org/changeset/238294:

    TestWebKitAPI.WKNavigation.ProcessCrashDuringCallback
        
        /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:269
        Expected equality of these values:
          WKErrorWebContentProcessTerminated
            Which is: 2
          error.code
            Which is: 3
        
        
        /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:262
        Expected equality of these values:
          WKErrorWebContentProcessTerminated
            Which is: 2
          error.code
            Which is: 3
        
        
        /Volumes/Data/slave/ios-simulator-12-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:255
        Expected equality of these values:
          WKErrorWebContentProcessTerminated
            Which is: 2
          error.code
            Which is: 3
        
https://build.webkit.org/builders/Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/1015/steps/run-api-tests/logs/stdio
Comment 1 Ryosuke Niwa 2018-11-16 17:26:38 PST 
Looking into this now.
Comment 2 Ryosuke Niwa 2018-11-16 19:07:50 PST 
Hm... I can't reproduce this failure locally.
Comment 3 Chris Dumez 2018-11-16 19:08:40 PST 
(In reply to Ryosuke Niwa from comment #2)
> Hm... I can't reproduce this failure locally.

Probably flaky?
Comment 4 Ryosuke Niwa 2018-11-16 19:28:39 PST 
(In reply to Chris Dumez from comment #3)
> (In reply to Ryosuke Niwa from comment #2)
> > Hm... I can't reproduce this failure locally.
> 
> Probably flaky?

Possibly. I've ran it like 5 times and couldn't repro. I am using debug instead of release builds so I'm gonna try release now. Sigh... another 40min wait...
Comment 5 Chris Dumez 2018-11-16 19:40:22 PST 
(In reply to Ryosuke Niwa from comment #4)
> (In reply to Chris Dumez from comment #3)
> > (In reply to Ryosuke Niwa from comment #2)
> > > Hm... I can't reproduce this failure locally.
> > 
> > Probably flaky?
> 
> Possibly. I've ran it like 5 times and couldn't repro. I am using debug
> instead of release builds so I'm gonna try release now. Sigh... another
> 40min wait...

Honestly, the test is calling [view _close] so getting a WKErrorWebViewInvalidated error instead of WKErrorWebContentProcessTerminated is not technically wrong.

I think we should allow both. It is not the first time this test has started failing with WKErrorWebViewInvalidated errors, see Bug 189148.
Comment 6 Chris Dumez 2018-11-16 19:41:01 PST 
(In reply to Chris Dumez from comment #5)
> (In reply to Ryosuke Niwa from comment #4)
> > (In reply to Chris Dumez from comment #3)
> > > (In reply to Ryosuke Niwa from comment #2)
> > > > Hm... I can't reproduce this failure locally.
> > > 
> > > Probably flaky?
> > 
> > Possibly. I've ran it like 5 times and couldn't repro. I am using debug
> > instead of release builds so I'm gonna try release now. Sigh... another
> > 40min wait...
> 
> Honestly, the test is calling [view _close] so getting a
> WKErrorWebViewInvalidated error instead of
> WKErrorWebContentProcessTerminated is not technically wrong.
> 
> I think we should allow both. It is not the first time this test has started
> failing with WKErrorWebViewInvalidated errors, see Bug 189148.

I wrote the test and what it was really testing was not the error code but the fact that all callbacks get invalidated.
Comment 7 Chris Dumez 2018-11-16 19:41:30 PST 
(In reply to Chris Dumez from comment #6)
> (In reply to Chris Dumez from comment #5)
> > (In reply to Ryosuke Niwa from comment #4)
> > > (In reply to Chris Dumez from comment #3)
> > > > (In reply to Ryosuke Niwa from comment #2)
> > > > > Hm... I can't reproduce this failure locally.
> > > > 
> > > > Probably flaky?
> > > 
> > > Possibly. I've ran it like 5 times and couldn't repro. I am using debug
> > > instead of release builds so I'm gonna try release now. Sigh... another
> > > 40min wait...
> > 
> > Honestly, the test is calling [view _close] so getting a
> > WKErrorWebViewInvalidated error instead of
> > WKErrorWebContentProcessTerminated is not technically wrong.
> > 
> > I think we should allow both. It is not the first time this test has started
> > failing with WKErrorWebViewInvalidated errors, see Bug 189148.
> 
> I wrote the test and what it was really testing was not the error code but
> the fact that all callbacks get invalidated.

Whether we get WKErrorWebContentProcessTerminated or WKErrorWebViewInvalidated error seems to be a matter of timing.
Comment 8 Ryosuke Niwa 2018-11-16 20:21:02 PST 
Created attachment 355176 [details]
Allow both errors
Comment 9 Chris Dumez 2018-11-16 20:21:44 PST 
Comment on attachment 355176 [details]
Allow both errors

r=me
Comment 10 Ryosuke Niwa 2018-11-16 20:23:33 PST 
Comment on attachment 355176 [details]
Allow both errors

Clearing flags on attachment: 355176

Committed r238340: <https://trac.webkit.org/changeset/238340>
Comment 11 Ryosuke Niwa 2018-11-16 20:23:36 PST 
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2018-11-16 20:24:24 PST 
<rdar://problem/46146920>