Bug 104139 - [EFL] [WK2] fast/dom/vertical-scrollbar-in-rtl.html makes fast/regions tests flaky
Summary: [EFL] [WK2] fast/dom/vertical-scrollbar-in-rtl.html makes fast/regions tests ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 103641 (view as bug list)
Depends on: 104251 137138
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-05 10:42 PST by Yael
Modified: 2014-11-03 08:04 PST (History)
3 users (show)

See Also:


Attachments
issue (22.85 KB, image/png)
2012-12-05 10:44 PST, Yael
no flags Details
Patch (1.47 KB, patch)
2012-12-05 10:48 PST, Yael
no flags Details | Formatted Diff | Diff
Patch (1.50 KB, patch)
2012-12-06 06:05 PST, Yael
no flags Details | Formatted Diff | Diff
Patch (1.57 KB, patch)
2012-12-06 09:00 PST, Yael
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yael 2012-12-05 10:42:26 PST
The test fast/dom/vertical-scrollbar-in-rtl.html is calling window.resizeTo, which resizes the window and the viewport. When the test ends, the viewport size is 500x500. We do not resize the window back to 800x600, so our gl viewport stays smaller, and that leads to the garbage we occasionally see in fast/regions tests. These tests fail if they run on the same worker as fast/dom/vertical-scrollbar-in-rtl.html . 
The attached image shows the problem.

A patch is coming.
Comment 1 Yael 2012-12-05 10:44:14 PST
Created attachment 177793 [details]
issue
Comment 2 Yael 2012-12-05 10:45:18 PST
The attachment needs to be downloaded first, in order to see the issue.
Comment 3 Yael 2012-12-05 10:48:55 PST
Created attachment 177794 [details]
Patch

The attached patch does not solve all the flakiness we currently have in EFL bot, but it helps fixing the issue described in this bug.
Comment 4 Kenneth Rohde Christiansen 2012-12-05 16:53:15 PST
Comment on attachment 177794 [details]
Patch

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

> Tools/WebKitTestRunner/TestController.cpp:568
> +    // Resize the window to original size. This is needed after calls to window.resizeTo.
> +    WKRect rect = m_mainWebView->windowFrame();

No test modifies location?
Comment 5 Yael 2012-12-05 17:02:16 PST
(In reply to comment #4)
> (From update of attachment 177794 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177794&action=review
> 
> > Tools/WebKitTestRunner/TestController.cpp:568
> > +    // Resize the window to original size. This is needed after calls to window.resizeTo.
> > +    WKRect rect = m_mainWebView->windowFrame();
> 
> No test modifies location?
Thank you for the review. As I mentioned above, there are more flaky tests to fix. If tests that move windows are causing flakiness, i will address that too.
Comment 6 WebKit Review Bot 2012-12-06 02:09:23 PST
Comment on attachment 177794 [details]
Patch

Clearing flags on attachment: 177794

Committed r136825: <http://trac.webkit.org/changeset/136825>
Comment 7 WebKit Review Bot 2012-12-06 02:09:27 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 WebKit Review Bot 2012-12-06 04:07:38 PST
Re-opened since this is blocked by bug 104251
Comment 9 Csaba Osztrogonác 2012-12-06 04:46:18 PST
(In reply to comment #8)
> Re-opened since this is blocked by bug 104251

Rolled out by http://trac.webkit.org/changeset/136830, because it made 100+ test fail on Qt-WK2 pixel bot, and 1800+ test flakey on Qt-WK2 non-pixel bot.

x86-64 Linux Qt Release WebKit2 (Amazon EC2) - non-pixel tester bot:
- after: http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Release%20WebKit2%20%28Amazon%20EC2%29/builds/11517
-  before: http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Release%20WebKit2%20%28Amazon%20EC2%29/builds/11516

x86-64 Linux Qt Release WebKit2 (Pixel Tests):
- after: http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Release%20WebKit2%20%28Pixel%20Tests%29/builds/737
- before: http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Release%20WebKit2%20%28Pixel%20Tests%29/builds/736
Comment 10 Yael 2012-12-06 06:05:49 PST
Created attachment 178003 [details]
Patch

Sorry for the regression.
This is the same patch, but the resize is only for EFL port. We definitely need that for EFL port.
Comment 11 Kenneth Rohde Christiansen 2012-12-06 06:30:09 PST
Comment on attachment 178003 [details]
Patch

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

> Tools/WebKitTestRunner/TestController.cpp:573
>  
> +#if PLATFORM(EFL)
> +    // Resize the window to original size. This is needed after calls to window.resizeTo.
> +    WKRect rect = m_mainWebView->windowFrame();
> +    m_mainWebView->setWindowFrame(WKRectMake(rect.origin.x, rect.origin.y, 800, 600));
> +#endif
> +
>      // Reset notification permissions

Could you explain why that is not needed for other platforms?
Comment 12 Yael 2012-12-06 06:43:21 PST
(In reply to comment #11)
> (From update of attachment 178003 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178003&action=review
> 
> > Tools/WebKitTestRunner/TestController.cpp:573
> >  
> > +#if PLATFORM(EFL)
> > +    // Resize the window to original size. This is needed after calls to window.resizeTo.
> > +    WKRect rect = m_mainWebView->windowFrame();
> > +    m_mainWebView->setWindowFrame(WKRectMake(rect.origin.x, rect.origin.y, 800, 600));
> > +#endif
> > +
> >      // Reset notification permissions
> 
> Could you explain why that is not needed for other platforms?
I don't know how other platforms deal with this, but I do know that we set the gl viewport to the size of the window, and if the window is not in the correct size, our gl viewport does not cover the screen properly.
You can see clearly in the attached image that the top and left parts are left clear/transparent, because they are clipped.
Comment 13 Yael 2012-12-06 06:46:04 PST
(In reply to comment #12)
> (In reply to comment #11)
> > (From update of attachment 178003 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=178003&action=review
> > 
> > > Tools/WebKitTestRunner/TestController.cpp:573
> > >  
> > > +#if PLATFORM(EFL)
> > > +    // Resize the window to original size. This is needed after calls to window.resizeTo.
> > > +    WKRect rect = m_mainWebView->windowFrame();
> > > +    m_mainWebView->setWindowFrame(WKRectMake(rect.origin.x, rect.origin.y, 800, 600));
> > > +#endif
> > > +
> > >      // Reset notification permissions
> > 
> > Could you explain why that is not needed for other platforms?
> I don't know how other platforms deal with this, but I do know that we set the gl viewport to the size of the window, and if the window is not in the correct size, our gl viewport does not cover the screen properly.
> You can see clearly in the attached image that the top and left parts are left clear/transparent, because they are clipped.
I meant top and right, of course :)
Comment 14 Yael 2012-12-06 08:43:25 PST
The difference between EFL and other platforms is that other platforms do not use a real window, but EFL does.
Comment 15 Yael 2012-12-06 09:00:20 PST
Created attachment 178019 [details]
Patch

Update the comment about why the changes is needed for EFL and not for other ports.
Comment 16 WebKit Review Bot 2012-12-06 09:44:28 PST
Comment on attachment 178019 [details]
Patch

Clearing flags on attachment: 178019

Committed r136848: <http://trac.webkit.org/changeset/136848>
Comment 17 WebKit Review Bot 2012-12-06 09:44:32 PST
All reviewed patches have been landed.  Closing bug.
Comment 18 Yael 2012-12-07 05:36:25 PST
*** Bug 103641 has been marked as a duplicate of this bug. ***