Bug 137660

Summary: [EFL] Add message APIs to communicate between ewk_context and extensions
Product: WebKit Reporter: Ryuan Choi <ryuan.choi>
Component: WebKit EFLAssignee: Ryuan Choi <ryuan.choi>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, gyuyoung.kim, jinwoo7.song, lucas.de.marchi, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
rebased
none
Patch none

Description Ryuan Choi 2014-10-13 10:48:06 PDT
SSIA
Comment 1 Ryuan Choi 2014-10-13 11:16:00 PDT
Created attachment 239735 [details]
Patch
Comment 2 Ryuan Choi 2014-10-29 15:30:24 PDT
Gyuyoung, jinwoo,

Could you take a look at this?
Comment 3 Ryuan Choi 2014-11-06 22:24:07 PST
Created attachment 241162 [details]
rebased
Comment 4 Gyuyoung Kim 2015-02-10 08:05:58 PST
Comment on attachment 241162 [details]
rebased

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

Could you explain what do you want to do in this patch ?

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:528
> +        eina_value_get(body, &value);

Why does messageBody need to get eina_value_get() ?
Comment 5 Ryuan Choi 2015-02-11 03:48:52 PST
Comment on attachment 241162 [details]
rebased

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

This is the part of ewk_extension which wraps InjectedBundle as ewk API.
This patch provides some APIs of ewk_extension to communicate between ewk_context(UIProcess) and ewk_extension(WebProcess).

Without this, we don't have a way to do it without exposing InjectedBundle and WK C-APIs like tizen did.

>> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:528
>> +        eina_value_get(body, &value);
> 
> Why does messageBody need to get eina_value_get() ?

It's to support various types in the future although this patch just implements string type.

For example, we can support double or integer type or any structures which EFL supports to post from UIProcess to WebProcess.
Comment 6 Gyuyoung Kim 2015-02-16 07:48:56 PST
Comment on attachment 241162 [details]
rebased

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

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:411
> +    if (messageBody && WKStringGetTypeID() == WKGetTypeID(messageBody)) {

Can messageBody be null ? Isn't WKTypeRef a reference, not a pointer ?

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:526
> +    if (type == EINA_VALUE_TYPE_STRINGSHARE || type == EINA_VALUE_TYPE_STRING) {

Looks like early return is better.

if (type != EINA_VALUE_TYPE_STRINGSHARE && type != EINA_VALUE_TYPE_STRING)
    return false;

const char* value;
eina_value_get(body, &value);
messageBody = adoptWK(WKStringCreateWithUTF8CString(value));

> Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:89
> +    if (messageBody && WKStringGetTypeID() == WKGetTypeID(messageBody)) {

ditto.

> Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:137
> +    if (type == EINA_VALUE_TYPE_STRINGSHARE || type == EINA_VALUE_TYPE_STRING) {

ditto.
Comment 7 Ryuan Choi 2015-02-24 01:32:57 PST
Created attachment 247213 [details]
Patch
Comment 8 Ryuan Choi 2015-02-24 01:35:30 PST
(In reply to comment #6)
> Comment on attachment 241162 [details]
> rebased
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241162&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:411
> > +    if (messageBody && WKStringGetTypeID() == WKGetTypeID(messageBody)) {
> 
> Can messageBody be null ? Isn't WKTypeRef a reference, not a pointer ?
> 

Yes, WKTypeRef is defined like below in Source/WebKit2/Shared/API/c/WKBase.h
typedef const void* WKTypeRef;

Although we don't pass the empty message logically, I think that this check routine is reasonable.

> > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:526
> > +    if (type == EINA_VALUE_TYPE_STRINGSHARE || type == EINA_VALUE_TYPE_STRING) {
> 
> Looks like early return is better.
> 
Sure, I fixed.

> if (type != EINA_VALUE_TYPE_STRINGSHARE && type != EINA_VALUE_TYPE_STRING)
>     return false;
> 
> const char* value;
> eina_value_get(body, &value);
> messageBody = adoptWK(WKStringCreateWithUTF8CString(value));
> 
> > Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:89
> > +    if (messageBody && WKStringGetTypeID() == WKGetTypeID(messageBody)) {
> 
> ditto.
> 
> > Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:137
> > +    if (type == EINA_VALUE_TYPE_STRINGSHARE || type == EINA_VALUE_TYPE_STRING) {
> 
> ditto.
Comment 9 WebKit Commit Bot 2015-02-24 03:02:31 PST
Comment on attachment 247213 [details]
Patch

Clearing flags on attachment: 247213

Committed r180553: <http://trac.webkit.org/changeset/180553>
Comment 10 WebKit Commit Bot 2015-02-24 03:02:39 PST
All reviewed patches have been landed.  Closing bug.