Bug 173258 - [GTK] fast/scrolling/scrolling-tree-includes-frame.html expected to fail
Summary: [GTK] fast/scrolling/scrolling-tree-includes-frame.html expected to fail
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Charlie Turner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-12 05:05 PDT by Charlie Turner
Modified: 2017-06-12 20:10 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.67 KB, patch)
2017-06-12 05:30 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch (2.07 KB, patch)
2017-06-12 08:39 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch (1.48 KB, patch)
2017-06-12 11:28 PDT, Charlie Turner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Charlie Turner 2017-06-12 05:05:43 PDT
fast/scrolling/scrolling-tree-includes-frame.html will only pass when PLATFORM(ios) is defined from what I can tell.

Mark as expected failure in GTK.
Comment 1 Charlie Turner 2017-06-12 05:30:37 PDT
Created attachment 312655 [details]
Patch
Comment 2 Michael Catanzaro 2017-06-12 05:48:31 PDT
Comment on attachment 312655 [details]
Patch

Thanks. First off, notice that you put it in the section of the TestExpectations file for passing tests (section 10), it should instead go in the section for expected failures (section 2).

If this test can only pass on iOS, then it should be marked as an expected failure in the global TestExpectations file, and marked as a Pass in the iOS expectations file. In that case, it should already be marked as Failure in the Mac expectations, so you'll have to remove it from there too.

Lastly, don't include a link to a bug report, since that's to be used for actual problems with reports that are to be left open until resolved, but if in this case it's expected that the test fails and we don't ever want to make it pass, that's not appropriate. Just leave a one-line comment explaining why the test is expected to fail.
Comment 3 Charlie Turner 2017-06-12 06:52:19 PDT
Oops, thanks for pointing that out. It looks like it passes on Sierra too. I will investigate a bit more deeply to find out if it should be disabled in global expectations file. I don't have Mac to test on unfortunately. https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#tests=fast%2Fscrolling%2Fscrolling-tree-includes-frame.html
Comment 4 Charlie Turner 2017-06-12 08:17:40 PDT
I got some extra clarification on this. It seems like GTK does not support the scrolling tree so this test isn't relevant to our port, or WK1. There's an empty text reference for WK1 (https://trac.webkit.org/browser/webkit/trunk/LayoutTests/platform/mac-wk1/fast/scrolling/scrolling-tree-includes-frame-expected.txt?rev=217730) should we do the same for GTK, or mark it as expected fail?
Comment 5 Carlos Garcia Campos 2017-06-12 08:24:42 PDT
I think we should skip it.
Comment 6 Charlie Turner 2017-06-12 08:39:18 PDT
Created attachment 312665 [details]
Patch
Comment 7 Michael Catanzaro 2017-06-12 11:03:46 PDT
Comment on attachment 312665 [details]
Patch

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

OK except for the debugging code

> Source/WebCore/html/HTMLMediaElement.cpp:1183
> +    CRASH();

Remove this debugging code
Comment 8 Charlie Turner 2017-06-12 11:28:42 PDT
Created attachment 312679 [details]
Patch
Comment 9 Charlie Turner 2017-06-12 11:29:24 PDT
Sorry about that Michael :( -- dirtied my tree by mistake.
Comment 10 WebKit Commit Bot 2017-06-12 20:10:27 PDT
Comment on attachment 312679 [details]
Patch

Clearing flags on attachment: 312679

Committed r218162: <http://trac.webkit.org/changeset/218162>
Comment 11 WebKit Commit Bot 2017-06-12 20:10:29 PDT
All reviewed patches have been landed.  Closing bug.