Bug 103347 - [EFL][WK2] Don't use the C API internally in ewk_color_picker
Summary: [EFL][WK2] Don't use the C API internally in ewk_color_picker
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jinwoo Song
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-26 19:36 PST by Jinwoo Song
Modified: 2013-01-23 16:16 PST (History)
8 users (show)

See Also:


Attachments
Patch (17.49 KB, patch)
2012-11-26 19:40 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (17.13 KB, patch)
2012-12-05 17:12 PST, Jinwoo Song
benjamin: review-
gtk-ews: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jinwoo Song 2012-11-26 19:36:04 PST
Used the C++ classes directly in ewk_color_picker and also fixed the wrong boilerplate styles.
Comment 1 Jinwoo Song 2012-11-26 19:40:40 PST
Created attachment 176153 [details]
Patch
Comment 2 Jinwoo Song 2012-12-05 17:12:12 PST
Created attachment 177875 [details]
Patch
Comment 3 kov's GTK+ EWS bot 2012-12-05 17:48:45 PST
Comment on attachment 177875 [details]
Patch

Attachment 177875 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15158520
Comment 4 Benjamin Poulain 2013-01-11 23:12:15 PST
Comment on attachment 177875 [details]
Patch

(In reply to comment #0)
> Used the C++ classes directly in ewk_color_picker

Why? This will just make changes under the C API harder.

I am not completely opposed to the idea, but it should have a good reason.
Comment 5 Chris Dumez 2013-01-12 00:29:45 PST
(In reply to comment #4)
> (From update of attachment 177875 [details])
> (In reply to comment #0)
> > Used the C++ classes directly in ewk_color_picker
> 
> Why? This will just make changes under the C API harder.
> 
> I am not completely opposed to the idea, but it should have a good reason.

We prefer to use C++ as much as possible in EFL port's internal code as we find it more convenient to use than the WK C API. This also avoids extra calls to toAPI() / toImpl(). We realize there is less API stability guarantee by bypassing the C API but we also want to avoid having too many abstraction layers.

If I understood correctly the new WebKit2 development process, it appears it is up to us to keep our WK2 port code building now. If so, our approach does not really make changes under the C API harder anymore. It does mean more work for us though.

Most of our port code is using the C++ API already so this patch actually makes it more consistent. Deciding to use the C API internally would mean a lot of changes.
Comment 6 Benjamin Poulain 2013-01-12 00:48:15 PST
(In reply to comment #5)
> We prefer to use C++ as much as possible in EFL port's internal code as we find it more convenient to use than the WK C API. This also avoids extra calls to toAPI() / toImpl(). We realize there is less API stability guarantee by bypassing the C API but we also want to avoid having too many abstraction layers.
> 
> If I understood correctly the new WebKit2 development process, it appears it is up to us to keep our WK2 port code building now. If so, our approach does not really make changes under the C API harder anymore. It does mean more work for us though.
> 
> Most of our port code is using the C++ API already so this patch actually makes it more consistent. Deciding to use the C API internally would mean a lot of changes.

I understand that you don't have to proactively move everything from C++ to C.

The new process does not mean people stop caring about the other ports before doing changes. The problem is precisely that people keep punching holes through layers until we have something impossible to maintain.

I'll think a bit more about it.
Comment 7 Chris Dumez 2013-01-12 02:25:51 PST
(In reply to comment #6)
> (In reply to comment #5)
> > We prefer to use C++ as much as possible in EFL port's internal code as we find it more convenient to use than the WK C API. This also avoids extra calls to toAPI() / toImpl(). We realize there is less API stability guarantee by bypassing the C API but we also want to avoid having too many abstraction layers.
> > 
> > If I understood correctly the new WebKit2 development process, it appears it is up to us to keep our WK2 port code building now. If so, our approach does not really make changes under the C API harder anymore. It does mean more work for us though.
> > 
> > Most of our port code is using the C++ API already so this patch actually makes it more consistent. Deciding to use the C API internally would mean a lot of changes.
> 
> I understand that you don't have to proactively move everything from C++ to C.
> 
> The new process does not mean people stop caring about the other ports before doing changes. The problem is precisely that people keep punching holes through layers until we have something impossible to maintain.

This is good to hear. I guess I am still unclear about the new development process and how it will affect us in practice. I can understand the possible maintainability issues with our approach and this is definitely open for discussion.

> 
> I'll think a bit more about it.

Thanks. I appreciate you working with us on this.
Comment 8 Benjamin Poulain 2013-01-14 14:15:07 PST
So I have been thinking a bit more about this. :)
I think we should really focus at using the same abstractions for all ports as much as possible. For the time being it is the C API, and that is the part we can change in the future.

My rationale is that we want the most uniform platform possible to avoid giving work to web developers. Our layout tests are written to test WebKit2 up to the boundaries of the C API (ports can go to higher layers but that is up to them), so that is a great place to enforce conformance.

If one of the API causes a performance problem, that is gonna be a problem for every port and we must address it.


---
NB: but feel free to update those incorrect copyright headers. :)