[webkit-reviews] review granted: [Bug 24263] [GTK] ref ResourceHandle until we are finished with it : [Attachment 28122] refsouphandlev2.patch

[bugzilla-daemon at webkit.org]

Holger Freyther <zecke at selfish.org> has granted Xan Lopez
<xan.lopez at gmail.com>'s request for review:
Bug 24263: [GTK] ref ResourceHandle until we are finished with it
https://bugs.webkit.org/show_bug.cgi?id=24263

Attachment 28122: refsouphandlev2.patch
https://bugs.webkit.org/attachment.cgi?id=28122&action=review

------- Additional Comments from Holger Freyther <zecke at selfish.org>
r=me with two strings attached:

  - If possible I would like to see the adopRef in the finishedCallback (after
this patch).
  - we need to find out if finishedCallback will always be called or if we just
created a memory leak. the consensus on #webkit-gtk was that it is better to
leak than to crash (for this release). We need to clean this up after the
release...


> @@ -262,30 +262,32 @@ static void finishedCallback(SoupSession *session,
SoupMessage* msg, gpointer da
>  
>      ResourceHandleClient* client = handle->client();
>      if (!client)
> -	   return;
> +	   goto exit;

this goto might be avoided with a RefPtr::adoptRef early on. I'm not quite
sure, it would be nice if you could investiage that.


>  
>      d->m_msg = static_cast<SoupMessage*>(g_object_ref(msg));
> +    ref(); // balanced by a deref() in finishedCallback, which should always
run

maybe you could move the comment above the line?
>      soup_session_queue_message(session, d->m_msg, finishedCallback, ma