Bug 22250 - PLATFORM(CAIRO) should be defined by WIN_CAIRO define
Summary: PLATFORM(CAIRO) should be defined by WIN_CAIRO define
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Laszlo Gombos
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-13 19:38 PST by Eric Seidel (no email)
Modified: 2009-12-29 15:20 PST (History)
6 users (show)

See Also:


Attachments
proposed solution (1.14 KB, patch)
2009-11-28 19:14 PST, Laszlo Gombos
no flags Details | Formatted Diff | Diff
2nd try (1.19 KB, patch)
2009-12-01 18:14 PST, Laszlo Gombos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2008-11-13 19:38:28 PST
CAIRO build should be defined by a __BUILDING_WIN_CAIRO define

Right now to specify the CAIRO build we have a "blacklist" of what builds are not the CAIRO build.  This is bad, IMO.  We should instead of a whitelist of what builds *are* the cairo build. :)

Right now every time someone ports WebCore they need to turn off cairo in JavaScriptCore/wtf/Platform.h or WebCore/config.h before they can turn on their own graphics system.
Comment 1 Eric Seidel (no email) 2008-11-13 20:26:51 PST
The most recent example of this:
http://trac.webkit.org/changeset/38382
Comment 2 Laszlo Gombos 2009-11-28 19:14:39 PST
Created attachment 43988 [details]
proposed solution

Revision r45831 (http://trac.webkit.org/changeset/45831) - after this bug was filed - essentially introduced a solution. WIN_CAIRO is defined by the build system and can be used to detect the CAIRO build instead of the "blacklist" approach currently used in Platform.h.

http://trac.webkit.org/changeset/45831 also defined WTF_PLATFORM_CAIRO in all relevant config.h files (it is not needed in JavaScriptCore/config.h as JavaScriptCore is not relying on the WTF_PLATFORM_CAIRO). The solution is to simply remove this part from Platform.h.
Comment 3 Eric Seidel (no email) 2009-11-29 07:07:05 PST
I'm confused.  Does WIN_CAIRO build JSC exactly the same as WIN_OS? if so, we can kill this, yes.
Comment 4 Laszlo Gombos 2009-11-29 08:06:47 PST
(In reply to comment #3)
> I'm confused.  Does WIN_CAIRO build JSC exactly the same as WIN_OS? if so, we
> can kill this, yes.

Eric, it looks to me that that is the case. I searched for WIN_CAIRO in JavaScriptCore and the only hit was in Platform.h.
Comment 5 Brent Fulgham 2009-11-29 12:34:56 PST
(In reply to comment #4)
> (In reply to comment #3)
> > I'm confused.  Does WIN_CAIRO build JSC exactly the same as WIN_OS? if so, we
> > can kill this, yes.
> 
> Eric, it looks to me that that is the case. I searched for WIN_CAIRO in
> JavaScriptCore and the only hit was in Platform.h.

It is not exactly the same.  It is built using the Open Source "OpenCFLite" version of CoreFoundation.  This has slightly different headers included by default, so there were a handful of places where includes had to be modified.

Please confirm that the WinCairo port builds after making this change before r+ this change.
Comment 6 Laszlo Gombos 2009-11-29 13:50:56 PST
Brent, Thanks for looking into this. I did _not_ had a chance to confirm that the WinCairo port builds after making with the patch. I was hoping that maybe you or someone that you can recommend can help us with that. I had no intention to commit the patch before the build gets tested.

Also, just to clarify my previous - somewhat vague - statement; under the JavaScriptCore directory there is no code behind any sort of CAIRO flag (WIN_CAIRO, PLATFORM(CAIRO), etc). 

Since WebCore and DumpRenderTree already properly sets PLATFORM(CAIRO) for WinCairo port in config.h - in theory - it should be safe to remove PLATFORM(CAIRO) from under JavaScriptCore/wtf/Platform.h.
Comment 7 Brent Fulgham 2009-11-30 17:10:32 PST
I just applied this change, and am building the WinCairo version.  I'll report back shortly!
Comment 8 Laszlo Gombos 2009-12-01 06:10:03 PST
Brent, any update on this ? Thanks !
Comment 9 Brent Fulgham 2009-12-01 09:05:03 PST
Build worked just fine.  Please go ahead and commit.
Comment 10 Laszlo Gombos 2009-12-01 10:47:41 PST
Comment on attachment 43988 [details]
proposed solution

Thanks Brent for helping out testing this !
Comment 11 WebKit Commit Bot 2009-12-01 11:01:17 PST
Comment on attachment 43988 [details]
proposed solution

Clearing flags on attachment: 43988

Committed r51551: <http://trac.webkit.org/changeset/51551>
Comment 12 WebKit Commit Bot 2009-12-01 11:01:22 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Gustavo Noronha (kov) 2009-12-01 13:17:33 PST
I reverted this commit because it broke GTK+ and nobody was around to fix it timely.
Comment 14 Laszlo Gombos 2009-12-01 18:14:23 PST
Created attachment 44118 [details]
2nd try

GTK port is also using CAIRO not just the WinCairo port. Sorry for the build break.
Comment 15 WebKit Review Bot 2009-12-01 18:18:18 PST
style-queue ran check-webkit-style on attachment 44118 [details] without any errors.
Comment 16 Maciej Stachowiak 2009-12-28 17:36:44 PST
Comment on attachment 44118 [details]
2nd try

r=me
Comment 17 Eric Seidel (no email) 2009-12-28 22:42:33 PST
Attachment 44118 [details] was posted by a committer and has review+, assigning to Laszlo Gombos for commit.
Comment 18 WebKit Commit Bot 2009-12-29 13:16:29 PST
Comment on attachment 44118 [details]
2nd try

Rejecting patch 44118 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueueSVN/LayoutTests
Testing 11854 test cases.
svg/W3C-SVG-1.1/animate-elem-30-t.svg -> crashed

Exiting early after 1 failures. 9849 tests run.
476.79s total testing time

9848 test cases (99%) succeeded
1 test case (<1%) crashed
7 test cases (<1%) had stderr output

Full output: http://webkit-commit-queue.appspot.com/results/150846
Comment 19 Eric Seidel (no email) 2009-12-29 13:27:36 PST
Comment on attachment 44118 [details]
2nd try

This failure was caused by bug 30098.  The bots were about to fail, they were just behind.  Re-queueing.
Comment 20 WebKit Commit Bot 2009-12-29 15:19:57 PST
Comment on attachment 44118 [details]
2nd try

Clearing flags on attachment: 44118

Committed r52637: <http://trac.webkit.org/changeset/52637>
Comment 21 WebKit Commit Bot 2009-12-29 15:20:07 PST
All reviewed patches have been landed.  Closing bug.