Bug 130511

Summary: Move all EFL typedefs into EflTypedefs.h.
Product: WebKit Reporter: Hyowon Kim <hw1008.kim>
Component: WebKit EFLAssignee: Hyowon Kim <hw1008.kim>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, bunhere, cdumez, cmarcelo, commit-queue, gyuyoung.kim, gyuyoung.kim, lucas.de.marchi, ryuan.choi, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Hyowon Kim 2014-03-20 08:12:00 PDT
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.
Comment 1 Hyowon Kim 2014-03-21 21:32:39 PDT
Created attachment 227533 [details]
Patch
Comment 2 Gyuyoung Kim 2014-03-21 21:43:17 PDT
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"
Comment 3 Hyowon Kim 2014-03-22 06:37:42 PDT
Created attachment 227549 [details]
Patch
Comment 4 Ryuan Choi 2014-03-22 15:43:20 PDT
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/ ?
Comment 5 Gyuyoung Kim 2014-03-22 17:25:36 PDT
(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 ?
Comment 6 Ryuan Choi 2014-03-22 17:50:41 PDT
(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 ?
Comment 7 Hyowon Kim 2014-03-22 21:30:12 PDT
Created attachment 227598 [details]
Patch
Comment 8 Hyowon Kim 2014-03-22 21:45:54 PDT
(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.
Comment 9 Gyuyoung Kim 2014-03-23 21:29:50 PDT
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.
Comment 10 Ryuan Choi 2014-03-23 21:51:40 PDT
Committed r166149: <http://trac.webkit.org/changeset/166149>
Comment 11 Ryuan Choi 2014-03-23 21:52:05 PDT
Comment on attachment 227598 [details]
Patch

landed after applied