Bug 93229

Summary: [EFL] EFL Webkit needs a class wrapping eina stringshare
Product: WebKit Reporter: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Component: WebKit EFLAssignee: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, gyuyoung.kim, hausmann, kenneth, lucas.de.marchi, rakuco, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 61838    
Attachments:
Description Flags
patch
none
patch v2
none
patch v3
none
patch v4
none
patch v5 none

Description Mikhail Pozdnyakov 2012-08-06 00:55:28 PDT
EFL WebKit is currently using eina_stringshare to manage strings in a shared pull, however its EFL C API turns to be not quite convenient comparing to a C++ wrapper class.
Comment 1 Mikhail Pozdnyakov 2012-08-07 02:01:12 PDT
Created attachment 156895 [details]
patch
Comment 2 Kenneth Rohde Christiansen 2012-08-07 02:50:27 PDT
Comment on attachment 156895 [details]
patch

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

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:32
> +    :m_string(eina_stringshare_ref(other.m_string))

space missing before m_

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:37
> +    :m_string(eina_stringshare_add(str))

same here

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:43
> +    if (m_string)

when will that happen? why not assert in the ctor instead?

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:34
> +class WK_EXPORT WKEinaSharedString {

maybe it should be made non inheritable

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:36
> +    WKEinaSharedString() : m_string(0) { }

ok I see now. It might still make sense to assert in the other ctors

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:38
> +    WKEinaSharedString(const char* str);

I think think it might make sense to make a WKEinaSharedString(WKStringRef) or so
Comment 3 Mikhail Pozdnyakov 2012-08-07 03:09:30 PDT
Comment on attachment 156895 [details]
patch

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

>> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:34
>> +class WK_EXPORT WKEinaSharedString {
> 
> maybe it should be made non inheritable

How to make it non inheritable? The only way that comes to my mind is to inherit it virtually from a friend class with private default constructor :)
Comment 4 Mikhail Pozdnyakov 2012-08-07 03:21:30 PDT
Created attachment 156903 [details]
patch v2

Kenneth, thanks for review!
Comment 5 Chris Dumez 2012-08-07 03:27:40 PDT
Comment on attachment 156903 [details]
patch v2

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

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:60
> +    ASSERT(str);

Eina stringshare accepts null
Comment 6 Chris Dumez 2012-08-07 03:29:08 PDT
It would be nice to use it in the same patch to see how convenient it is. Personally I'm not convinced yet.
Comment 7 Mikhail Pozdnyakov 2012-08-07 03:33:22 PDT
Comment on attachment 156903 [details]
patch v2

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

>> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:60
>> +    ASSERT(str);
> 
> Eina stringshare accepts null

right, but do we want "einaSharedString = 0;" ?
Comment 8 Kenneth Rohde Christiansen 2012-08-07 03:34:12 PDT
Comment on attachment 156903 [details]
patch v2

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

>> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:60
>> +    ASSERT(str);
> 
> Eina stringshare accepts null

and the _del doesn't?

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:36
> +    WKEinaSharedString() : m_string(0) { }

ALWAYS_INLINE WKEinaSharedString() ? RefPtr does this
Comment 9 Mikhail Pozdnyakov 2012-08-07 03:35:51 PDT
(In reply to comment #6)
> It would be nice to use it in the same patch to see how convenient it is. Personally I'm not convinced yet.

Please see example here: https://bugs.webkit.org/attachment.cgi?id=156900&action=prettypatch
Comment 10 Kenneth Rohde Christiansen 2012-08-07 03:37:54 PDT
(In reply to comment #6)
> It would be nice to use it in the same patch to see how convenient it is. Personally I'm not convinced yet.

It basically provides the same as OwnPtr in that it takes care of freeing for you which in complex code and refactorings easily can be forgotten. But anyway I agree with you and think it would be nice to include it
Comment 11 Mikhail Pozdnyakov 2012-08-07 03:39:20 PDT
Comment on attachment 156903 [details]
patch v2

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

>>>> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:60
>>>> +    ASSERT(str);
>>> 
>>> Eina stringshare accepts null
>> 
>> right, but do we want "einaSharedString = 0;" ?
> 
> and the _del doesn't?

From eina_stringshare_del docs "Note that if the given pointer is not shared or NULL, bad things will happen, likely a segmentation fault. " :)
Comment 12 Mikhail Pozdnyakov 2012-08-07 03:42:19 PDT
(In reply to comment #10)
> (In reply to comment #6)
> > It would be nice to use it in the same patch to see how convenient it is. Personally I'm not convinced yet.
> 
> It basically provides the same as OwnPtr in that it takes care of freeing for you which in complex code and refactorings easily can be forgotten. But anyway I agree with you and think it would be nice to include it

Shall I use WKEinaSharedString class through all the ewk_ classes in same patch? (Can do it but I'm a bit afraid that the patch becomes huge)
Comment 13 Chris Dumez 2012-08-07 03:44:33 PDT
Eina string share replace accepts null and it is used in several places in our code. The fact that you need to do a null check before calling del() is unrelated to the matter.
Comment 14 Chris Dumez 2012-08-07 03:45:52 PDT
(In reply to comment #7)
> (From update of attachment 156903 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156903&action=review
> 
> >> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:60
> >> +    ASSERT(str);
> > 
> > Eina stringshare accepts null
> 
> right, but do we want "einaSharedString = 0;" ?

Why not. I think of your class as OwnPtr. It accepts 0.
Comment 15 Chris Dumez 2012-08-07 03:51:08 PDT
(In reply to comment #12)
> (In reply to comment #10)
> > (In reply to comment #6)
> > > It would be nice to use it in the same patch to see how convenient it is. Personally I'm not convinced yet.
> > 
> > It basically provides the same as OwnPtr in that it takes care of freeing for you which in complex code and refactorings easily can be forgotten. But anyway I agree with you and think it would be nice to include it
> 
> Shall I use WKEinaSharedString class through all the ewk_ classes in same patch? (Can do it but I'm a bit afraid that the patch becomes huge)

Kenneth should decide but I propose to update just the back forward list code for now?
Comment 16 Mikhail Pozdnyakov 2012-08-07 03:56:20 PDT
(In reply to comment #14)
> (In reply to comment #7)
> > (From update of attachment 156903 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=156903&action=review
> > 
> > >> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:60
> > >> +    ASSERT(str);
> > > 
> > > Eina stringshare accepts null
> > 
> > right, but do we want "einaSharedString = 0;" ?
> 
> Why not. I think of your class as OwnPtr. It accepts 0.

That's a string class at the same time and such semantics would look weird IMHO.
I'd rather add empty() function or write einaSharedString = "";
Comment 17 Mikhail Pozdnyakov 2012-08-07 03:57:05 PDT
(In reply to comment #15)
> (In reply to comment #12)
> > (In reply to comment #10)
> > > (In reply to comment #6)
> > > > It would be nice to use it in the same patch to see how convenient it is. Personally I'm not convinced yet.
> > > 
> > > It basically provides the same as OwnPtr in that it takes care of freeing for you which in complex code and refactorings easily can be forgotten. But anyway I agree with you and think it would be nice to include it
> > 
> > Shall I use WKEinaSharedString class through all the ewk_ classes in same patch? (Can do it but I'm a bit afraid that the patch becomes huge)
> 
> Kenneth should decide but I propose to update just the back forward list code for now?

Agree with you, that I will do for sure
Comment 18 Chris Dumez 2012-08-07 04:27:59 PDT
(In reply to comment #16)
> (In reply to comment #14)
> > (In reply to comment #7)
> > > (From update of attachment 156903 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=156903&action=review
> > > 
> > > >> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:60
> > > >> +    ASSERT(str);
> > > > 
> > > > Eina stringshare accepts null
> > > 
> > > right, but do we want "einaSharedString = 0;" ?
> > 
> > Why not. I think of your class as OwnPtr. It accepts 0.
> 
> That's a string class at the same time and such semantics would look weird IMHO.
> I'd rather add empty() function or write einaSharedString = "";

Well, having isNull(), isEmpty() and reset() methods is a good idea. I still think supporting 0 is a good idea because it is used in EFL port. We use char* for in / out values and they may be null. If you don't support 0, this will result in extra code and checks when refactoring.
Comment 19 Chris Dumez 2012-08-07 04:29:44 PDT
(In reply to comment #18)
> (In reply to comment #16)
> > (In reply to comment #14)
> > > (In reply to comment #7)
> > > > (From update of attachment 156903 [details] [details] [details] [details])
> > > > View in context: https://bugs.webkit.org/attachment.cgi?id=156903&action=review
> > > > 
> > > > >> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:60
> > > > >> +    ASSERT(str);
> > > > > 
> > > > > Eina stringshare accepts null
> > > > 
> > > > right, but do we want "einaSharedString = 0;" ?
> > > 
> > > Why not. I think of your class as OwnPtr. It accepts 0.
> > 
> > That's a string class at the same time and such semantics would look weird IMHO.
> > I'd rather add empty() function or write einaSharedString = "";
> 
> Well, having isNull(), isEmpty() and reset() methods is a good idea. I still think supporting 0 is a good idea because it is used in EFL port. We use char* for in / out values and they may be null. If you don't support 0, this will result in extra code and checks when refactoring.

BTW, using "" instead of 0 has a different meaning. Empty string is different than null.
Comment 20 Mikhail Pozdnyakov 2012-08-07 04:33:37 PDT
(In reply to comment #19)
> (In reply to comment #18)
> > (In reply to comment #16)
> > > (In reply to comment #14)
> > > > (In reply to comment #7)
> > > > > (From update of attachment 156903 [details] [details] [details] [details] [details])
> > > > > View in context: https://bugs.webkit.org/attachment.cgi?id=156903&action=review
> > > > > 
> > > > > >> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:60
> > > > > >> +    ASSERT(str);
> > > > > > 
> > > > > > Eina stringshare accepts null
> > > > > 
> > > > > right, but do we want "einaSharedString = 0;" ?
> > > > 
> > > > Why not. I think of your class as OwnPtr. It accepts 0.
> > > 
> > > That's a string class at the same time and such semantics would look weird IMHO.
> > > I'd rather add empty() function or write einaSharedString = "";
> > 
> > Well, having isNull(), isEmpty() and reset() methods is a good idea. I still think supporting 0 is a good idea because it is used in EFL port. We use char* for in / out values and they may be null. If you don't support 0, this will result in extra code and checks when refactoring.
> 
> BTW, using "" instead of 0 has a different meaning. Empty string is different than null.

Yeah, that's just in case you want make smth empty..
Comment 21 Mikhail Pozdnyakov 2012-08-07 04:35:03 PDT
(In reply to comment #18)
> (In reply to comment #16)
> > (In reply to comment #14)
> > > (In reply to comment #7)
> > > > (From update of attachment 156903 [details] [details] [details] [details])
> > > > View in context: https://bugs.webkit.org/attachment.cgi?id=156903&action=review
> > > > 
> > > > >> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:60
> > > > >> +    ASSERT(str);
> > > > > 
> > > > > Eina stringshare accepts null
> > > > 
> > > > right, but do we want "einaSharedString = 0;" ?
> > > 
> > > Why not. I think of your class as OwnPtr. It accepts 0.
> > 
> > That's a string class at the same time and such semantics would look weird IMHO.
> > I'd rather add empty() function or write einaSharedString = "";
> 
> Well, having isNull(), isEmpty() and reset() methods is a good idea. I still think supporting 0 is a good idea because it is used in EFL port. We use char* for in / out values and they may be null. If you don't support 0, this will result in extra code and checks when refactoring.

Ok, let's keep it so far
Comment 22 Mikhail Pozdnyakov 2012-08-07 04:41:38 PDT
Created attachment 156912 [details]
patch v3

Considered comments from Kenneth and Chris. Thanks.
Comment 23 Kenneth Rohde Christiansen 2012-08-08 05:26:08 PDT
Comment on attachment 156912 [details]
patch v3

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

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:41
> +    ALWAYS_INLINE WKEinaSharedString(WKAdoptTag, WKRefType strRef)

Is the WKAdoptTag supposed to be ignored?

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:52
> +    ALWAYS_INLINE WKEinaSharedString(WKRefType strRef)

WKRefType could be something that doesn't have ->string(), no? Maybe it is better to specialize it to only accept WKStringRef and WKURLRef ?
Comment 24 Mikhail Pozdnyakov 2012-08-08 05:36:16 PDT
Comment on attachment 156912 [details]
patch v3

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

>> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:41
>> +    ALWAYS_INLINE WKEinaSharedString(WKAdoptTag, WKRefType strRef)
> 
> Is the WKAdoptTag supposed to be ignored?

Yes, at least WKRetainPtr ignores it.

>> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:52
>> +    ALWAYS_INLINE WKEinaSharedString(WKRefType strRef)
> 
> WKRefType could be something that doesn't have ->string(), no? Maybe it is better to specialize it to only accept WKStringRef and WKURLRef ?

I guess we can have other WKRefType having ->string() in future (for instance StringWithDirection ;-) ). Specialization will mean repeating of existing code basically.. I would say if we at some point have WKRefType having ->someWeirdStringGetter() we will specialize for that type.
Comment 25 Kenneth Rohde Christiansen 2012-08-08 05:39:18 PDT
(In reply to comment #24)
> (From update of attachment 156912 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156912&action=review
> 
> >> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:41
> >> +    ALWAYS_INLINE WKEinaSharedString(WKAdoptTag, WKRefType strRef)
> > 
> > Is the WKAdoptTag supposed to be ignored?
> 
> Yes, at least WKRetainPtr ignores it.
> 
> >> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:52
> >> +    ALWAYS_INLINE WKEinaSharedString(WKRefType strRef)
> > 
> > WKRefType could be something that doesn't have ->string(), no? Maybe it is better to specialize it to only accept WKStringRef and WKURLRef ?

Then we could ASSERT if it is not AdoptWK right?

> 
> I guess we can have other WKRefType having ->string() in future (for instance StringWithDirection ;-) ). Specialization will mean repeating of existing code basically.. I would say if we at some point have WKRefType having ->someWeirdStringGetter() we will specialize for that type.

But are there any WKRefType not having the string() method?
Comment 26 Mikhail Pozdnyakov 2012-08-08 05:43:46 PDT
Comment on attachment 156912 [details]
patch v3

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

>>>> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:41
>>>> +    ALWAYS_INLINE WKEinaSharedString(WKAdoptTag, WKRefType strRef)
>>> 
>>> Is the WKAdoptTag supposed to be ignored?
>> 
>> Yes, at least WKRetainPtr ignores it.
> 
> Then we could ASSERT if it is not AdoptWK right?

enum WKAdoptTag { AdoptWK }; -> guess, it has to be AdoptWK
Comment 27 Mikhail Pozdnyakov 2012-08-08 05:45:59 PDT
(In reply to comment #25)
> (In reply to comment #24)
> > (From update of attachment 156912 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=156912&action=review
> > 
> > >> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:41
> > >> +    ALWAYS_INLINE WKEinaSharedString(WKAdoptTag, WKRefType strRef)
> > > 
> > > Is the WKAdoptTag supposed to be ignored?
> > 
> > Yes, at least WKRetainPtr ignores it.
> > 
> > >> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:52
> > >> +    ALWAYS_INLINE WKEinaSharedString(WKRefType strRef)
> > > 
> > > WKRefType could be something that doesn't have ->string(), no? Maybe it is better to specialize it to only accept WKStringRef and WKURLRef ?
> 
> Then we could ASSERT if it is not AdoptWK right?
> 
> > 
> > I guess we can have other WKRefType having ->string() in future (for instance StringWithDirection ;-) ). Specialization will mean repeating of existing code basically.. I would say if we at some point have WKRefType having ->someWeirdStringGetter() we will specialize for that type.
> 
> But are there any WKRefType not having the string() method?
Think we have plenty of them, but if the Client puts inappropriate WKRefType to function, there will be compiler error..
Comment 28 Kenneth Rohde Christiansen 2012-08-08 05:47:05 PDT
> > But are there any WKRefType not having the string() method?
> Think we have plenty of them, but if the Client puts inappropriate WKRefType to function, there will be compiler error..

Sure I just find it more safe to open this up for classes we trust and where we know it makes sense.
Comment 29 Kenneth Rohde Christiansen 2012-08-08 05:48:08 PDT
(In reply to comment #26)
> (From update of attachment 156912 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156912&action=review
> 
> >>>> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.h:41
> >>>> +    ALWAYS_INLINE WKEinaSharedString(WKAdoptTag, WKRefType strRef)
> >>> 
> >>> Is the WKAdoptTag supposed to be ignored?
> >> 
> >> Yes, at least WKRetainPtr ignores it.
> > 
> > Then we could ASSERT if it is not AdoptWK right?
> 
> enum WKAdoptTag { AdoptWK }; -> guess, it has to be AdoptWK

It one is added at some point, you could still ASSERT. It is an enum so basically a number
Comment 30 Mikhail Pozdnyakov 2012-08-08 06:27:04 PDT
Created attachment 157201 [details]
patch v4

Kenneth, thanks for your patience.
Comment 31 Kenneth Rohde Christiansen 2012-08-08 06:36:21 PDT
Comment on attachment 157201 [details]
patch v4

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

Very nice

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:44
> +        if (adopt)
> +            WKRelease(strRef); // Have stored a copy into eina_stringshare, do not need adopted strRef.

What about the comment now?

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:61
> +    : m_string(fromRefType(stringRef, true /*adopt*/))

Don't we usually do /* adopt */ true? and not the other way around

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:63
> +    ASSERT(adoptTag == AdoptWK);

// Guard for future enum changes.

> Source/WebKit2/UIProcess/API/cpp/efl/WKEinaSharedString.cpp:82
> +
> +

unneeded double newline
Comment 32 Mikhail Pozdnyakov 2012-08-08 06:45:49 PDT
Created attachment 157205 [details]
patch v5
Comment 33 WebKit Review Bot 2012-08-08 07:41:10 PDT
Comment on attachment 157205 [details]
patch v5

Clearing flags on attachment: 157205

Committed r125035: <http://trac.webkit.org/changeset/125035>
Comment 34 WebKit Review Bot 2012-08-08 07:41:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 35 Gyuyoung Kim 2012-08-15 18:40:24 PDT
Mikhail, thank you for your great patch. :)  As I said webkit-efl mailing list, could you add this WKEinaSharedString usage to EFL WebKit coding style wiki page ?

http://trac.webkit.org/wiki/EFLWebKitCodingStyle
Comment 36 Kenneth Rohde Christiansen 2012-08-16 01:34:56 PDT
(In reply to comment #35)
> Mikhail, thank you for your great patch. :)  As I said webkit-efl mailing list, could you add this WKEinaSharedString usage to EFL WebKit coding style wiki page ?
> 
> http://trac.webkit.org/wiki/EFLWebKitCodingStyle

What list is that? can you add me ? :-)
Comment 37 Gyuyoung Kim 2012-08-16 02:53:11 PDT
(In reply to comment #36)
> (In reply to comment #35)
> > Mikhail, thank you for your great patch. :)  As I said webkit-efl mailing list, could you add this WKEinaSharedString usage to EFL WebKit coding style wiki page ?
> > 
> > http://trac.webkit.org/wiki/EFLWebKitCodingStyle
> 
> What list is that? can you add me ? :-)

As you know, EFL port had efl specific coding style in last year. So, many reviewer couldn't review patches for EFL port because of EFL specific coding style. EFL port decided to use WebKit style except for some specific cases. To guide changed EFL port coding style, I made the wiki page. In my humble opinion, everybody can change it after getting agreements from WebKit EFL folks. :-)
Comment 38 Kenneth Rohde Christiansen 2012-08-16 02:56:25 PDT
(In reply to comment #37)
> (In reply to comment #36)
> > (In reply to comment #35)
> > > Mikhail, thank you for your great patch. :)  As I said webkit-efl mailing list, could you add this WKEinaSharedString usage to EFL WebKit coding style wiki page ?
> > > 
> > > http://trac.webkit.org/wiki/EFLWebKitCodingStyle
> > 
> > What list is that? can you add me ? :-)
> 
> As you know, EFL port had efl specific coding style in last year. So, many reviewer couldn't review patches for EFL port because of EFL specific coding style. EFL port decided to use WebKit style except for some specific cases. To guide changed EFL port coding style, I made the wiki page. In my humble opinion, everybody can change it after getting agreements from WebKit EFL folks. :-)

I was talking about your mailing list, if you could add me.
Comment 39 Chris Dumez 2012-08-16 03:08:34 PDT
(In reply to comment #38)
> (In reply to comment #37)
> > (In reply to comment #36)
> > > (In reply to comment #35)
> > > > Mikhail, thank you for your great patch. :)  As I said webkit-efl mailing list, could you add this WKEinaSharedString usage to EFL WebKit coding style wiki page ?
> > > > 
> > > > http://trac.webkit.org/wiki/EFLWebKitCodingStyle
> > > 
> > > What list is that? can you add me ? :-)
> > 
> > As you know, EFL port had efl specific coding style in last year. So, many reviewer couldn't review patches for EFL port because of EFL specific coding style. EFL port decided to use WebKit style except for some specific cases. To guide changed EFL port coding style, I made the wiki page. In my humble opinion, everybody can change it after getting agreements from WebKit EFL folks. :-)
> 
> I was talking about your mailing list, if you could add me.

I thought you had already subscribed (didn't you comment about licensing there not so long ago?) Anyway, here is the link:
http://lists.webkit.org/mailman/listinfo/webkit-efl