[webkit-reviews] review denied: [Bug 76700] [Qt][WK2] Application URL schemes cause asserts when using debug. : [Attachment 123297] patch for review.

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Zeno Albisser
<zeno at webkit.org>'s request for review:
Bug 76700: [Qt][WK2] Application URL schemes cause asserts when using debug.
https://bugs.webkit.org/show_bug.cgi?id=76700

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

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=123297&action=review


Looks good in general, but I think a few fixes need to be applied to the refptr
handling. See also http://www.webkit.org/coding/RefPtr.html. Setters that take
ownership should take a PassRefPtr. Similarly for getters unless ownership is
transferred the return type should be a raw pointer.

> Source/WebKit2/UIProcess/API/qt/qquicknetworkrequest_p.h:38
> +    void
setNetworkRequestData(WTF::RefPtr<WebKit::QtRefCountedNetworkRequestData>
data);

Shouldn't this be a PassRefPtr?

> Source/WebKit2/UIProcess/API/qt/qquicknetworkrequest_p.h:39
> +    WTF::RefPtr<WebKit::QtRefCountedNetworkRequestData> networkRequestData()
const;

I believe this should return a raw pointer, as ownership is not transferred.