WKNavigation.ProcessCrashDuringCallback is failing after r235321, which seems to be an irrelevant change. Add some error information to help debug the failure.
Created attachment 348242 [details] Patch
Created attachment 348304 [details] Patch
What does the failure look like?
(In reply to Chris Dumez from comment #3) > What does the failure look like? https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK2%20%28Tests%29/builds/4651/steps/run-api-tests/logs/stdio /Volumes/Data/slave/highsierra-debug/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:232 Value of: !!error Actual: false Expected: true
Comment on attachment 348304 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348304&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:233 > + EXPECT_EQ(error.code, WKErrorWebContentProcessTerminated); I believe the parameters are reversed. Left side should be expected value iirc.
Created attachment 348320 [details] Patch
(In reply to Chris Dumez from comment #5) > Comment on attachment 348304 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=348304&action=review > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:233 > > + EXPECT_EQ(error.code, WKErrorWebContentProcessTerminated); > > I believe the parameters are reversed. Left side should be expected value > iirc. I saw this WKContentExtensionStoreStore.mm so I thought it's correct... Fixed.
(In reply to Sihui Liu from comment #7) > (In reply to Chris Dumez from comment #5) > > Comment on attachment 348304 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=348304&action=review > > > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:233 > > > + EXPECT_EQ(error.code, WKErrorWebContentProcessTerminated); > > > > I believe the parameters are reversed. Left side should be expected value > > iirc. > > I saw this WKContentExtensionStoreStore.mm so I thought it's correct... > Fixed. Yes, people get it wrong all the time.
Comment on attachment 348320 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348320&action=review r=me > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4576 > + NSError* callbackError = [NSError errorWithDomain:WKErrorDomain code:static_cast<int>(error) userInfo:nil]; FYI, we use star on right size for NS types. But in this case, I'd rather we drop this local variable completely: completionHandlerBlock(nil, [NSError errorWithDomain:WKErrorDomain code:static_cast<int>(error) userInfo:nil]); > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4591 > + NSError* callbackError = [NSError errorWithDomain:WKErrorDomain code:static_cast<int>(error) userInfo:nil]; ditto. > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4605 > + NSError* callbackError = [NSError errorWithDomain:WKErrorDomain code:static_cast<int>(error) userInfo:nil]; ditto.
Created attachment 348338 [details] Patch for landing
Comment on attachment 348338 [details] Patch for landing Clearing flags on attachment: 348338 Committed r235440: <https://trac.webkit.org/changeset/235440>
All reviewed patches have been landed. Closing bug.
<rdar://problem/43814405>
Do we now have the needed information to resolve the issue? This test is consistently failing on iOS bots. https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20%28Tests%29/builds/7110/steps/run-api-tests/logs/stdio
(In reply to Ryan Haddad from comment #14) > Do we now have the needed information to resolve the issue? This test is > consistently failing on iOS bots. > > https://build.webkit.org/builders/ > Apple%20iOS%2011%20Simulator%20Release%20WK2%20%28Tests%29/builds/7110/steps/ > run-api-tests/logs/stdio Probably the WebView is getting destroyed *before* we realize the WebProcess has exited. When The WebView gets destroyed, we call WebPageProxy::close() which causes error WKErrorWebViewInvalidated as we see in the new test output.
(In reply to Chris Dumez from comment #15) > (In reply to Ryan Haddad from comment #14) > > Do we now have the needed information to resolve the issue? This test is > > consistently failing on iOS bots. > > > > https://build.webkit.org/builders/ > > Apple%20iOS%2011%20Simulator%20Release%20WK2%20%28Tests%29/builds/7110/steps/ > > run-api-tests/logs/stdio > > Probably the WebView is getting destroyed *before* we realize the WebProcess > has exited. When The WebView gets destroyed, we call WebPageProxy::close() > which causes error WKErrorWebViewInvalidated as we see in the new test > output. We should probably drop this line: webView = nullptr; Not sure why I did this in the first place.