Make waitFor check predicate before logging slowness.

Make waitFor check predicate before logging slowness.

Before it was possible for an overloaded main thread to repeatedly print
the "Still waiting..." message without actually checking the predicate,
which in turn can cause tests to time out.

BUG=

Review-Url: https://codereview.chromium.org/2868933003
Cr-Commit-Position: refs/heads/master@{#470520}
Committed: https://chromium.googlesource.com/chromium/src/+/86f6afd077fb8b9aecd444fd27b51b2cfd0dece6

[Dan Elphick, 2017-05-09 14:08:26]

Description was changed from

==========
Make waitFor check predicate before logging slowness.

Previously for tsan tests, it's possible that the DOMTimer from
setInterval would not fire within 3 seconds, which caused the "Still
waiting..." message to be logged without ever checking the predicate.

BUG=
==========

to

==========
Make waitFor check predicate before logging slowness.

Previously for tsan tests, it's possible that the DOMTimer from
setInterval would not fire within 3 seconds, which caused the "Still
waiting..." message to be logged without ever checking the predicate.

BUG=
==========

[Dan Elphick, 2017-05-09 14:08:26]

delphick@chromium.org changed reviewers:
+ mcasas@chromium.org

[Dan Elphick, 2017-05-09 14:11:49]

PTAL Miguel.

This change is needed to land https://codereview.chromium.org/2810423003/ as
that prioritizes compositor tasks which in combination with tsan means the
setInterval runs with an interval of >3s which causes the
WebRtcMediaRecorderTest.RecordWithTransparency/* tests to always time out.

[Dan Elphick, 2017-05-09 14:14:58]

The CQ bit was checked by delphick@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 14:15:25]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 15:25:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-09 15:25:41]

Dry run: This issue passed the CQ dry run.

[mcasas, 2017-05-09 18:21:35]

On 2017/05/09 14:11:49, Dan Elphick wrote:
> PTAL Miguel.
> 
> This change is needed to land https://codereview.chromium.org/2810423003/ as
> that prioritizes compositor tasks which in combination with tsan means the
> setInterval runs with an interval of >3s which causes the
> WebRtcMediaRecorderTest.RecordWithTransparency/* tests to always time out.

lgtm, thanks!

(Also, the CL description says "Previously for tsan tests...",
did you mean "On ToT...", or "After CL bla lands then..." ?
Rationale: the CL description should talk about the CL
in present tense, and make explicit what is ToT and what is
the near future, etc.

[Dan Elphick, 2017-05-10 08:32:06]

On 2017/05/09 18:21:35, mcasas wrote:
> On 2017/05/09 14:11:49, Dan Elphick wrote:
> > PTAL Miguel.
> > 
> > This change is needed to land https://codereview.chromium.org/2810423003/ as
> > that prioritizes compositor tasks which in combination with tsan means the
> > setInterval runs with an interval of >3s which causes the
> > WebRtcMediaRecorderTest.RecordWithTransparency/* tests to always time out.
> 
> lgtm, thanks!
> 
> (Also, the CL description says "Previously for tsan tests...",
> did you mean "On ToT...", or "After CL bla lands then..." ?
> Rationale: the CL description should talk about the CL
> in present tense, and make explicit what is ToT and what is
> the near future, etc.

Thanks and sorry the description I wrote is horrible. It should say 'Before it
was possible for an overloaded main thread to repeatedly print the "Still
waiting..." message without actually checking the predicate.' Currently that
doesn't seem to happen but it will do when my other CL lands (although again
only for tsan tests which seem to make everything 10+ times slower).

[Dan Elphick, 2017-05-10 08:34:17]

Description was changed from

==========
Make waitFor check predicate before logging slowness.

Previously for tsan tests, it's possible that the DOMTimer from
setInterval would not fire within 3 seconds, which caused the "Still
waiting..." message to be logged without ever checking the predicate.

BUG=
==========

to

==========
Make waitFor check predicate before logging slowness.

Before it was possible for an overloaded main thread to repeatedly print
the "Still waiting..." message without actually checking the predicate,
which in turn can cause tests to time out.

BUG=
==========

[Dan Elphick, 2017-05-10 08:36:35]

The CQ bit was checked by delphick@chromium.org

[commit-bot: I haz the power, 2017-05-10 08:37:17]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 08:42:06]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1494405395286080, "parent_rev":
"80db61a2f1eaa7555d0a6bfa6e432b52f9530592", "commit_rev":
"86f6afd077fb8b9aecd444fd27b51b2cfd0dece6"}

[commit-bot: I haz the power, 2017-05-10 08:42:22]

Description was changed from

==========
Make waitFor check predicate before logging slowness.

Before it was possible for an overloaded main thread to repeatedly print
the "Still waiting..." message without actually checking the predicate,
which in turn can cause tests to time out.

BUG=
==========

to

==========
Make waitFor check predicate before logging slowness.

Before it was possible for an overloaded main thread to repeatedly print
the "Still waiting..." message without actually checking the predicate,
which in turn can cause tests to time out.

BUG=

Review-Url: https://codereview.chromium.org/2868933003
Cr-Commit-Position: refs/heads/master@{#470520}
Committed:
https://chromium.googlesource.com/chromium/src/+/86f6afd077fb8b9aecd444fd27b5...
==========

[commit-bot: I haz the power, 2017-05-10 08:42:23]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/86f6afd077fb8b9aecd444fd27b5...