Bug 96631 – [EFL] Remove a lot of C'ism from Ewk_Tiled_Backing_Store

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

[Chris Dumez]

Ewk_Tiled_Backing_Store contains a lot of C'ism and does not always follow coding style. It needs to be cleaned up.

[Chris Dumez]

Created attachment 163838 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 163838 [details]
Patch

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

LGTM but I guess someone else should have a look, lots of changes, easy to miss something

> Source/WebKit/efl/ChangeLog:9
> +        Remove a lot of C'ism from Ewk_Tiled_Backing_Store and
> +        fix coding style.

For WebKit2 we should really just use the tiled backing store Qt uses. It integrates very well with accelerated compositing etc and is 100% implemented in WebCore

> Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:202
>          CRITICAL("item->tile=%p, but it should be 0!", item->tile);

Why not using the EINA_ERROR_CRIT ?

> Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:234
> +    for (int i = 0; i < priv->view.rows; ++i) {

unsigned?

> Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:239
>          EINA_INLIST_FOREACH(list, item)
>              if (item->tile)
>                  _ewk_tiled_backing_store_tile_dissociate(priv, item, last_used);

This is wrong coding style, should use braces ebcause the content spans multiple actual lines

> Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:1014
> +    Eina_Inlist** iterator = priv->view.items;
> +    Eina_Inlist** iteratorEnd = iterator + priv->view.rows;

it
end ?

> Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:1777
>      }
>  
> -    if (!eina_tile_grid_slicer_setup(&renderSlicer,
> -        renderRect->x, renderRect->y, renderRect->w, renderRect->h, tileWidth, tileHeight)) {
> +        Eina_Tile_Grid_Slicer renderSlicer;
> +    if (!eina_tile_grid_slicer_setup(&renderSlicer, renderRect->x, renderRect->y, renderRect->w, renderRect->h, tileWidth, tileHeight)) {

isnt that indented wrongly?

[Chris Dumez]

Comment on attachment 163838 [details]
Patch

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

>> Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:202
>>          CRITICAL("item->tile=%p, but it should be 0!", item->tile);
> 
> Why not using the EINA_ERROR_CRIT ?

Well, it's not my code but it seems we use those macros in WK1 EFL. They are defined in ewk_private.h. Basically this macro is a wrapper for EINA_LOG_DOM_CRIT().

>> Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:234
>> +    for (int i = 0; i < priv->view.rows; ++i) {
> 
> unsigned?

Hmm. It is defined as a long in the struct :(
I'll switch to "unsigned long" then.

[Chris Dumez]

Created attachment 163863 [details]
Patch

Take Kenneth's feedback into consideration.

[WebKit Review Bot]

Comment on attachment 163863 [details]
Patch

Rejecting attachment 163863 [details] from commit-queue.

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

Last 500 characters of output:
  return self.open(self.click(*args, **kwds))
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 203, in open
    return self._mech_open(url, data, timeout=timeout)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 255, in _mech_open
    raise response
webkitpy.thirdparty.autoinstalled.mechanize._response.httperror_seek_wrapper: HTTP Error 500: Internal Server Error

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

[WebKit Review Bot]

Comment on attachment 163863 [details]
Patch

Clearing flags on attachment: 163863

Committed r128464: <http://trac.webkit.org/changeset/128464>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.