Bug 11405

Summary: Linux/Gdk build fixes
Product: WebKit Reporter: Krzysztof Kowalczyk <kkowalczyk>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: mrowe
Priority: P2    
Version: 420+   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Fix Linux/Gdk build.
none
Additional fixes
info: review-
linux\gdk build fixes
none
linux\gdk build fixes
mjs: review-
linux\gdk build fixes
none
linux\gdk build fixes mjs: review+

Description Krzysztof Kowalczyk 2006-10-24 12:38:38 PDT
Linux/Gdk build fixes.
Comment 1 Krzysztof Kowalczyk 2006-10-24 12:40:47 PDT
Created attachment 11193 [details]
Fix Linux/Gdk build.

Fix Linux/Gdk build by fixing up the code to account for recent changes.
Comment 2 Niels Leenheer (HTML5test) 2006-10-27 02:26:26 PDT
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.
Comment 3 Niels Leenheer (HTML5test) 2006-10-27 02:58:12 PDT
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.
Comment 4 Krzysztof Kowalczyk 2006-10-31 00:08:45 PST
Created attachment 11293 [details]
linux\gdk build fixes

Updated linux\gdk fixes that work against r17472 (and hopefully later).
Comment 5 Krzysztof Kowalczyk 2006-10-31 22:39:57 PST
Created attachment 11320 [details]
linux\gdk build fixes

updated linux\gdk build fixes, work against r17513 (and hopefully later)
Comment 6 Maciej Stachowiak 2006-11-01 19:13:01 PST
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.
Comment 7 Krzysztof Kowalczyk 2006-11-04 14:22:25 PST
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.
Comment 8 Krzysztof Kowalczyk 2006-11-05 18:14:33 PST
Created attachment 11389 [details]
linux\gdk build fixes

Updated linux\gdk fixes to r17606
Comment 9 Maciej Stachowiak 2006-11-05 19:30:55 PST
+            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.
Comment 10 Maciej Stachowiak 2006-11-05 19:31:42 PST
Comment on attachment 11389 [details]
linux\gdk build fixes

r=me
Comment 11 Mark Rowe (bdash) 2006-11-06 17:17:46 PST
Landed in r17636.  Krzysztof, can you please consider fixing the two issues Maciej pointed out in a future patch?