Bug 67588 - [EFL] Move assignment of some variables out of loop for ewk_tiled_backing_store.
Summary: [EFL] Move assignment of some variables out of loop for ewk_tiled_backing_store.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-05 00:46 PDT by KwangHyuk
Modified: 2011-10-25 18:14 PDT (History)
5 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description KwangHyuk 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.
Comment 1 KwangHyuk 2011-09-05 00:48:59 PDT 
Created attachment 106312 [details]
Patch.
Comment 2 Gyuyoung Kim 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?
Comment 3 KwangHyuk 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.
Comment 4 Raphael Kubo da Costa (:rakuco) 2011-09-05 04:41:27 PDT 
LGTM too.
Comment 5 Kenneth Rohde Christiansen 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++
Comment 6 KwangHyuk 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. :-)
Comment 7 KwangHyuk 2011-10-21 03:49:58 PDT 
Created attachment 111936 [details]
Patch rebased.
Comment 8 KwangHyuk 2011-10-21 03:51:06 PDT 
Created attachment 111937 [details]
patch rebased.
Comment 9 KwangHyuk 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.
Comment 10 KwangHyuk 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.
Comment 11 WebKit Review Bot 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.
Comment 12 WebKit Review Bot 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.
Comment 13 KwangHyuk 2011-10-23 21:49:14 PDT 
Created attachment 112143 [details]
Remove tab.
Comment 14 Gyuyoung Kim 2011-10-23 22:40:31 PDT 
Comment on attachment 112143 [details]
Remove tab.

LGTM. I missed to change these variables on Bug 69988.
Comment 15 Adam Barth 2011-10-25 17:31:18 PDT 
Comment on attachment 112143 [details]
Remove tab.

Looks reasonable.
Comment 16 WebKit Review Bot 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>
Comment 17 WebKit Review Bot 2011-10-25 18:14:34 PDT 
All reviewed patches have been landed.  Closing bug.