Bug 87638 – [EFL][DRT] Implement dumpFrameScrollPosition

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Kangil Han]

This patch implements dumpFrameScrollPosition function.

[Kangil Han]

Code is done.
Be back after regression check.

[Ryuan Choi]

Created attachment 151893 [details]
Patch

[Gyuyoung Kim]

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.

[Ryuan Choi]

Created attachment 152233 [details]
Patch

[Ryuan Choi]

(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.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Ryuan Choi]

Created attachment 152468 [details]
rebased for green bot

[Chris Dumez]

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.

[Chris Dumez]

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.

[Gyuyoung Kim]

Comment on attachment 152468 [details]
rebased for green bot

Looks fine.

[Hajime Morrita]

Comment on attachment 152468 [details]
rebased for green bot

rubberstamping.

[WebKit Review Bot]

Comment on attachment 152468 [details]
rebased for green bot

Clearing flags on attachment: 152468

Committed r122798: <http://trac.webkit.org/changeset/122798>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.