REGRESSION (r179958): Crash in WebCore::DocumentLoader::detachFromFrame when -[id<WebPolicyDelegate> decidePolicyForMIMEType:request:frame:decisionListener:] fails to call -[id<WebPolicyDecisionListener> download|ignore|use]

• Component: Page Loading
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

If an app overrides -decidePolicyForMIMEType:request:frame:decisionListener: in their id<WebPolicyDelegate> object and doesn't call one of three methods on id<WebPolicyDecisionListener> (-download, -ignore or -use), then the app will crash due to a RELEASE_ASSERT() added in r179958.

<http://trac.webkit.org/r179958>

This was not the intention of these RELEASE_ASSERT() changes (they were to catch the cause of a different crash which we think we found the cause of), so we can change these back to Debug-only ASSERT() statements.

Note that the app SHOULD be calling -download, -ignore or -use inside their -[id<WebPolicyDelegate> -decidePolicyForMIMEType:request:frame:decisionListener:] method, but it shouldn't cause a crash, and there's no easy way to fix this without further risk of compatibility issues (due to additional callbacks being made).

[David Kilzer (:ddkilzer)]

<rdar://problem/20905401>

[David Kilzer (:ddkilzer)]

Created attachment 253076 [details]
Patch v1

[WebKit Commit Bot]

Comment on attachment 253076 [details]
Patch v1

Clearing flags on attachment: 253076

Committed r184323: <http://trac.webkit.org/changeset/184323>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

Thanks for taking care of this!

[Brady Eidson]

(In reply to comment #0)
> Note that the app SHOULD be calling -download, -ignore or -use inside their
> -[id<WebPolicyDelegate>
> -decidePolicyForMIMEType:request:frame:decisionListener:] method, but it
> shouldn't cause a crash, and there's no easy way to fix this without further
> risk of compatibility issues (due to additional callbacks being made).

This is not actually true - It's perfectly legal for an app to hang on to the decision listener and call one of the three methods later, asynchronously.

That's why it's set up as a listener object instead of requiring a synchronous return value.

[Brady Eidson]

Yah, these asserts are simply bogus in general.

If the client hangs on to the policy listener object for "awhile", say maybe to ask the user what they want to do, and the document is torn down in the meantime... DocumentLoader should handle that gracefully, release build or not.