Ewk_Tiled_Backing_Store contains a lot of C'ism and does not always follow coding style. It needs to be cleaned up.
Created attachment 163838 [details] Patch
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?
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.
Created attachment 163863 [details] Patch Take Kenneth's feedback into consideration.
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
Comment on attachment 163863 [details] Patch Clearing flags on attachment: 163863 Committed r128464: <http://trac.webkit.org/changeset/128464>
All reviewed patches have been landed. Closing bug.