Bug 160587 – Write API test to cover crash fix in r204135

• Component: WebKit2
• Status: REOPENED

[Chris Dumez]

Write API test to cover crash fix in r204135.

One way I managed to reproduce was by destroying another WKWebView sharing the same WebContent process in the webViewWebContentProcessDidTerminate callback.

[Chris Dumez]

Created attachment 285397 [details]
Patch

[Chris Dumez]

Was a pain to reproduce but I finally managed.

[WebKit Commit Bot]

Comment on attachment 285397 [details]
Patch

Clearing flags on attachment: 285397

Committed r204226: <http://trac.webkit.org/changeset/204226>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

This broke 7 API tests: https://build.webkit.org/builders/Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/7163/steps/run-api-tests/logs/stdio

Tests that failed:
  WKNavigation.WebContentProcessDidTerminate
  WebKit2.GetPIDAfterAbortedProcessLaunch
  WebKit2.LoadPageAfterCrash
  WebKit2.MouseMoveAfterCrash
  WebKit2.ReloadPageAfterCrash
  WebKit2.ResizeWindowAfterCrash
  WebKit2.RestoreShouldOpenExternalURLsPolicyAfterCrash

[Chris Dumez]

Reverted r204226 for reason:

Broke some API tests

Committed r204238: <http://trac.webkit.org/changeset/204238>

[Chris Dumez]

(In reply to comment #5)
> This broke 7 API tests:
> https://build.webkit.org/builders/
> Apple%20El%20Capitan%20Debug%20WK2%20%28Tests%29/builds/7163/steps/run-api-
> tests/logs/stdio
> 
> Tests that failed:
>   WKNavigation.WebContentProcessDidTerminate
>   WebKit2.GetPIDAfterAbortedProcessLaunch
>   WebKit2.LoadPageAfterCrash
>   WebKit2.MouseMoveAfterCrash
>   WebKit2.ReloadPageAfterCrash
>   WebKit2.ResizeWindowAfterCrash
>   WebKit2.RestoreShouldOpenExternalURLsPolicyAfterCrash

Thanks for letting me know. I rolled out for now until I get a chance to investigate.

[Chris Dumez]

Comment on attachment 285397 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285397&action=review

> Source/WebKit2/UIProcess/WebProcessProxy.cpp:807
> +    didClose(*connection());

Looks like the issue was that didClose() already calls shutDown(), and we are calling shutDown() again below. We would hit an assertion because shutDown() was called twice.

[Chris Dumez]

Created attachment 285529 [details]
Patch

[Chris Dumez]

Fixed the assertion hits in debug builds. It now passes all the tests in Debug as well.

[WebKit Commit Bot]

Comment on attachment 285529 [details]
Patch

Clearing flags on attachment: 285529

Committed r204243: <http://trac.webkit.org/changeset/204243>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

Reverted r204243 for reason:

Caused some layout test failures

Committed r204841: <http://trac.webkit.org/changeset/204841>