Bug 137660 - [EFL] Add message APIs to communicate between ewk_context and extensions
Summary: [EFL] Add message APIs to communicate between ewk_context and extensions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-13 10:48 PDT by Ryuan Choi
Modified: 2015-02-24 03:02 PST (History)
6 users (show)

See Also:


Attachments
Patch (29.03 KB, patch)
2014-10-13 11:16 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
rebased (29.07 KB, patch)
2014-11-06 22:24 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (29.04 KB, patch)
2015-02-24 01:32 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.