Bug 148507 - Add a Layout test for r188991
Summary: Add a Layout test for r188991
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-26 17:54 PDT by Wenson Hsieh
Modified: 2015-08-27 07:26 PDT (History)
1 user (show)

See Also:

Attachments
Patch (4.60 KB, patch)
2015-08-26 18:14 PDT, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Patch (3.30 KB, patch)
2015-08-26 18:29 PDT, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Patch (3.20 KB, patch)
2015-08-26 19:20 PDT, Wenson Hsieh 		thorton: review+
Details | Formatted Diff | Diff
Patch (3.10 KB, patch)
2015-08-26 21:01 PDT, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2015-08-26 17:54:37 PDT 
https://bugs.webkit.org/show_bug.cgi?id=148442 should be covered by a LayoutTest on iOS simulator.
Comment 1 Wenson Hsieh 2015-08-26 18:14:30 PDT 
Created attachment 260015 [details]
Patch
Comment 2 Tim Horton 2015-08-26 18:21:42 PDT 
Comment on attachment 260015 [details]
Patch

Waaaay too many long delays in here. Also, it should run on all platforms.
Comment 3 Wenson Hsieh 2015-08-26 18:29:29 PDT 
Created attachment 260016 [details]
Patch
Comment 4 Wenson Hsieh 2015-08-26 19:20:20 PDT 
Created attachment 260020 [details]
Patch
Comment 5 Tim Horton 2015-08-26 19:23:57 PDT 
Comment on attachment 260020 [details]
Patch

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

> LayoutTests/animations/crash-on-removing-animation.html:4
> +    <meta name="viewport" content="initial-scale=1, maximum-scale=1">

I believe we force a particular viewport in layout tests, this should be unnecessary.
Comment 6 Wenson Hsieh 2015-08-26 19:24:40 PDT 
Comment on attachment 260020 [details]
Patch

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

>> LayoutTests/animations/crash-on-removing-animation.html:4
>> +    <meta name="viewport" content="initial-scale=1, maximum-scale=1">
> 
> I believe we force a particular viewport in layout tests, this should be unnecessary.

Good to know -- thanks!
Comment 7 Wenson Hsieh 2015-08-26 21:01:30 PDT 
Created attachment 260027 [details]
Patch
Comment 8 Tim Horton 2015-08-26 21:57:26 PDT 
Comment on attachment 260027 [details]
Patch

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

> LayoutTests/ChangeLog:8
> +        Tests that stopping an animation early and simultaneously closing the window

So glad that there's a test! It's not simultaneous, though :P

> LayoutTests/animations/crash-on-removing-animation.html:24
> +        if (window.testRunner == undefined)

No need for the == undefined; every other test just does "if (window.testRunner)". Also possible that you might want to document.write("This test must be run in DRT/WKTR.") or something like that so that people (QA, mostly) don't open the test and see "If you are reading this, we managed to avoid crashing!" and think that the test passed.

> LayoutTests/animations/crash-on-removing-animation.html:44
> +<body onload="setup();">

No need for the semicolon, and it looks funny, but it doesn't really matter.
Comment 9 Wenson Hsieh 2015-08-27 07:26:38 PDT 
Committed r189022: <http://trac.webkit.org/changeset/189022>