A matrix is created inside ewk_tile_matrix_new and another matrix is created inside ewk_tile_matrix_zoom_level_set. So the first matrix is the duplicated one without any use. This patch is for getting rid of a duplicated creation of the matrix.
Created attachment 129218 [details] proposal patch
Created attachment 129455 [details] patch_1
Created attachment 130028 [details] patch_2
Comment on attachment 130028 [details] patch_2 Patch looks good, one minor thing: View in context: https://bugs.webkit.org/attachment.cgi?id=130028&action=review > Source/WebKit/efl/ewk/ewk_tiled_matrix.cpp:371 > + eina_matrixsparse_size_get(tileMatrix->matrix , rows, columns); remove space before "rows"
Created attachment 131304 [details] patch_3
Comment on attachment 131304 [details] patch_3 View in context: https://bugs.webkit.org/attachment.cgi?id=131304&action=review > Source/WebKit/efl/ewk/ewk_tiled_matrix.cpp:238 > + * Find the matrix with the same zoom(@param) and set it as current matrix. Is @param needed certainly in this comment ? > Source/WebKit/efl/ewk/ewk_tiled_matrix.cpp:366 > + * Get the current matrix size. Missing comments related to param, return for this function.
Comment on attachment 131304 [details] patch_3 View in context: https://bugs.webkit.org/attachment.cgi?id=131304&action=review >> Source/WebKit/efl/ewk/ewk_tiled_matrix.cpp:238 >> + * Find the matrix with the same zoom(@param) and set it as current matrix. > > Is @param needed certainly in this comment ? I thought It could make more clear but I could remove it. >> Source/WebKit/efl/ewk/ewk_tiled_matrix.cpp:366 >> + * Get the current matrix size. > > Missing comments related to param, return for this function. This API is obvious for purpose. Does it need more description?
Created attachment 131308 [details] patch_4 Remove a word on the comment.
(In reply to comment #7) > This API is obvious for purpose. Does it need more description? If you want to add function description, in my humble opinion, it is better to adhere doxyzen rule regardless of internal | public. ewk_view.cpp also added function description as below, /** * @internal * * @param ewkView View. * @param message String to show on console. * @param lineNumber Line number. * @sourceID Source id. * */ void ewk_view_add_console_message(Evas_Object* ewkView, const char* message, unsigned int lineNumber, const char* sourceID) Grzegorz, how do you think about it ?
(In reply to comment #9) > (In reply to comment #7) > > This API is obvious for purpose. Does it need more description? > If you want to add function description, in my humble opinion, it is better to adhere doxyzen rule regardless of internal | public. ewk_view.cpp also added function description as below, > /** > * @internal > * > * @param ewkView View. > * @param message String to show on console. > * @param lineNumber Line number. > * @sourceID Source id. > * > */ > void ewk_view_add_console_message(Evas_Object* ewkView, const char* message, unsigned int lineNumber, const char* sourceID) > Grzegorz, how do you think about it ? If this function requires documentation I also will use doxygen. Generally backing store doesn't have extended documentation (its purpose is rather for internal use).In my opinion there is no needed to add doxygen documentation for each backing store function.
Created attachment 131565 [details] patch_5 Minor fix : remove the comment.
LGTM.
Comment on attachment 131565 [details] patch_5 r=me
Comment on attachment 131565 [details] patch_5 Clearing flags on attachment: 131565 Committed r110556: <http://trac.webkit.org/changeset/110556>
All reviewed patches have been landed. Closing bug.