Bug 10604 – Make regression testing work with Qt (Adding KIO support)

• Component: Platform
• Status: RESOLVED FIXED

[Nikolas Zimmermann]

Now that KDE4's KIO is somewhat usable, we can finally kill the
libcurl usage. It made for regression testing very slow, using KIO
it works much faster.

HTTP POST/GET is supported, as well as working JS XMLHTTPRequest.
(assembleResponseHeaders() etc. is implemented).

I'll attach a few patches (with comments) soon.

[Nikolas Zimmermann]

Created attachment 10270 [details]
Offer QString -> DeprecatedString conversion

Needed for the KIO patch.
(QString 'response' -> DeprecatedString 'responseHeaders' conversion)

[Nikolas Zimmermann]

Created attachment 10271 [details]
Replace libcurl usage by KIO

Removes the ResourceLoaderCurl.cpp, adds ResourceLoaderQt.cpp.
Rewrote ResourceLoaderManager from scratch for our needs :-)

[Nikolas Zimmermann]

Created attachment 10272 [details]
run-webkit-tests modifications for Qt

Faster polling in DumpRenderTree, "-qt" prefixes for run-webkit-tests.
Removing one nonImplemented() usage in RenderThemeQt.

[Nikolas Zimmermann]

Created attachment 10273 [details]
Fix crash hit in css2.1 - in RenderMenuList

Don't crash in RenderMenuList, if the RenderTheme
doesn't implement createPopupMenu. Like Qt :-)

[Alexey Proskuryakov]

Comment on attachment 10270 [details]
Offer QString -> DeprecatedString conversion

According to Qt docs, utf8Data.data() will not be 0 for null arrays, so this does not convert null strings properly. Yes, this doesn't really matter in WebCore/platform, but I think we should have this basic conversion right.

[Alexey Proskuryakov]

Comment on attachment 10272 [details]
run-webkit-tests modifications for Qt

r=me

[Adam Roben (:aroben)]

Comment on attachment 10273 [details]
Fix crash hit in css2.1 - in RenderMenuList

Why not just abort early if createPopupMenu fails?

[Nikolas Zimmermann]

Created attachment 10275 [details]
Updated: Offer QString -> DeprecatedString conversion

Incorporated ap's comment.

[Alexey Proskuryakov]

Comment on attachment 10275 [details]
Updated: Offer QString -> DeprecatedString conversion

r=me

[Nikolas Zimmermann]

Created attachment 10280 [details]
Updated: Fix crash hit in css2.1 - in RenderMenuList

As discussed with Alexey: Provide stub implementation of RenderPopupMenuQt.

[Alexey Proskuryakov]

Comment on attachment 10272 [details]
run-webkit-tests modifications for Qt

Committed revision 16077, clearing review flag.

[Alexey Proskuryakov]

Comment on attachment 10275 [details]
Updated: Offer QString -> DeprecatedString conversion

Committed revision 16078, clearing review flag.

[Alexey Proskuryakov]

Comment on attachment 10280 [details]
Updated: Fix crash hit in css2.1 - in RenderMenuList

r=me

[Alexey Proskuryakov]

Comment on attachment 10280 [details]
Updated: Fix crash hit in css2.1 - in RenderMenuList

Committed revision 16079, clearing review flag.

[Eric Seidel (no email)]

Comment on attachment 10271 [details]
Replace libcurl usage by KIO

style: (no {} needed)
+    if (job->method() == "POST") {
+        // Will take care of informing our client...
+        // This must be called before receivedAllData(),
+        // otherwhise assembleResponseHeaders() is called too early...
+        job->receivedResponse(kioJob->queryMetaData("HTTP-Headers"));
+    }

The changes in general look sane.  They won't harm any other port.  However I have neither the Qt expertise nor the Loader architecture expertise to really grill you with this review.  If you're comfortable with a "yeah this looks sane" review, then r=me.  Otherwise you'll need to corner maciej when he's around and ask him about loader architectures.  I doubt anyone on the team has much in the way of Qt expertise.

[Nikolas Zimmermann]

(In reply to comment #15)
> The changes in general look sane.  They won't harm any other port.  However I
> have neither the Qt expertise nor the Loader architecture expertise to really
> grill you with this review.  If you're comfortable with a "yeah this looks
> sane" review, then r=me.  Otherwise you'll need to corner maciej when he's
> around and ask him about loader architectures.  I doubt anyone on the team has
> much in the way of Qt expertise.
> 

Sounds fair enough, when I implement the bridge-like architecture
(the next days :-) I'll definately ask Maciej for the begin() stuff
- as discussed on IRC. Thanks for the review anyway!

[Eric Seidel (no email)]

looks to be landed already.