[webkit-reviews] review denied: [Bug 56981] CSSStyleSheet#insertRule doesn't work well with imported stylesheets : [Attachment 94996] Updated test case - should reduce the flakiness & solve Simon's comments

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 56981: CSSStyleSheet#insertRule doesn't work well with imported stylesheets
https://bugs.webkit.org/show_bug.cgi?id=56981

Attachment 94996: Updated test case - should reduce the flakiness & solve
Simon's comments
https://bugs.webkit.org/attachment.cgi?id=94996&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=94996&action=review

> LayoutTests/fast/css/import-and-insert-rule-no-update.html:39
> +	   var testArea = document.getElementById("testArea");
> +	   if (getComputedStyle(testArea).backgroundColor == "rgb(0, 128, 0)")
{
> +	       testArea.innerHTML = 'PASS';
> +	       remainingTests = 0;
> +	   } else {
> +	       if (--remainingTests)
> +		   testArea.innerHTML = 'FAIL, backgroundColor was ' +
getComputedStyle(testArea).backgroundColor;
> +	   }
> +    } catch (e) {
> +	   testArea.innerHTML = 'FAIL, exception raised (' + e.message + ')';
> +	   remainingTests = 0;
> +    }
> +    if (!remainingTests)
> +	   window.setTimeout(test, 25);
> +    if (window.layoutTestController)
> +	   layoutTestController.notifyDone();
> +}
> +
> +window.onload = function() {
> +    document.styleSheets[0].insertRule('@import "green.css";', 1);
> +    // We need to wait some time to let the stylesheet load before testing.
> +    window.setTimeout(test, 25);

This is a confusing way to wait for the stylesheet. I'd much prefer to
centralize the wait logic in one function, which should call the "doTest"
function when it detects that the load is complete.

Do you expect exceptions to fire here? We don't normally test for them.