Bug 84072 - Layout Test platform/chromium/compositing/filters/background-filter-blur-outsets.html is failing
Summary: Layout Test platform/chromium/compositing/filters/background-filter-blur-outs...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dana Jansens
URL:
Keywords:
: 83886 (view as bug list)
Depends on:
Blocks: 84654
  Show dependency treegraph
 
Reported: 2012-04-16 13:52 PDT by Vincent Scheib
Modified: 2012-04-23 21:27 PDT (History)
7 users (show)

See Also:


Attachments
Patch (12.14 KB, patch)
2012-04-20 17:06 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (12.02 KB, patch)
2012-04-23 16:29 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (13.03 KB, patch)
2012-04-23 17:00 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vincent Scheib 2012-04-16 13:52:48 PDT
The following layout test is failing on Chromium Linux
(And is slow on other platforms...)

platform/chromium/compositing/filters/background-filter-blur-outsets.html
See:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=platform%2Fchromium%2Fcompositing%2Ffilters%2Fbackground-filter-blur-outsets.html

It has been doing so since it's introduction in
https://bugs.webkit.org/show_bug.cgi?id=80046
  [chromium] Background filters for composited layers

I'm marking it as failing in test_expectations.
Comment 1 Dana Jansens 2012-04-16 13:59:48 PDT
Er, super weird that this test fails and others don't. Go ahead and I'll see if I can find the difference.
Comment 2 James Robinson 2012-04-19 11:28:49 PDT
This is a flaky test:

13	  function setBlur() {
14	    var blurNode = window.document.getElementById('blur');
15	    window.internals.setBackgroundBlurOnNode(blurNode, 10);
16	    layoutTestController.notifyDone();
17	  }
18	
19	  if (window.layoutTestController) {
20	    if (window.internals) {
21	      window.setTimeout(setBlur, 0);
22	      layoutTestController.waitUntilDone();
23	    }
24	    layoutTestController.dumpAsText(true);
25	  }

it's setting a timeout before the <body> and relying the <div id="blur" being parsed when the timer runs. this isn't guaranteed - the timer might fire before we parse the rest of the page.  we should run the test code in the load event.
Comment 3 Dana Jansens 2012-04-19 11:34:36 PDT
Ooh, thanks.
Comment 4 Dana Jansens 2012-04-20 16:43:19 PDT
window.onload = setBlur; just ends up timing out, and I'm not sure why.
window.onload = function() { setTimeout(setBlur, 0); } works locally.

Is that what you were thinking then James?
Comment 5 Dana Jansens 2012-04-20 17:06:06 PDT
Created attachment 138205 [details]
Patch

layoutTestController.display() ftw! Thanks to enne.
Comment 6 Adrienne Walker 2012-04-23 16:07:16 PDT
Comment on attachment 138205 [details]
Patch

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

> LayoutTests/platform/chromium/compositing/filters/background-filter-blur-off-axis.html:29
> +    layoutTestController.waitUntilDone();

I don't think you need waitUntilDone/notifyDone anymore if you're doing everything in the onload handler.
Comment 7 Dana Jansens 2012-04-23 16:29:09 PDT
Created attachment 138454 [details]
Patch

right-o
Comment 8 Adrienne Walker 2012-04-23 16:53:45 PDT
Comment on attachment 138454 [details]
Patch

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

> LayoutTests/platform/chromium/compositing/filters/background-filter-blur-outsets.html:16
> +    window.internals.setBackgroundBlurOnNode(blurNode, 5);

Is there a reason to change the blur size on this test? Do you also need to add other platform-specific results?
Comment 9 Dana Jansens 2012-04-23 16:56:14 PDT
Comment on attachment 138454 [details]
Patch

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

> LayoutTests/ChangeLog:11
> +        Also, smaller blur for faster test bots.

The test was pretty slow on the bots, slow enough to cause race conditions. It seems a smaller blur radius doesn't change the effectiveness but should speed it up some.
Comment 10 Dana Jansens 2012-04-23 16:57:31 PDT
(In reply to comment #8)
> (From update of attachment 138454 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138454&action=review
> 
> > LayoutTests/platform/chromium/compositing/filters/background-filter-blur-outsets.html:16
> > +    window.internals.setBackgroundBlurOnNode(blurNode, 5);
> 
> Is there a reason to change the blur size on this test? Do you also need to add other platform-specific results?

Oh, and I spose I should disable it on !LINUX. Thanks.
Comment 11 Dana Jansens 2012-04-23 16:59:55 PDT
*** Bug 83886 has been marked as a duplicate of this bug. ***
Comment 12 Dana Jansens 2012-04-23 17:00:20 PDT
Created attachment 138462 [details]
Patch
Comment 13 Adrienne Walker 2012-04-23 17:09:17 PDT
Comment on attachment 138462 [details]
Patch

Ah, changing the blur to make the test faster makes a lot of sense.
Comment 14 WebKit Review Bot 2012-04-23 21:27:44 PDT
Comment on attachment 138462 [details]
Patch

Clearing flags on attachment: 138462

Committed r114995: <http://trac.webkit.org/changeset/114995>
Comment 15 WebKit Review Bot 2012-04-23 21:27:49 PDT
All reviewed patches have been landed.  Closing bug.