Bug 90936 - [EFL] Add a API for getting security origin string
Summary: [EFL] Add a API for getting security origin string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 90954
Blocks: 90603
  Show dependency treegraph
 
Reported: 2012-07-10 21:43 PDT by Kihong Kwon
Modified: 2012-07-15 20:22 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.61 KB, patch)
2012-07-10 21:53 PDT, Kihong Kwon
no flags Details | Formatted Diff | Diff
Patch (3.97 KB, patch)
2012-07-13 05:23 PDT, Kihong Kwon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kihong Kwon 2012-07-10 21:43:03 PDT
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.
Comment 1 Kihong Kwon 2012-07-10 21:53:53 PDT
Created attachment 151594 [details]
Patch
Comment 2 Chris Dumez 2012-07-10 22:35:53 PDT
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.
Comment 3 Kihong Kwon 2012-07-10 22:58:06 PDT
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.
Comment 4 Chris Dumez 2012-07-10 23:01:19 PDT
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.
Comment 5 Kihong Kwon 2012-07-13 05:23:00 PDT
Created attachment 152224 [details]
Patch
Comment 6 Chris Dumez 2012-07-13 05:26:33 PDT
Comment on attachment 152224 [details]
Patch

LGTM.
Comment 7 Gyuyoung Kim 2012-07-15 19:44:19 PDT
Comment on attachment 152224 [details]
Patch

LGTM, thanks.
Comment 8 WebKit Review Bot 2012-07-15 20:22:47 PDT
Comment on attachment 152224 [details]
Patch

Clearing flags on attachment: 152224

Committed r122693: <http://trac.webkit.org/changeset/122693>
Comment 9 WebKit Review Bot 2012-07-15 20:22:52 PDT
All reviewed patches have been landed.  Closing bug.