Quick fix for windows try bots flakiness.

Quick fix for windows try bots flakiness.

It was a race between animation scheduled by WebTestProxyBase and ResourceLoader which received the test script content.

In case of failure I see the next sequence of top level events:
1) RootGraphicsLayer was cleared
2) html resource arrived and animation was scheduled
3) js resource arrived, onload event handler was called and the test finished in the ASSERT

The expected sequence of top level events:
1) RootGraphicsLayer was cleared
2) html resource arrived and animation was scheduled
3) scheduled animation was executed
4) js resource arrived, onload event handler was called and the test finished successfully

The simplest solution is to run pending animations.

BUG=397321
Committed: https://crrev.com/0ef3f039c5d5101cc950a88244d244ac7c4a574e
Cr-Commit-Position: refs/heads/master@{#295938}

[loislo, 2014-09-18 09:39:16]

loislo@chromium.org changed reviewers:
+ danakj@chromium.org, dcheng@chromium.org, dpranke@google.com,
enne@chromium.org

[loislo, 2014-09-18 09:59:13]

loislo@chromium.org changed reviewers:
+ abarth@chromium.org, dpranke@chromium.org, pfeldman@chromium.org
- danakj@chromium.org, dcheng@chromium.org, dpranke@google.com,
enne@chromium.org

[loislo, 2014-09-18 10:04:32]

ptal

[loislo, 2014-09-18 10:11:25]

loislo@chromium.org changed reviewers:
+ danakj@chromium.org

[Dirk Pranke, 2014-09-18 15:27:59]

dpranke@chromium.org changed reviewers:
+ jamesr@chromium.org

[Dirk Pranke, 2014-09-18 15:28:00]

I don't really understand this change, but I'm keen to see if anything improves
the win bot flakiness, so I'll give this a rubber-stamp LGTM.

Adding +jamesr, who hopefully might be another person who would actually be able
to review the change.

[danakj, 2014-09-18 15:33:41]

https://codereview.chromium.org/581983002/diff/20001/content/shell/renderer/t...
File content/shell/renderer/test_runner/web_test_proxy.cc (right):

https://codereview.chromium.org/581983002/diff/20001/content/shell/renderer/t...
content/shell/renderer/test_runner/web_test_proxy.cc:488: AnimateNow();
I think we need to fix WebViewImpl/blink, not just the test harness.

[loislo, 2014-09-18 18:37:21]

On 2014/09/18 15:33:41, danakj wrote:
>
https://codereview.chromium.org/581983002/diff/20001/content/shell/renderer/t...
> File content/shell/renderer/test_runner/web_test_proxy.cc (right):
> 
>
https://codereview.chromium.org/581983002/diff/20001/content/shell/renderer/t...
> content/shell/renderer/test_runner/web_test_proxy.cc:488: AnimateNow();
> I think we need to fix WebViewImpl/blink, not just the test harness.

We scheduled animation from testRunner code and when another asynchronous event
happened we called another testRunner function which expected that the layer had
been created. 
So it looks like a testRunner problem. Probably we have another problem in
WebViewImpl code but I think we could fix the test harness and fix the rest in a
separate patch because it is too late to revert the original change :)

[loislo, 2014-09-22 08:02:16]

The CQ bit was checked by loislo@chromium.org

[commit-bot: I haz the power, 2014-09-22 08:02:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/581983002/20001

[loislo, 2014-09-22 08:15:45]

On 2014/09/18 18:37:21, loislo wrote:
> On 2014/09/18 15:33:41, danakj wrote:
> >
>
https://codereview.chromium.org/581983002/diff/20001/content/shell/renderer/t...
> > File content/shell/renderer/test_runner/web_test_proxy.cc (right):
> > 
> >
>
https://codereview.chromium.org/581983002/diff/20001/content/shell/renderer/t...
> > content/shell/renderer/test_runner/web_test_proxy.cc:488: AnimateNow();
> > I think we need to fix WebViewImpl/blink, not just the test harness.
> 
> We scheduled animation from testRunner code and when another asynchronous
event
> happened we called another testRunner function which expected that the layer
had
> been created. 
> So it looks like a testRunner problem. Probably we have another problem in
> WebViewImpl code but I think we could fix the test harness and fix the rest in
a
> separate patch because it is too late to revert the original change :)

I made a test with approach suggested by danakj@ and found that it doesn't fix
the race.
So I decided to land the patch. Feel free to roll it back if there is a better
solution.

[commit-bot: I haz the power, 2014-09-22 08:33:25]

Committed patchset #2 (id:20001) as 80f9e7f20887c46212966acc302a78a30aead99d

[commit-bot: I haz the power, 2014-09-22 08:34:15]

Patchset 2 (id:??) landed as
https://crrev.com/0ef3f039c5d5101cc950a88244d244ac7c4a574e
Cr-Commit-Position: refs/heads/master@{#295938}

[jamesr, 2014-09-22 14:40:02]

This is a bad change.  Blink is doing the wrong thing here, not the test runner.
 As soon as the main resources arrive Blink should be setting a new root
graphics layer up.  If any animations are pending, they should run when the
compositor says to run them, not at arbitrary other times.  Copying image data
out in a layout test should not have script-visible side effects.