Bug 67588 - [EFL] Move assignment of some variables out of loop for ewk_tiled_backing_store.
: [EFL] Move assignment of some variables out of loop for ewk_tiled_backing_store.
Status: RESOLVED FIXED
: WebKit
WebKit EFL
: 528+ (Nightly build)
: Other Linux
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2011-09-05 00:46 PST by
Modified: 2011-10-25 18:14 PST (History)


Attachments
Patch. (4.19 KB, patch)
2011-09-05 00:48 PST, KwangHyuk
no flags Review Patch | Details | Formatted Diff | Diff
Patch rebased. (4.20 KB, application/octet-stream)
2011-10-21 03:49 PST, KwangHyuk
no flags Details
patch rebased. (4.20 KB, patch)
2011-10-21 03:51 PST, KwangHyuk
no flags Review Patch | Details | Formatted Diff | Diff
Patch rebase. (6.50 KB, application/octet-stream)
2011-10-23 21:34 PST, KwangHyuk
no flags Details
Patch rebased. (6.50 KB, patch)
2011-10-23 21:36 PST, KwangHyuk
no flags Review Patch | Details | Formatted Diff | Diff
Remove tab. (6.51 KB, patch)
2011-10-23 21:49 PST, KwangHyuk
no flags Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2011-09-05 00:46:39 PST
Some of variables which aren't related with loop count or other variables inside of loop are moved out of loop.
------- Comment #1 From 2011-09-05 00:48:59 PST -------
Created an attachment (id=106312) [details]
Patch.
------- Comment #2 From 2011-09-05 01:01:12 PST -------
(From update of attachment 106312 [details])
LGTM. However, are there any test cases in order to check if this patch makes side effects?
------- Comment #3 From 2011-09-05 01:22:34 PST -------
> 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 From 2011-09-05 04:41:27 PST -------
LGTM too.
------- Comment #5 From 2011-09-12 16:02:07 PST -------
(From update of attachment 106312 [details])
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 From 2011-09-13 17:47:43 PST -------
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 From 2011-10-21 03:49:58 PST -------
Created an attachment (id=111936) [details]
Patch rebased.
------- Comment #8 From 2011-10-21 03:51:06 PST -------
Created an attachment (id=111937) [details]
patch rebased.
------- Comment #9 From 2011-10-23 21:34:28 PST -------
Created an attachment (id=112140) [details]
Patch rebase.

Refactoring code from https://bugs.webkit.org/show_bug.cgi?id=69988 was migrated.
------- Comment #10 From 2011-10-23 21:36:02 PST -------
Created an attachment (id=112142) [details]
Patch rebased.

Refactoring code from https://bugs.webkit.org/show_bug.cgi?id=69988 was migrated.
------- Comment #11 From 2011-10-23 21:37:05 PST -------
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 From 2011-10-23 21:38:01 PST -------
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 From 2011-10-23 21:49:14 PST -------
Created an attachment (id=112143) [details]
Remove tab.
------- Comment #14 From 2011-10-23 22:40:31 PST -------
(From update of attachment 112143 [details])
LGTM. I missed to change these variables on Bug 69988.
------- Comment #15 From 2011-10-25 17:31:18 PST -------
(From update of attachment 112143 [details])
Looks reasonable.
------- Comment #16 From 2011-10-25 18:14:28 PST -------
(From update of attachment 112143 [details])
Clearing flags on attachment: 112143

Committed r98419: <http://trac.webkit.org/changeset/98419>
------- Comment #17 From 2011-10-25 18:14:34 PST -------
All reviewed patches have been landed.  Closing bug.