Bug 70990 - [EFL] Fix building with Glib support disabled
Summary: [EFL] Fix building with Glib support disabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-26 21:07 PDT by Ivan Briano
Modified: 2011-12-23 02:06 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.22 KB, patch)
2011-10-26 21:12 PDT, Ivan Briano
no flags Details | Formatted Diff | Diff
Patch (1.32 KB, patch)
2011-10-27 08:10 PDT, Ivan Briano
no flags Details | Formatted Diff | Diff
Patch (1.20 KB, patch)
2011-12-22 11:05 PST, Ivan Briano
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Briano 2011-10-26 21:07:45 PDT
[EFL] Fix building with Glib support disabled
Comment 1 Ivan Briano 2011-10-26 21:12:02 PDT
Created attachment 112637 [details]
Patch
Comment 2 WebKit Review Bot 2011-10-26 21:14:35 PDT
Attachment 112637 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/cairo/WidgetBackingStore.h:28:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Ivan Briano 2011-10-27 08:10:33 PDT
Created attachment 112684 [details]
Patch
Comment 4 Ivan Briano 2011-10-27 08:12:16 PDT
Comment on attachment 112684 [details]
Patch

I'm not sure this is the right way to fix this, but since platforms others than Gtk may want to use cairo, and the usage of cairo_surface_t is not under conditionals, it seems coherent to just have the right header included as well.
Comment 5 Martin Robinson 2011-10-27 08:39:01 PDT
Comment on attachment 112684 [details]
Patch

Why not add the typedefs here or create a similar typedefs file for yourself. We worked pretty hard to keep system headers out of WebCore/JSC headers.
Comment 6 Raphael Kubo da Costa (:rakuco) 2011-10-27 08:50:26 PDT
(In reply to comment #5)
> (From update of attachment 112684 [details])
> Why not add the typedefs here or create a similar typedefs file for yourself. 

Just adding a typedef there should be fine IMHO.

> We worked pretty hard to keep system headers out of WebCore/JSC headers.

Hmm. Does it mean the same should be done to other headers (platform/graphics/CairoUtilities.h), for example?
Comment 7 Martin Robinson 2011-10-27 08:58:23 PDT
(In reply to comment #6)

> Hmm. Does it mean the same should be done to other headers (platform/graphics/CairoUtilities.h), for example?

Yes, if possible and within reason. :)
Comment 8 Eric Seidel (no email) 2011-12-19 11:49:06 PST
Comment on attachment 112684 [details]
Patch

Do we forward-declare these instead of including cairo.h?
Comment 9 Ivan Briano 2011-12-19 11:58:30 PST
Oh my, I had completely forgotten about this.
I'll make a new patch this week.
Comment 10 Ivan Briano 2011-12-22 11:05:45 PST
Created attachment 120344 [details]
Patch

New patch. Leaving the declaration unconditional to match its usage below.
Comment 11 Martin Robinson 2011-12-22 12:36:44 PST
Comment on attachment 120344 [details]
Patch

I would have preferred this go within the PLATFORM(EFL) block, but this is fine.
Comment 12 Ivan Briano 2011-12-22 12:38:28 PST
Even when the type is used without any #ifdef later? I will keep that in mind if so.
Thanks.
Comment 13 WebKit Review Bot 2011-12-23 02:06:11 PST
Comment on attachment 120344 [details]
Patch

Clearing flags on attachment: 120344

Committed r103614: <http://trac.webkit.org/changeset/103614>
Comment 14 WebKit Review Bot 2011-12-23 02:06:15 PST
All reviewed patches have been landed.  Closing bug.