Bug 98889 – [EFL][WK2] Add APIs for cache model

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

[Jongseok Yang]

Add ewk APIs for WKContextSetCacheModel, WKContextGetCacheModel.

The patch will be uploaded soon.

[Jongseok Yang]

Created attachment 168372 [details]
Patch

[Jongseok Yang]

Created attachment 168624 [details]
Patch

[Chris Dumez]

Comment on attachment 168624 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:323
> +    switch (cacheModel) {

How about using COMPILE_ASSERT_MATCHING_ENUM() from ewk_private so that we can use casting instead of a switch?

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:58
> +    EWK_CACHE_MODEL_DOCUMENT_VIEWER,

Missing documentation for each enum value.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:282
> + */

Missing documentation for return value.

> Source/WebKit2/UIProcess/API/efl/ewk_context.h:292
> +EAPI Ewk_Cache_Model ewk_context_cache_model_get(Ewk_Context *context);

Argument should be const.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context.cpp:180
> +    ASSERT_TRUE(ewk_context_cache_model_set(context, EWK_CACHE_MODEL_DOCUMENT_VIEWER));

Please add a test for the default value as well.

[Jongseok Yang]

Created attachment 168650 [details]
Patch

[Jongseok Yang]

(In reply to comment #3)
> (From update of attachment 168624 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168624&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:323
> > +    switch (cacheModel) {
> 
> How about using COMPILE_ASSERT_MATCHING_ENUM() from ewk_private so that we can use casting instead of a switch?
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_context.h:58
> > +    EWK_CACHE_MODEL_DOCUMENT_VIEWER,
> 
> Missing documentation for each enum value.
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_context.h:282
> > + */
> 
> Missing documentation for return value.
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_context.h:292
> > +EAPI Ewk_Cache_Model ewk_context_cache_model_get(Ewk_Context *context);
> 
> Argument should be const.
> 
> > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context.cpp:180
> > +    ASSERT_TRUE(ewk_context_cache_model_set(context, EWK_CACHE_MODEL_DOCUMENT_VIEWER));
> 
> Please add a test for the default value as well.

Thank you for your review. I uploaded the new patch. Could you please check it?

[Chris Dumez]

Comment on attachment 168650 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 168650 [details]
Patch

Rejecting attachment 168650 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
mit-queue/Source/WebKit/chromium/third_party/skia/gyp --revision 5907 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
43>At revision 5907.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

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

[Jongseok Yang]

Created attachment 168838 [details]
Patch

[Jongseok Yang]

I rebased this patch.

[Jongseok Yang]

Created attachment 168841 [details]
Patch

[WebKit Review Bot]

Comment on attachment 168841 [details]
Patch

Clearing flags on attachment: 168841

Committed r131401: <http://trac.webkit.org/changeset/131401>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

The new unit test is failing on the bots:
/home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context.cpp:188: Failure
Value of: ewk_context_cache_model_get(context)
  Actual: 0
Expected: EWK_CACHE_MODEL_DOCUMENT_BROWSER
Which is: 1
[  FAILED  ] EWK2UnitTestBase.ewk_context_cache_model (12 ms)

Can you please fix it? I'm guessing the default value is wrong so the doc may need to be updated as well.

[Gyuyoung Kim]

(In reply to comment #13)
> The new unit test is failing on the bots:
> /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_context.cpp:188: Failure
> Value of: ewk_context_cache_model_get(context)
>   Actual: 0
> Expected: EWK_CACHE_MODEL_DOCUMENT_BROWSER
> Which is: 1
> [  FAILED  ] EWK2UnitTestBase.ewk_context_cache_model (12 ms)
> 
> Can you please fix it? I'm guessing the default value is wrong so the doc may need to be updated as well.

I already let him know this. He is fixing this.