Bug 148507

Summary: Add a Layout test for r188991
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: thorton
Priority: P2    
Version: WebKit Nightly Build   
Hardware: iPhone / iPad   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
thorton: review+
Patch none

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>