[webkit-reviews] review granted: [Bug 77007] [Qt][WK2] Use QVariant for payload data in application URL schemes. : [Attachment 124091] patch for review. - updated as commented previously.

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Zeno Albisser
<zeno at webkit.org>'s request for review:
Bug 77007: [Qt][WK2] Use QVariant for payload data in application URL schemes.
https://bugs.webkit.org/show_bug.cgi?id=77007

Attachment 124091: patch for review. - updated as commented previously.
https://bugs.webkit.org/attachment.cgi?id=124091&action=review

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


r=me. One comment below and one more here, they can be ignored or addressed in
later changes ;)

One aspect I'm not too happy about is that the _uiprocess_ decides to store
input _strings_ as-is (utf-16) encoded and the _web process_ side - in the
QtNetworkReply constructor - makes the assumption that the encoding of the
provided data is utf-16 unless otherwise specified. I think it would be much
cleaner if this could all be handled on the ui process side without assumptions
across process boundaries.

That means that ::send() should look like this:

if (data.type() == QVariant::String) {
    contentType = "text/html; charset=utf-16";
    QString string = data.toString();
    memcpy(blah, string.constData(), ...);
} else if (data.canConvert<QByteArray>()) {
    // copy bytes
}

if (contentType.isEmpty()) {
   abort and print a warning or something
}

> Source/WebKit2/UIProcess/API/qt/qquicknetworkreply.cpp:91
> +	   QObject* schemeParent = parent()->parent();
> +	   if (schemeParent) {
> +	       QQuickWebViewExperimental* webViewExperimental =
qobject_cast<QQuickWebViewExperimental*>(schemeParent->parent());

parent()->parent()->parent() is quite a hack. I think it would be nicer if
QQuickNetworkReply kept a QWeakPointer to the QQuickWebView and created the
QQuickWebViewExperimental on the fly for sending.