[Gtk] ASSERT(d->m_response.isNull()) in contentSniffedCallback

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Sergio Villar Senin]

We are easily reaching that assertion since 72695. The problem is located in soup-request-http.c. We're emitting the "content-sniffed" callback without even considering that the sniffing was not enabled for each particular message (see startHttp in ResourceHandleSoup.cpp)

[Sergio Villar Senin]

Created attachment 74881 [details]
Fix for the bug

This should be "the right thing". We should not emit the content-sniffed callback if it was disabled for that particular SoupMessage. The problem is that soup_message_disables_feature is not in the public API of SoupMessage.

My proposal would be to just apply the changes in ResourceHandleSoup.cpp (i.e. do not connect to the "content-sniffed" signal) because:
   * it's correct anyway
   * it'll fix the bug
   * the invalid signal emission is located in the soup imported code, and thus, when moved there the soup_message_disables_feature could be easily added then (we can add that code as a comment)

[Sergio Villar Senin]

Adding Mr. Lopez (a.k.a SoupCache bug hunter)

[Xan Lopez]

Comment on attachment 74881 [details]
Fix for the bug

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

> WebCore/platform/network/soup/ResourceHandleSoup.cpp:591
>  

As discussed yesterday in person this is logically wrong, should go in an else block. Other than that the patch makes sense to me (let's get it in ASAP, it ASSERTs really frequently).

[Sergio Villar Senin]

Created attachment 74934 [details]
Fix for the bug

Final version of the patch:
   * connect to content-sniffed signal if handle->shouldContentSniff()
   * fixed an potential early finalization of an object
   * added comments with the "proper" fix inside the cache code to be migrated to libsoup

[Xan Lopez]

Comment on attachment 74934 [details]
Fix for the bug

r=me

[WebKit Commit Bot]

Comment on attachment 74934 [details]
Fix for the bug

Clearing flags on attachment: 74934

Committed r72762: <http://trac.webkit.org/changeset/72762>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.