[webkit-reviews] review denied: [Bug 72227] [GTK] improve platformSetDefersLoading : [Attachment 115169] updated patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Dan Winship
<danw at gnome.org>'s request for review:
Bug 72227: [GTK] improve platformSetDefersLoading
https://bugs.webkit.org/show_bug.cgi?id=72227

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

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=115169&action=review


Looks good to me, except that I think you are leaking the asyncResult.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:750
> +    // We only need to take action here to UN-defer loading

Nit: Missing a period.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:759
>	   return;

Is it actually possible for m_cancellable to be non-null and d->m_soupRequest
to be null? If so it'd be good to leave a small comment here explaining when
and if not, it'd be better to ASSERT(d->m_soupRequest) instead.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:763
> +	   GAsyncResult* asyncResult = d->m_deferredResult.leakRef();

I think you actually need to call adoptGRef here because the naked assignment
adds another reference and the pointer leaks. Sorry for the confusion.