Bug 67588

Summary:

[EFL] Move assignment of some variables out of loop for ewk_tiled_backing_store.

Product:

WebKit

Reporter:

KwangHyuk <hyuki.kim>

Component:

WebKit EFL

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

gyuyoung.kim, leandro, lucas.de.marchi, rakuco, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

Linux

Attachments:

Description	Flags
Patch.	none
Patch rebased.	none
patch rebased.	none
Patch rebase.	none
Patch rebased.	none
Remove tab.	none

KwangHyuk

Reported 2011-09-05 00:46:39 PDT

Some of variables which aren't related with loop count or other variables inside of loop are moved out of loop.

Attachments
Patch. (4.19 KB, patch) 2011-09-05 00:48 PDT, KwangHyuk	no flags	Details Formatted Diff Diff
Patch rebased. (4.20 KB, application/octet-stream) 2011-10-21 03:49 PDT, KwangHyuk	no flags	Details
patch rebased. (4.20 KB, patch) 2011-10-21 03:51 PDT, KwangHyuk	no flags	Details Formatted Diff Diff
Patch rebase. (6.50 KB, application/octet-stream) 2011-10-23 21:34 PDT, KwangHyuk	no flags	Details
Patch rebased. (deleted) 2011-10-23 21:36 PDT, KwangHyuk	no flags	Details Formatted Diff Diff
Remove tab. (6.51 KB, patch) 2011-10-23 21:49 PDT, KwangHyuk	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

KwangHyuk

Comment 1 2011-09-05 00:48:59 PDT

Created attachment 106312 [details] Patch.

Gyuyoung Kim

Comment 2 2011-09-05 01:01:12 PDT

Comment on attachment 106312 [details] Patch. LGTM. However, are there any test cases in order to check if this patch makes side effects?

KwangHyuk

Comment 3 2011-09-05 01:22:34 PDT

> LGTM. However, are there any test cases in order to check if this patch makes side effects? I believe that scroll via EWebLauncher would be enough for test usage. :) However, there was no logic change because this is just for small optimization to reduce repetition of ineffective variable assignment inside of loop.

Raphael Kubo da Costa (:rakuco)

Comment 4 2011-09-05 04:41:27 PDT

LGTM too.

Kenneth Rohde Christiansen

Comment 5 2011-09-12 16:02:07 PDT

Comment on attachment 106312 [details] Patch. I really hope you can move over to using the Qt tiling implementation. That is quite easy to understand and written in C++

KwangHyuk

Comment 6 2011-09-13 17:47:43 PDT

Hi, Kenneth, I am thinking that webkit EFL tiling implementation should be maintained and optimized too until we will be able to get advantage from qt tiling implementation. And moreover, I believe that both two can be mixed too. Fortunately, One of our guys is trying to let WebCore inside backing store logic be integrated with webkit efl. You can see the patch set that he will contribute soon. :-)

KwangHyuk

Comment 7 2011-10-21 03:49:58 PDT

Created attachment 111936 [details] Patch rebased.

KwangHyuk

Comment 8 2011-10-21 03:51:06 PDT

Created attachment 111937 [details] patch rebased.

KwangHyuk

Comment 9 2011-10-23 21:34:28 PDT

Created attachment 112140 [details] Patch rebase. Refactoring code from https://bugs.webkit.org/show_bug.cgi?id=69988 was migrated.

KwangHyuk

Comment 10 2011-10-23 21:36:02 PDT

Created attachment 112142 [details] Patch rebased. Refactoring code from https://bugs.webkit.org/show_bug.cgi?id=69988 was migrated.

WebKit Review Bot

Comment 11 2011-10-23 21:37:05 PDT

Attachment 112140 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/efl/ChangeLog', u'Source/Web..." exit_code: 1 Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:1058: Tab found; better to use spaces [whitespace/tab] [1] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Review Bot

Comment 12 2011-10-23 21:38:01 PDT

Attachment 112142 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/efl/ChangeLog', u'Source/Web..." exit_code: 1 Source/WebKit/efl/ewk/ewk_tiled_backing_store.cpp:1058: Tab found; better to use spaces [whitespace/tab] [1] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

KwangHyuk

Comment 13 2011-10-23 21:49:14 PDT

Created attachment 112143 [details] Remove tab.

Gyuyoung Kim

Comment 14 2011-10-23 22:40:31 PDT

Comment on attachment 112143 [details] Remove tab. LGTM. I missed to change these variables on Bug 69988.

Adam Barth

Comment 15 2011-10-25 17:31:18 PDT

Comment on attachment 112143 [details] Remove tab. Looks reasonable.

WebKit Review Bot

Comment 16 2011-10-25 18:14:28 PDT

Comment on attachment 112143 [details] Remove tab. Clearing flags on attachment: 112143 Committed r98419: <http://trac.webkit.org/changeset/98419>

WebKit Review Bot

Comment 17 2011-10-25 18:14:34 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.