Bug 52139 – [EFL] Move cache_directory api

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Ryuan Choi]

I think that cache directory is not related to ewk_view.

So, I want to change two cache directory apis like below.
ewk_view_cache_directory_path_[set|get] -> ewk_settings_cache_directory_path_[set|get]

[Ryuan Choi]

Created attachment 78368 [details]
Patch

[Gyuyoung Kim]

LGTM :)

BTW, Why don't you move the description to above "@return" ?


/**
 * Return cache directory path.
 *
 * This is guaranteed to be eina_stringshare, so whenever possible
 * save yourself some cpu cycles and use eina_stringshare_ref() instead of 
 * eina_stringshare_add() or strdup().
 *
 * @return cache directory path.
 */

[Lucas De Marchi]

(In reply to comment #0)
> I think that cache directory is not related to ewk_view.
> 
> So, I want to change two cache directory apis like below.
> ewk_view_cache_directory_path_[set|get] -> ewk_settings_cache_directory_path_[set|get]

Looks good, however this breaks the API. Leandro, what do you think about this?

[Rafael Antognolli]

(In reply to comment #3)
> (In reply to comment #0)
> > I think that cache directory is not related to ewk_view.
> > 
> > So, I want to change two cache directory apis like below.
> > ewk_view_cache_directory_path_[set|get] -> ewk_settings_cache_directory_path_[set|get]
> 
> Looks good, however this breaks the API. Leandro, what do you think about this?

I think Ryuan is right, cache directory is not related to ewk_view, it's a global setting. Just my 2 cents.

[Lukasz Slachciak]

*** Bug 55258 has been marked as a duplicate of this bug. ***

[Ryuan Choi]

Created attachment 84063 [details]
Patch

[Ryuan Choi]

(In reply to comment #6)
> Created an attachment (id=84063) [details]
> Patch

update lukasz's comment in Bug 55258.

[Gyuyoung Kim]

Comment on attachment 84063 [details]
Patch

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

> Source/WebKit/efl/ewk/ewk_settings.cpp:58
> +#endif

Hmm, why don't you move this character constant to 53 line according to alphabet order ? But, when we use macro, I'm not sure if we should adhere alphabet order.

[Ryuan Choi]

Created attachment 84352 [details]
Patch

[Ryuan Choi]

Created attachment 84354 [details]
Patch

[Ryuan Choi]

(In reply to comment #8)
> (From update of attachment 84063 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84063&action=review
> 
> > Source/WebKit/efl/ewk/ewk_settings.cpp:58
> > +#endif
> 
> Hmm, why don't you move this character constant to 53 line according to alphabet order ? But, when we use macro, I'm not sure if we should adhere alphabet order.

Thank you and I moved it like you mentioned.

[WebKit Commit Bot]

Comment on attachment 84354 [details]
Patch

Rejecting attachment 84354 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'apply-..." exit_code: 2

Last 500 characters of output:
ain.cpp
patching file Source/WebKit/efl/ewk/ewk_settings.cpp
Hunk #2 FAILED at 339.
1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/efl/ewk/ewk_settings.cpp.rej
patching file Source/WebKit/efl/ewk/ewk_settings.h
patching file Source/WebKit/efl/ewk/ewk_view.cpp
Hunk #5 succeeded at 2717 (offset -2 lines).
patching file Source/WebKit/efl/ewk/ewk_view.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christia..." exit_code: 1

Full output: http://queues.webkit.org/results/8114498

[Gyuyoung Kim]

(In reply to comment #12)
> (From update of attachment 84354 [details])
> Rejecting attachment 84354 [details] from commit-queue.
> 
> Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'apply-..." exit_code: 2
> 
> Last 500 characters of output:
> ain.cpp
> patching file Source/WebKit/efl/ewk/ewk_settings.cpp
> Hunk #2 FAILED at 339.
> 1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/efl/ewk/ewk_settings.cpp.rej
> patching file Source/WebKit/efl/ewk/ewk_settings.h
> patching file Source/WebKit/efl/ewk/ewk_view.cpp
> Hunk #5 succeeded at 2717 (offset -2 lines).
> patching file Source/WebKit/efl/ewk/ewk_view.h
> 
> Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christia..." exit_code: 1
> 
> Full output: http://queues.webkit.org/results/8114498

Hmm, It seems this patch is a little old. Please make this page again on latest trunk.

[Ryuan Choi]

Created attachment 85578 [details]
revised_patch

[Ryuan Choi]

(In reply to comment #14)
> Created an attachment (id=85578) [details]
> revised_patch

I revised previous patch on latest trunk.
One difference is comment style gyuyoung mentioned above.

Other is same as old one.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 85578 [details]:

transitions/interrupted-accelerated-transition.html bug 56242 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 85578 [details]
revised_patch

Clearing flags on attachment: 85578

Committed r81004: <http://trac.webkit.org/changeset/81004>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.