[webkit-reviews] review denied: [Bug 59912] Automated Bug 5768 test from manual test. : [Attachment 91875] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Naoki Takano
<takano.naoki at gmail.com>'s request for review:
Bug 59912: Automated Bug 5768 test from manual test.
https://bugs.webkit.org/show_bug.cgi?id=59912

Attachment 91875: Patch
https://bugs.webkit.org/attachment.cgi?id=91875&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=91875&action=review

> LayoutTests/fast/frames/frame-with-noresize-refresh.html:12
> +if (window.layoutTestController)
> +    layoutTestController.dumpAsText();
> +
> +if (window.layoutTestController)
> +    layoutTestController.dumpChildFramesAsText();
> +
> +if (window.layoutTestController)
> +    layoutTestController.waitUntilDone();

This can all be combined into one if-block.

> LayoutTests/fast/frames/frame-with-noresize-refresh.html:14
> +alert("This test checks if multiple frame loading correctly refreshes layout
with noresize attribute in frame.");

I'd just make this a comment instead of an alert.  An alert can be annoying
when running the test manually.

> LayoutTests/fast/frames/frame-with-noresize-refresh.html:23
> +	       window.setTimeout(test, 100);

Why these set timeouts?  They look like race conditions to me.