Bug 87638 - [EFL][DRT] Implement dumpFrameScrollPosition
Summary: [EFL][DRT] Implement dumpFrameScrollPosition
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: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks: 87637
  Show dependency treegraph
 
Reported: 2012-05-28 01:50 PDT by Kangil Han
Modified: 2012-07-16 20:01 PDT (History)
9 users (show)

See Also:


Attachments
Patch (62.94 KB, patch)
2012-07-12 02:28 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (62.88 KB, patch)
2012-07-13 05:58 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-08 (361.87 KB, application/zip)
2012-07-13 09:14 PDT, WebKit Review Bot
no flags Details
rebased for green bot (62.92 KB, patch)
2012-07-15 18:39 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kangil Han 2012-05-28 01:50:51 PDT
This patch implements dumpFrameScrollPosition function.
Comment 1 Kangil Han 2012-05-28 01:51:31 PDT
Code is done.
Be back after regression check.
Comment 2 Ryuan Choi 2012-07-12 02:28:10 PDT
Created attachment 151893 [details]
Patch
Comment 3 Gyuyoung Kim 2012-07-12 21:58:10 PDT
Comment on attachment 151893 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=151893&action=review

> Tools/DumpRenderTree/efl/DumpRenderTree.cpp:115
> +    }

nit : It looks empty line is needed.

> Tools/DumpRenderTree/efl/DumpRenderTree.cpp:118
> +    if (abs(x) > 0 || abs(y) > 0) {

In GTK port case, it print frame name as well. Is it better to print frame name as well ?

> Tools/DumpRenderTree/efl/DumpRenderTree.cpp:125
> +    printf("%s", result.toString().utf8().data());

In GTK port case, it print result only when if (abs(x) > 0 || abs(y) > 0) is true. Don't we need to do as well ?

> Tools/DumpRenderTree/efl/DumpRenderTree.cpp:133
> +

nit : It seems to me this line is not needed.
Comment 4 Ryuan Choi 2012-07-13 05:58:55 PDT
Created attachment 152233 [details]
Patch
Comment 5 Ryuan Choi 2012-07-13 06:01:46 PDT
(In reply to comment #3)
> (From update of attachment 151893 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=151893&action=review
> 
> > Tools/DumpRenderTree/efl/DumpRenderTree.cpp:115
> > +    }
> 
> nit : It looks empty line is needed.
> 
I moved them.

> > Tools/DumpRenderTree/efl/DumpRenderTree.cpp:118
> > +    if (abs(x) > 0 || abs(y) > 0) {
> 
> In GTK port case, it print frame name as well. Is it better to print frame name as well ?
moved. It looks better.

> 
> > Tools/DumpRenderTree/efl/DumpRenderTree.cpp:125
> > +    printf("%s", result.toString().utf8().data());
> 
> In GTK port case, it print result only when if (abs(x) > 0 || abs(y) > 0) is true. Don't we need to do as well ?
> 
> > Tools/DumpRenderTree/efl/DumpRenderTree.cpp:133
> > +
> 
> nit : It seems to me this line is not needed.

Thank you. I fixed like GTK port did.
Comment 6 WebKit Review Bot 2012-07-13 09:13:59 PDT
Comment on attachment 152233 [details]
Patch

Attachment 152233 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13221690

New failing tests:
http/tests/w3c/webperf/approved/navigation-timing/html5/test_performance_attributes_exist_in_object.html
Comment 7 WebKit Review Bot 2012-07-13 09:14:04 PDT
Created attachment 152273 [details]
Archive of layout-test-results from gce-cr-linux-08

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-08  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 8 Ryuan Choi 2012-07-15 18:39:05 PDT
Created attachment 152468 [details]
rebased for green bot
Comment 9 Chris Dumez 2012-07-15 23:01:13 PDT
Comment on attachment 152468 [details]
rebased for green bot

View in context: https://bugs.webkit.org/attachment.cgi?id=152468&action=review

> Tools/ChangeLog:7
> +

Missing Changelog.
Comment 10 Chris Dumez 2012-07-15 23:06:58 PDT
Comment on attachment 152468 [details]
rebased for green bot

View in context: https://bugs.webkit.org/attachment.cgi?id=152468&action=review

>> Tools/ChangeLog:7
>> +
> 
> Missing Changelog.

Nevermind, I failed to notice you added per function comments. LGTM then.
Comment 11 Gyuyoung Kim 2012-07-16 18:00:50 PDT
Comment on attachment 152468 [details]
rebased for green bot

Looks fine.
Comment 12 Hajime Morrita 2012-07-16 19:04:55 PDT
Comment on attachment 152468 [details]
rebased for green bot

rubberstamping.
Comment 13 WebKit Review Bot 2012-07-16 20:01:44 PDT
Comment on attachment 152468 [details]
rebased for green bot

Clearing flags on attachment: 152468

Committed r122798: <http://trac.webkit.org/changeset/122798>
Comment 14 WebKit Review Bot 2012-07-16 20:01:51 PDT
All reviewed patches have been landed.  Closing bug.