Bug 67538

Summary: Layout Test http/tests/security/xssAuditor/dom-write-innerHTML.html is flaky
Product: WebKit Reporter: James Robinson <jamesr>
Component: Tools / TestsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, dbates, eric, haraken, webkit.review.bot
Priority: P2 Keywords: XSSAuditor
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description James Robinson 2011-09-02 16:32:46 PDT
The following layout test is flaky on all platforms in debug

http/tests/security/xssAuditor/dom-write-innerHTML.html

Probable cause:

I think the setTimeout() is failing the race against the img onerror handler, although I haven't looked too closely.

Example failure diff:

ALERT: /XSS/
Comment 1 Alexey Proskuryakov 2011-09-02 22:06:12 PDT
This test is pretty old, was it always flaky?

Seems unlikely that notifyDone() can trigger a failure alert, could be an actual bug.
Comment 2 Adam Barth 2012-02-05 22:09:53 PST
Created attachment 125571 [details]
Patch
Comment 3 Kentaro Hara 2012-02-05 22:20:01 PST
Comment on attachment 125571 [details]
Patch

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

> LayoutTests/http/tests/security/xssAuditor/resources/echo-dom-write-innerHTML.html:9
> -</html>
>  \ No newline at end of file
> +</html>

What is the change for?
Comment 4 Adam Barth 2012-02-05 22:21:27 PST
Comment on attachment 125571 [details]
Patch

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

>> LayoutTests/http/tests/security/xssAuditor/resources/echo-dom-write-innerHTML.html:9
>> +</html>
> 
> What is the change for?

vi just adds newlines at the end of the file.  It's not needed to fix the bug.  I can remove the newline again if you'd prefer.
Comment 5 Kentaro Hara 2012-02-05 22:26:28 PST
Comment on attachment 125571 [details]
Patch

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

>>> LayoutTests/http/tests/security/xssAuditor/resources/echo-dom-write-innerHTML.html:9
>>> +</html>
>> 
>> What is the change for?
> 
> vi just adds newlines at the end of the file.  It's not needed to fix the bug.  I can remove the newline again if you'd prefer.

Then, maybe we can remove the "\ No newline at end of file" line (because now the comment has no meaning).
Comment 6 Adam Barth 2012-02-05 22:33:57 PST
> Then, maybe we can remove the "\ No newline at end of file" line (because now the comment has no meaning).

Oh, that's just added by the diff tool.  It's not something that ends up in the repository.
Comment 7 Kentaro Hara 2012-02-05 22:35:25 PST
(In reply to comment #6)
> > Then, maybe we can remove the "\ No newline at end of file" line (because now the comment has no meaning).
> 
> Oh, that's just added by the diff tool.  It's not something that ends up in the repository.

Ah, I got it:-)
Comment 8 WebKit Review Bot 2012-02-06 01:22:44 PST
Comment on attachment 125571 [details]
Patch

Clearing flags on attachment: 125571

Committed r106785: <http://trac.webkit.org/changeset/106785>
Comment 9 WebKit Review Bot 2012-02-06 01:22:49 PST
All reviewed patches have been landed.  Closing bug.