WebKit/EFL have many APIs, but some of them looks not useless. First, functions including Ewk_View_Private_Data should be private or change it to Evas_Object. ewk_view_repaints_get(Ewk_View_Private_Data*, ...) ewk_view_scroll_requests_get(Ewk_View_Private_Data*, ...) ewk_view_repaint_add(Ewk_View_Private_Data*, ...) ewk_view_layout_if_needed_recursive(Ewk_View_Private_Data*, ...) ewk_view_scrolls_process(Ewk_View_Private_Data*, ...) I checked old eve(I'll check latest in my home) and our browser, but those are only used internally. Does it required? My first suggestion is moving them to ewk_private.h If not, how do you think about change it to Evas_Object?
Good Idea, As ewk_private has some WebCore dependency, I prefer creation of ewk_view_private.h. I don't want both two (single and tiled view) to be tightly coupled with WebCore.
(In reply to comment #0) > WebKit/EFL have many APIs, but some of them looks not useless. > > First, functions including Ewk_View_Private_Data should be private or change it to Evas_Object. > > ewk_view_repaints_get(Ewk_View_Private_Data*, ...) > ewk_view_scroll_requests_get(Ewk_View_Private_Data*, ...) > ewk_view_repaint_add(Ewk_View_Private_Data*, ...) > ewk_view_layout_if_needed_recursive(Ewk_View_Private_Data*, ...) > ewk_view_scrolls_process(Ewk_View_Private_Data*, ...) > > I checked old eve(I'll check latest in my home) and our browser, but those are only used internally. > Does it required? It's needed if user wants to subclass the backing store. It's explained in comments above their declarations in Source/WebKit/efl/ewk/ewk_view.h So, there's not much we can do here.
(In reply to comment #1) > Good Idea, > > As ewk_private has some WebCore dependency, I prefer creation of ewk_view_private.h. No, every private functions goes to ewk_private.h, but as explained above we can not move these functions. > I don't want both two (single and tiled view) to be tightly coupled with WebCore. Changing the header doesn't change anything about this.
(In reply to comment #0) > WebKit/EFL have many APIs, but some of them looks not useless. > > First, functions including Ewk_View_Private_Data should be private or change it to Evas_Object. > > ewk_view_repaints_get(Ewk_View_Private_Data*, ...) > ewk_view_scroll_requests_get(Ewk_View_Private_Data*, ...) > ewk_view_repaint_add(Ewk_View_Private_Data*, ...) > ewk_view_layout_if_needed_recursive(Ewk_View_Private_Data*, ...) > ewk_view_scrolls_process(Ewk_View_Private_Data*, ...) > > I checked old eve(I'll check latest in my home) and our browser, but those are only used internally. > Does it required? > > My first suggestion is moving them to ewk_private.h > If not, how do you think about change it to Evas_Object? Thinking again... I think not all of them are used by a subclass implementation and the others we could indeed change to use Evas_Object as argument.
>> Good Idea, >> >> As ewk_private has some WebCore dependency, I prefer creation of >> ewk_view_private.h. > No, every private functions goes to ewk_private.h, but as explained above we can > not move these functions. >> I don't want both two (single and tiled view) to be tightly coupled with WebCore. > Changing the header doesn't change anything about this. Hi, Lucas. Long time no see. If ewk_view_tiled and ewk_view_single would include ewk_private.h, It would include unnecessary header file, I don't like it.
(In reply to comment #0) > WebKit/EFL have many APIs, but some of them looks not useless. > > First, functions including Ewk_View_Private_Data should be private or change it to Evas_Object. > > ewk_view_repaints_get(Ewk_View_Private_Data*, ...) > ewk_view_scroll_requests_get(Ewk_View_Private_Data*, ...) > ewk_view_repaint_add(Ewk_View_Private_Data*, ...) > ewk_view_layout_if_needed_recursive(Ewk_View_Private_Data*, ...) > ewk_view_scrolls_process(Ewk_View_Private_Data*, ...) > > I checked old eve(I'll check latest in my home) and our browser, but those are only used internally. > Does it required? > > My first suggestion is moving them to ewk_private.h I talked privately to Antognolli about this. We agreed that all these functions can be moved to ewk_private.h > If not, how do you think about change it to Evas_Object? If at any time they are needed outside of WebKit, we can move them back, but using the Evas_Object.
Created attachment 97398 [details] Patch
May be, ewk_view_single.c and ewk_view_tiled.c would need ewk_private.h.
Comment on attachment 97398 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97398&action=review > Source/WebKit/efl/ChangeLog:5 > + [EFL] Functions including Ewk_View_Private_Data should not an API. Functions needing Ewk_View_Private_Data as parameter are not part of public API > Source/WebKit/efl/ChangeLog:8 > + Move functions which use Ewk_View_Private and may not be required by applcations Ewk_View_Private_Data, applications > Source/WebKit/efl/ewk/ewk_private.h:130 > #if ENABLE(CONTEXT_MENUS) > - > Ewk_Context_Menu *ewk_context_menu_new(Evas_Object *view, WebCore::ContextMenuController *controller); > Eina_Bool ewk_context_menu_free(Ewk_Context_Menu *o); > void ewk_context_menu_item_append(Ewk_Context_Menu *o, WebCore::ContextMenuItem& core); > Ewk_Context_Menu *ewk_context_menu_custom_get(Ewk_Context_Menu *o); > void ewk_context_menu_show(Ewk_Context_Menu *o); > - > #endif It's not related to the patch, but it's ok to go in.
(In reply to comment #8) > May be, ewk_view_single.c and ewk_view_tiled.c would need ewk_private.h. I tested when I upload and it's fine for me. And although those files required ewk_private.h, those can't include ewk_private.h because ewk_private.h include c++ matter. So, we need to find proper way if it required. I think that we can make other bugs to control it when it required. BTW, Can I know why those files use *.c ? If we don't have any reasons, it looks be better to change *.cpp. it's just my two cents. (In reply to comment #9) > (From update of attachment 97398 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=97398&action=review > > > Source/WebKit/efl/ChangeLog:5 > > + [EFL] Functions including Ewk_View_Private_Data should not an API. > > Functions needing Ewk_View_Private_Data as parameter are not part of public API > Thanks, english is more difficult than code fore me. :) I'll update like you mentioned.
In fact, there are three more apis which are using private data as parameter of theirs. EAPI Ewk_View_Paint_Context *ewk_view_paint_context_new(Ewk_View_Private_Data *priv, cairo_t *cr); EAPI Eina_Bool ewk_view_paint(Ewk_View_Private_Data *priv, cairo_t *cr, const Eina_Rectangle *area); EAPI Eina_Bool ewk_view_paint_contents(Ewk_View_Private_Data *priv, cairo_t *cr, const Eina_Rectangle *area); I hope that you can consider them too.
Created attachment 97538 [details] Patch
(In reply to comment #12) > Created an attachment (id=97538) [details] > Patch I updated like lucas mentioned. (In reply to comment #11) > In fact, there are three more apis which are using private data as parameter of theirs. > > EAPI Ewk_View_Paint_Context *ewk_view_paint_context_new(Ewk_View_Private_Data *priv, cairo_t *cr); > EAPI Eina_Bool ewk_view_paint(Ewk_View_Private_Data *priv, cairo_t *cr, const Eina_Rectangle *area); > EAPI Eina_Bool ewk_view_paint_contents(Ewk_View_Private_Data *priv, cairo_t *cr, const Eina_Rectangle *area); > > I hope that you can consider them too. Right, I want refactoring them also. But now they can't go into ewk_private.h and I need to check whether it should be moved or not. I'll investigate them.
Could you review this If you have time?
Comment on attachment 97538 [details] Patch Clearing flags on attachment: 97538 Committed r89776: <http://trac.webkit.org/changeset/89776>
All reviewed patches have been landed. Closing bug.