Bug 187008 – REGRESSION(r229722): WebKitLegacy clients can crash when loading alternate page

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brent Fulgham]

The new call to 'clearProvisionalLoadForPolicyCheck' added in r229722 broke loading behavior in WebKitLegacy.

1. We can now enter 'cancelPolicyCheckIfNeeded' without a Frame loader, in what appears to be a recursive call during the load cancellation (the 'm_waitingForContentPolicy' and 'm_waitingForNavigationPolicy' have already been nulled). It seems like we should return early here, or perhaps just move the RELEASE_ASSERT inside the case where we have an active policy check happening.

2. We also enter FrameLoader::checkContentPolicy without an active document loader. We should recognize this case and handle it, rather than trying to dereference a nullptr document loader.

[Brent Fulgham]

Created attachment 343516 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/41430690>

[Radar WebKit Bug Importer]

<rdar://problem/41430650>

[Chris Dumez]

Comment on attachment 343516 [details]
Patch

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

> Source/WebCore/loader/FrameLoader.cpp:363
>  void FrameLoader::checkContentPolicy(const ResourceResponse& response, ContentPolicyDecisionFunction&& function)

The crash traces attached to the radar do not seem to involve FrameLoader::checkContentPolicy(), could you clarify why this change is needed?

[Brent Fulgham]

Comment on attachment 343516 [details]
Patch

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

>> Source/WebCore/loader/FrameLoader.cpp:363
>>  void FrameLoader::checkContentPolicy(const ResourceResponse& response, ContentPolicyDecisionFunction&& function)
> 
> The crash traces attached to the radar do not seem to involve FrameLoader::checkContentPolicy(), could you clarify why this change is needed?

Yes, sorry -- this code is hit once you clear the RELEASE_ASSERT from DocumentLoader.cpp.

(Historical Note: I went and spoke with Chris in person about the issue before he completed the review).

[Brent Fulgham]

Committed r233176: <https://trac.webkit.org/changeset/233176>