Bug 22250 – PLATFORM(CAIRO) should be defined by WIN_CAIRO define

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

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

The most recent example of this:
http://trac.webkit.org/changeset/38382

[Laszlo Gombos]

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.

[Eric Seidel (no email)]

I'm confused.  Does WIN_CAIRO build JSC exactly the same as WIN_OS? if so, we can kill this, yes.

[Laszlo Gombos]

(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.

[Brent Fulgham]

(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.

[Laszlo Gombos]

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.

[Brent Fulgham]

I just applied this change, and am building the WinCairo version.  I'll report back shortly!

[Laszlo Gombos]

Brent, any update on this ? Thanks !

[Brent Fulgham]

Build worked just fine.  Please go ahead and commit.

[Laszlo Gombos]

Comment on attachment 43988 [details]
proposed solution

Thanks Brent for helping out testing this !

[WebKit Commit Bot]

Comment on attachment 43988 [details]
proposed solution

Clearing flags on attachment: 43988

Committed r51551: <http://trac.webkit.org/changeset/51551>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Gustavo Noronha (kov)]

I reverted this commit because it broke GTK+ and nobody was around to fix it timely.

[Laszlo Gombos]

Created attachment 44118 [details]
2nd try

GTK port is also using CAIRO not just the WinCairo port. Sorry for the build break.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44118 [details] without any errors.

[Maciej Stachowiak]

Comment on attachment 44118 [details]
2nd try

r=me

[Eric Seidel (no email)]

Attachment 44118 [details] was posted by a committer and has review+, assigning to Laszlo Gombos for commit.

[WebKit Commit Bot]

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

[Eric Seidel (no email)]

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.

[WebKit Commit Bot]

Comment on attachment 44118 [details]
2nd try

Clearing flags on attachment: 44118

Committed r52637: <http://trac.webkit.org/changeset/52637>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.