Bug 130511 – Move all EFL typedefs into EflTypedefs.h.

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Hyowon Kim]

I think we don't have to spread EFL typedefs in every headers.
We will get a lot more work if EFL changes them, and it looks not good.
So we need to clean up and add a dedicated file to place all EFL typedefs.

[Hyowon Kim]

Created attachment 227533 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 227533 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=227533&action=review

LGTM, nice !

> Source/JavaScriptCore/ChangeLog:3
> +        Clean up EFL typedefs.

It looks this bug title isn't correct for this patch. Patch description looks more correct for title.

"Move all EFL typedefs into GTypedefs.h"

[Hyowon Kim]

Created attachment 227549 [details]
Patch

[Ryuan Choi]

Comment on attachment 227549 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=227549&action=review

> Source/WTF/wtf/gobject/GTypedefs.h:129
> +#if PLATFORM(EFL)
> +typedef unsigned char Eina_Bool;
> +typedef struct _Evas_Point Evas_Point;
> +typedef struct _Evas_GL Evas_GL;
> +typedef struct _Evas_GL_Context Evas_GL_Context;
> +typedef struct _Evas_GL_Surface Evas_GL_Surface;
> +typedef struct _Ecore_Evas Ecore_Evas;
> +typedef struct _Ecore_Fd_Handler Ecore_Fd_Handler;
> +typedef struct _Eina_List Eina_List;
> +typedef struct _Eina_Module Eina_Module;
> +typedef struct _Eina_Rectangle Eina_Rectangle;
> +#if USE(EO)
> +typedef struct _Eo_Opaque Evas;
> +typedef struct _Eo_Opaque Evas_Object;
> +typedef struct _Eo_Opaque Ecore_Timer;
> +#else
> +typedef struct _Evas Evas;
> +typedef struct _Evas_Object Evas_Object;
> +typedef struct _Ecore_Timer Ecore_Timer;
> +#endif
> +#endif
> +

This structure is not really related to GType and gobject.

Why don't you add new header such as EFLTypedefs.h in WTF/wtf/efl/ ?

[Gyuyoung Kim]

(In reply to comment #4)
> (From update of attachment 227549 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=227549&action=review
> 
> > Source/WTF/wtf/gobject/GTypedefs.h:129
> > +#if PLATFORM(EFL)
> > +typedef unsigned char Eina_Bool;
> > +typedef struct _Evas_Point Evas_Point;
> > +typedef struct _Evas_GL Evas_GL;
> > +typedef struct _Evas_GL_Context Evas_GL_Context;
> > +typedef struct _Evas_GL_Surface Evas_GL_Surface;
> > +typedef struct _Ecore_Evas Ecore_Evas;
> > +typedef struct _Ecore_Fd_Handler Ecore_Fd_Handler;
> > +typedef struct _Eina_List Eina_List;
> > +typedef struct _Eina_Module Eina_Module;
> > +typedef struct _Eina_Rectangle Eina_Rectangle;
> > +#if USE(EO)
> > +typedef struct _Eo_Opaque Evas;
> > +typedef struct _Eo_Opaque Evas_Object;
> > +typedef struct _Eo_Opaque Ecore_Timer;
> > +#else
> > +typedef struct _Evas Evas;
> > +typedef struct _Evas_Object Evas_Object;
> > +typedef struct _Ecore_Timer Ecore_Timer;
> > +#endif
> > +#endif
> > +
> 
> This structure is not really related to GType and gobject.
> 
> Why don't you add new header such as EFLTypedefs.h in WTF/wtf/efl/ ?

Ah, right. +1. How about ETypeDefs.h ?

[Ryuan Choi]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 227549 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=227549&action=review
> > 
> > > Source/WTF/wtf/gobject/GTypedefs.h:129
> > > +#if PLATFORM(EFL)
> > > +typedef unsigned char Eina_Bool;
> > > +typedef struct _Evas_Point Evas_Point;
> > > +typedef struct _Evas_GL Evas_GL;
> > > +typedef struct _Evas_GL_Context Evas_GL_Context;
> > > +typedef struct _Evas_GL_Surface Evas_GL_Surface;
> > > +typedef struct _Ecore_Evas Ecore_Evas;
> > > +typedef struct _Ecore_Fd_Handler Ecore_Fd_Handler;
> > > +typedef struct _Eina_List Eina_List;
> > > +typedef struct _Eina_Module Eina_Module;
> > > +typedef struct _Eina_Rectangle Eina_Rectangle;
> > > +#if USE(EO)
> > > +typedef struct _Eo_Opaque Evas;
> > > +typedef struct _Eo_Opaque Evas_Object;
> > > +typedef struct _Eo_Opaque Ecore_Timer;
> > > +#else
> > > +typedef struct _Evas Evas;
> > > +typedef struct _Evas_Object Evas_Object;
> > > +typedef struct _Ecore_Timer Ecore_Timer;
> > > +#endif
> > > +#endif
> > > +
> > 
> > This structure is not really related to GType and gobject.
> > 
> > Why don't you add new header such as EFLTypedefs.h in WTF/wtf/efl/ ?
> 
> Ah, right. +1. How about ETypeDefs.h ?

GType is common word in GObject area (http://www.gtk.org/api/2.6/gobject/gobject-Type-Information.html).
So, EType looks quite ambiguous to me and I didn't hear it before.

How about EFLdefs.h ?

[Hyowon Kim]

Created attachment 227598 [details]
Patch

[Hyowon Kim]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (From update of attachment 227549 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=227549&action=review
> > > 
> > > > Source/WTF/wtf/gobject/GTypedefs.h:129
> > > 
> > > This structure is not really related to GType and gobject.
> > > 
> > > Why don't you add new header such as EFLTypedefs.h in WTF/wtf/efl/ ?
> > 
> > Ah, right. +1. How about ETypeDefs.h ?
> 
> GType is common word in GObject area (http://www.gtk.org/api/2.6/gobject/gobject-Type-Information.html).
> So, EType looks quite ambiguous to me and I didn't hear it before.
> 
> How about EFLdefs.h ?

I think adding EFL typedefs into GTypedefs.h is not bad, since Cairo, Clutter and GTK are already using it.
But adding EflTypedefs.h is not a problem, so I've uploaded the new patch.

[Gyuyoung Kim]

Comment on attachment 227598 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=227598&action=review

> Source/WTF/wtf/Platform.h:914
> +#if PLATFORM(EFL)

Please add an empty line above this line.

[Ryuan Choi]

Committed r166149: <http://trac.webkit.org/changeset/166149>

[Ryuan Choi]

Comment on attachment 227598 [details]
Patch

landed after applied