Bug 182532 – REGRESSION(r227758): Webpage fails to load due to crash in com.apple.WebKit: WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse + 267

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Antti Koivisto]

boom

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
    frame #0: 0x000000047098cfe4 JavaScriptCore`::WTFCrash() at Assertions.cpp:272
    frame #1: 0x0000000103d7d381 WebKit`WTF::Function<void (WebCore::PolicyAction)>::operator(this=0x00007ffeec018890, in=Ignore)(WebCore::PolicyAction) const at Function.h:55
  * frame #2: 0x000000010450d2c4 WebKit`WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse(this=0x00007fd3cd427640, response=0x00000004767f7b18, request=0x00000004767f7a08, function=0x00007ffeec018890)>&&) at WebFrameLoaderClient.cpp:753
    frame #3: 0x00000004624dfe91 WebCore`WebCore::FrameLoader::checkContentPolicy(this=0x00007fd3cd41c240, response=0x00000004767f7b18, function=0x00007ffeec018890)>&&) at FrameLoader.cpp:360
    frame #4: 0x0000000462497ba0 WebCore`WebCore::DocumentLoader::responseReceived(this=0x00000004767f7400, response=0x0000000475774910) at DocumentLoader.cpp:786
    frame #5: 0x000000046249955e WebCore`WebCore::DocumentLoader::responseReceived(this=0x00000004767f7400, resource=0x0000000475774700, response=0x0000000475774910) at DocumentLoader.cpp:699
    frame #6: 0x00000004624995a4 WebCore`non-virtual thunk to WebCore::DocumentLoader::responseReceived(this=0x00000004767f7400, resource=0x0000000475774700, response=0x0000000475774910) at DocumentLoader.cpp:0
    frame #7: 0x00000004625b7751 WebCore`WebCore::CachedRawResource::responseReceived(this=0x0000000475774700, response=0x00007ffeec0193d8) at CachedRawResource.cpp:201
    frame #8: 0x0000000462554665 WebCore`WebCore::SubresourceLoader::didReceiveResponse(this=0x0000000475770800, response=0x00007ffeec0193d8) at SubresourceLoader.cpp:353
    frame #9: 0x0000000104960755 WebKit`WebKit::WebResourceLoader::didReceiveResponse(this=0x00000004757f20e0, response=0x00007ffeec0193d8, needsContinueDidReceiveResponseMessage=true) at WebResourceLoader.cpp:115

[Antti Koivisto]

<rdar://problem/36414017>

[Antti Koivisto]

Created attachment 333168 [details]
patch

[Antti Koivisto]

Created attachment 333169 [details]
patch

[Chris Dumez]

Comment on attachment 333169 [details]
patch

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

> Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:749
>      if (!coreFrame)

Couldn't we move those null checks *betore* setting up the policy listener on the WebFrame earlier? This way, we can keep using the function as is, and we do not unnecessarily set up a policy listener.

[Antti Koivisto]

> Couldn't we move those null checks *betore* setting up the policy listener
> on the WebFrame earlier? This way, we can keep using the function as is, and
> we do not unnecessarily set up a policy listener.

I assumed there was a reason why they are after. Specifically setUpPolicyListener may call arbitrary code (m_willSubmitFormCompletionHandlers in invalidatePolicyListener) and if those null coreFrame/provisionalDocumentLoader we would crash.

Alex needs to answer whether that was the reason the checks are after setUpPolicyListener.

[Antti Koivisto]

> I assumed there was a reason why they are after. Specifically
> setUpPolicyListener may call arbitrary code
> (m_willSubmitFormCompletionHandlers in invalidatePolicyListener) and if
> those null coreFrame/provisionalDocumentLoader we would crash.

Also it calls any existing m_policyFunction.

[Alexey Proskuryakov]

Comment on attachment 333169 [details]
patch

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

> Source/WebKit/ChangeLog:9
> +        No test case, don't know how to make one. The repro involves multipart HTTP streaming and details are hazy.

We have some tests for multipart, those are actually not hard to write. Is there some particular detail that makes this unreproducible?

[youenn fablet]

Created attachment 333341 [details]
Patch

[Chris Dumez]

Comment on attachment 333341 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 333341 [details]
Patch

Clearing flags on attachment: 333341

Committed r228257: <https://trac.webkit.org/changeset/228257>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.