Bug 11405 – Linux/Gdk build fixes

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Krzysztof Kowalczyk]

Linux/Gdk build fixes.

[Krzysztof Kowalczyk]

Created attachment 11193 [details]
Fix Linux/Gdk build.

Fix Linux/Gdk build by fixing up the code to account for recent changes.

[Niels Leenheer (HTML5test)]

Created attachment 11235 [details]
Additional fixes

I've added a second patch to solve a number of additional build problem for Gdk/Linux. This patch does not obsolete the patch by Krzysztof Kowalczyk. Both patches should be applied.

[Niels Leenheer (HTML5test)]

Comment on attachment 11235 [details]
Additional fixes

Review request stopped because there are even more build fixes needed. Will create a new patch that obsoletes this one.

[Krzysztof Kowalczyk]

Created attachment 11293 [details]
linux\gdk build fixes

Updated linux\gdk fixes that work against r17472 (and hopefully later).

[Krzysztof Kowalczyk]

Created attachment 11320 [details]
linux\gdk build fixes

updated linux\gdk build fixes, work against r17513 (and hopefully later)

[Maciej Stachowiak]

Comment on attachment 11320 [details]
linux\gdk build fixes

Thanks for the patch! A few comments:

What warning do you get that resulted in these changes:

-            return NaN64AsBits;
+            return (uintptr_t)NaN64AsBits;

It would be nicer if NanAsBits was templatized on sizeof(uintptr_t) so that this cast wasn't necessary. It's kind of sloppy to rely on the dead code paths to be optimized out in the first place (admittedly a pre-existing flaw in the code).

There is a more complete AffineTransformCairo attached to <http://bugs.webkit.org/show_bug.cgi?id=11433>, I suggest starting with that. There's one bug in that version which I pointed out in review comments. (I think the right way to map a rect with a transform is to map each corner and find the bounding box). However, the other version doesn't have all the notImplemented() stuff this does, so you may want to start with that.

Otherwise, this looks great to me.

r- to consider the two points I raised. If you can't figure out how to do the template solution for JSImmediate.h that's ok, that can be done later, but I think it is worth taking the better AffineTransform.

[Krzysztof Kowalczyk]

Created attachment 11381 [details]
linux\gdk build fixes

Updated linux\gdk build fixes. Dropped JSImmediate.h and cairo fixes, since they were fixed in the meantime. Updated gdk-specific fixes to latest r17594.

[Krzysztof Kowalczyk]

Created attachment 11389 [details]
linux\gdk build fixes

Updated linux\gdk fixes to r17606

[Maciej Stachowiak]

+            d->m_client->receivedAllData(job, 0);

On other platforms we usually prefer to write this as client() instead of d->m_client and also to null-check the client.

+#if 0
+void FrameGdk::openURL(const KURL& url)
+{
+    ASSERT(m_client);
+    m_client->openURL(url);
+}
+#endif
+

We prefer not to have ifdef'd out code in the tree. Despite these minor quibbles, r=me so this patch doesn't keep getting outdated by further changes.

[Maciej Stachowiak]

Comment on attachment 11389 [details]
linux\gdk build fixes

r=me

[Mark Rowe (bdash)]

Landed in r17636.  Krzysztof, can you please consider fixing the two issues Maciej pointed out in a future patch?