Bug 52139

Created attachment 78368 [details] Patch

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. */

(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?

(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.

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

Created attachment 84063 [details] Patch

(In reply to comment #6) > Created an attachment (id=84063) [details] > Patch update lukasz's comment in Bug 55258.

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.

Created attachment 84352 [details] Patch

Created attachment 84354 [details] Patch

(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.

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

(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.

Created attachment 85578 [details] revised_patch

(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.

The commit-queue encountered the following flaky tests while processing attachment 85578 [details]: transitions/interrupted-accelerated-transition.html bug 56242 (authors: simon.fraser@apple.com and tonyg@chromium.org) The commit-queue is continuing to process your patch.

Comment on attachment 85578 [details] revised_patch Clearing flags on attachment: 85578 Committed r81004: <http://trac.webkit.org/changeset/81004>

All reviewed patches have been landed. Closing bug.