Bug 90936 – [EFL] Add a API for getting security origin string

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Kihong Kwon]

Application(e.g browser) need to keep permitted origin list for the Web Notification.
But We don't have a way to store security origin in the application side.
We can support to store security origin by security origin string getter.

[Kihong Kwon]

Created attachment 151594 [details]
Patch

[Chris Dumez]

Comment on attachment 151594 [details]
Patch

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

> Source/WebKit/efl/ewk/ewk_security_origin.cpp:59
> +const char* ewk_security_origin_string_get(Ewk_Security_Origin *origin)

star should be next to Ewk_Security_Origin, not origin.

> Source/WebKit/efl/ewk/ewk_security_origin.h:78
> + * Returns the security origin string from Ewk_Security_Origin.

Returns the string representation of the security origin?

> Source/WebKit/efl/ewk/ewk_security_origin.h:79
> + *

It would be nice to copy the doc from WebCore here to provide more information: e.g. "Convert this SecurityOrigin into a string. The string representation of a SecurityOrigin is similar to a URL, except it lacks a path component. The string representation does not encode the value of the security origin's domain property".

We should document that it may return the string "null" when the security origin is unique (check WebCore::SecurityOrigin::toString() documentation for more details).

> Source/WebKit/efl/ewk/ewk_security_origin.h:82
> + * @return url stirng from Ewk_Securiry_Origin.

It does not necessarily return a URL and there is a type in "string". I would use "String representation of the security origin".

> Source/WebKit/efl/ewk/ewk_security_origin.h:84
> +EAPI const char          *ewk_security_origin_string_get(Ewk_Security_Origin *o);

Argument should be const.

[Kihong Kwon]

Comment on attachment 151594 [details]
Patch

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

>> Source/WebKit/efl/ewk/ewk_security_origin.cpp:59
>> +const char* ewk_security_origin_string_get(Ewk_Security_Origin *origin)
> 
> star should be next to Ewk_Security_Origin, not origin.

OK.

>> Source/WebKit/efl/ewk/ewk_security_origin.h:78
>> + * Returns the security origin string from Ewk_Security_Origin.
> 
> Returns the string representation of the security origin?

OK. Thanks.

>> Source/WebKit/efl/ewk/ewk_security_origin.h:84
>> +EAPI const char          *ewk_security_origin_string_get(Ewk_Security_Origin *o);
> 
> Argument should be const.

It can't be.
Argument can be changed in the ewk_security_origin_string_get.

[Chris Dumez]

Comment on attachment 151594 [details]
Patch

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

>>> Source/WebKit/efl/ewk/ewk_security_origin.h:84
>>> +EAPI const char          *ewk_security_origin_string_get(Ewk_Security_Origin *o);
>> 
>> Argument should be const.
> 
> It can't be.
> Argument can be changed in the ewk_security_origin_string_get.

Sure it can. It is a getter so the argument MUST be const. I believe you're having trouble because of the estringshare optimization. For that, just use const_cast.

[Kihong Kwon]

Created attachment 152224 [details]
Patch

[Chris Dumez]

Comment on attachment 152224 [details]
Patch

LGTM.

[Gyuyoung Kim]

Comment on attachment 152224 [details]
Patch

LGTM, thanks.

[WebKit Review Bot]

Comment on attachment 152224 [details]
Patch

Clearing flags on attachment: 152224

Committed r122693: <http://trac.webkit.org/changeset/122693>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.