Bug 77325 – [EFL] Broken rendering occurs when scrolling in ewk_view_single.

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

[Ryuan Choi]

After r104687, broken rendering occur when scrolling contents.

You can see this when loading and scrolling http://news.google.co.kr/nwshp?hl=ko&tab=wn.

[Ryuan Choi]

Created attachment 124517 [details]
Patch

[KwangHyuk]

> Source/WebKit/efl/ChangeLog:3
> +        [EFL] Broken rendering occur when scrolling in ewk_view_single.

occur -> occurs ? :)

> Source/WebKit/efl/ChangeLog:8
> +        After r104687, broken rendering occur when scrolling contents.

Ditto.

> Source/WebKit/efl/ChangeLog:11
> +        This patch fix it and rename parameter to imageWidth to avoid confusion.

fix -> fixes.
rename -> renames.
"to imageWidth" may not be required. :)

[Ryuan Choi]

Created attachment 124665 [details]
Patch

[KwangHyuk]

(In reply to comment #3)
> Created an attachment (id=124665) [details]
> Patch

LGTM.

[Ryuan Choi]

Add Jungjik for double-check and Zoltan who can review this.

[JungJik Lee]

(In reply to comment #5)
> Add Jungjik for double-check and Zoltan who can review this.

Oh, it's embarrassing me and I am sorry for the mistake. I thought I had tested enough. Could you let me know which page occur this issue? I will check it more deeply and next time i'll try to write a test script.

[Ryuan Choi]

(In reply to comment #6)
> (In reply to comment #5)
> > Add Jungjik for double-check and Zoltan who can review this.
> 
> Oh, it's embarrassing me and I am sorry for the mistake. I thought I had tested enough. Could you let me know which page occur this issue? I will check it more deeply and next time i'll try to write a test script.

No problem. This is just a bug caused by good refactoring.

I found this issue in http://news.google.co.kr/nwshp?hl=ko&tab=wn.

Unfortunately, Efl port doesn't have pixel-tests yet.
So I didn't create test case for this bug.

[Gyuyoung Kim]

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Add Jungjik for double-check and Zoltan who can review this.
> > 
> > Oh, it's embarrassing me and I am sorry for the mistake. I thought I had tested enough. Could you let me know which page occur this issue? I will check it more deeply and next time i'll try to write a test script.
> 
> No problem. This is just a bug caused by good refactoring.
> 
> I found this issue in http://news.google.co.kr/nwshp?hl=ko&tab=wn.
> 
> Unfortunately, Efl port doesn't have pixel-tests yet.
> So I didn't create test case for this bug.

Ryuan, EFL DRT runs pixel-tests by default.

[Ryuan Choi]

Created attachment 124915 [details]
scrolltest.html

[Ryuan Choi]

Created attachment 124916 [details]
results

[Ryuan Choi]

(In reply to comment #8)
> (In reply to comment #7)
> > Unfortunately, Efl port doesn't have pixel-tests yet.
> > So I didn't create test case for this bug.
> 
> Ryuan, EFL DRT runs pixel-tests by default.

I didn't know that. thanks for the information.

I tried to create test case like scrolltest.html, but pixel-test just captured before scrolling.

For now, I don't have idea.

If we need test case for this bug, I'll try to find a way more.

[Zoltan Herczeg]

> For now, I don't have idea.

Check LayoutTestController.waitUntilDone and LayoutTestController.notifyDone

[Ryuan Choi]

(In reply to comment #12)
> > For now, I don't have idea.
> 
> Check LayoutTestController.waitUntilDone and LayoutTestController.notifyDone

Thank you for your kindness.
I studied them and it is exactly what I want.

However, I found many issues with DRT/Efl.

1) DRT/Efl does not test ewk_view_single as a default.
So, I'm not sure whether I should create test cases for this bug.
I want to know whether DRT/Efl will test both ewk_view_single version and ewk_view_tiled version.

gyuyoung, could you give me your opinion?

2) DRT/Efl has an option to test ewk_view_single, but it looks not working. 
I test like below, but DRT/Efl only tested ewk_view_tiled version.
# export DRT_USE_SINGLE_BACKING_STORE=1
# ./Tools/Scripts/run-webkit-tests --efl --release --pixel fast/events/scroll-after-loading.html

I'll investigate more and create bug when I found solution.
Now, I am thinking that env is not working well for script.

3) The implementation of PixelDumpSupportEfl looks wrong.
For now, PixelDumpSupportEfl paint weview instead of capturing buffer of webview.
So, ewk_view_single version of DRT/Efl can not reproduce this issue well although I fixed second issue locally.

To reproduce this, DRT/Efl should capture the current screen like DRT/Gtk doing.

I have a prototype for this issue, so I'll create bug for this after getting internal review.

Thank you.

[Ryuan Choi]

Created attachment 125513 [details]
Patch

[Ryuan Choi]

Created attachment 125516 [details]
Patch

[Ryuan Choi]

(In reply to comment #15)
> Created an attachment (id=125516) [details]
> Patch

updated patch to include test case.

[Ryuan Choi]

Created attachment 125518 [details]
rebased

[Ryuan Choi]

Comment on attachment 125516 [details]
Patch

Sorry for the mistake.

[Zoltan Herczeg]

Is the test efl specific? I think other ports should be able to run it. Perhaps we don't need a platform specific expected result at all, since we get the same results on all ports. Anyway, bot maintainers should be notified.

[Ryuan Choi]

Created attachment 125592 [details]
Patch

[Ryuan Choi]

(In reply to comment #19)
> Is the test efl specific? I think other ports should be able to run it. Perhaps we don't need a platform specific expected result at all, since we get the same results on all ports. Anyway, bot maintainers should be notified.

Thank you, I tested this on DRT/Gtk and got same result.

And add bot maintainers which mentioned in http://build.webkit.org/buildslaves

[WebKit Review Bot]

Comment on attachment 125592 [details]
Patch

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

New failing tests:
fast/events/scroll-after-loading.html

[Ryuan Choi]

Created attachment 128968 [details]
Patch

[Ryuan Choi]

(In reply to comment #22)
> (From update of attachment 125592 [details])
> Attachment 125592 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/11434229
> 
> New failing tests:
> fast/events/scroll-after-loading.html

I checked chromium-linux and realized that result is different.

In case of WebKit/Gtk, scrollbar area is rendered as empty.
but WebKit/chromium seems to fill it as black.

I am not sure which one is correct.
Now, I just added chromium specific result on LayoutTest/platform/chromium-linu.

Please let me know whether I am wrong.

[Hajime Morrita]

Comment on attachment 128968 [details]
Patch

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

I changed the outermost scrollbar painting behavior at http://trac.webkit.org/changeset/109483,
which will have invalidated the current expectation image. Could you update them?

> LayoutTests/fast/events/scroll-after-loading.html:1
> +<head>

You can use layoutTestController.dumpAsText(true) to suppress dumping render tree while keeping pixels being dumped.

[Ryuan Choi]

Created attachment 132199 [details]
Patch

[Ryuan Choi]

(In reply to comment #25)
> (From update of attachment 128968 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=128968&action=review
> 
> I changed the outermost scrollbar painting behavior at http://trac.webkit.org/changeset/109483,
> which will have invalidated the current expectation image. Could you update them?

Wow, great.
I updated patch.

> 
> > LayoutTests/fast/events/scroll-after-loading.html:1
> > +<head>
> 
> You can use layoutTestController.dumpAsText(true) to suppress dumping render tree while keeping pixels being dumped.

Thanks, I fixed like you mentioned.

[Ryuan Choi]

Created attachment 132221 [details]
Patch

[Ryuan Choi]

*** Bug 85375 has been marked as a duplicate of this bug. ***

[Gyuyoung Kim]

Comment on attachment 132221 [details]
Patch

Ryuan, I'm wondering if this patch is still valid.

[Ryuan Choi]

(In reply to comment #30)
> (From update of attachment 132221 [details])
> Ryuan, I'm wondering if this patch is still valid.

Bug and patch is valid, but new test case is unnecessary because it is covered by existing test cases such as scale-and-scroll-window.html and DRT/Efl can not test this issue well as I mentioned before.

In order to reproduce this in LayoutTest,
we should capture the pixels of webview instead of painting webview.
But it is not easy now because EFL does not have easy way to capture x backend.

I will rebase the patch without adding test case.

[Ryuan Choi]

Created attachment 175908 [details]
Patch

[Ryuan Choi]

Created attachment 176664 [details]
Patch

[WebKit Review Bot]

Comment on attachment 176664 [details]
Patch

Clearing flags on attachment: 176664

Committed r136108: <http://trac.webkit.org/changeset/136108>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Mariusz Grzegorczyk]

*** Bug 85375 has been marked as a duplicate of this bug. ***