931169 - waitForCondition should guard the condition evaluation

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

I recently ran in to a case where evaluation the condition function threw an exception. This short-circutied waitForCondition, causing |tries| to never get incremented and the test got stuck in an infinite loop.

Placing a try/catch around the condition call fixes this.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Comment 2

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 822510 [details] [diff] [review]
Patch

Review of attachment 822510 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like a good idea to me. On an unrelated note, it's a shame this function got duplicated into multiple files instead of somehow sharing it :/
And, actually, what's going on with these?
https://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/general/browser_bug822367.js#183
https://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/general/browser_bug902156.js#48
Can they not use the definition in browser/base/content/test/general/head.js?
If you don't feel like investigating and possibly taking out the unnecessary duplications, maybe file a bug on dealing with this duplication.
Thanks!

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Those can safely be deleted, I missed them because I only searched within files named head.js. Thanks for catching those!

https://hg.mozilla.org/integration/fx-team/rev/954741a373f3

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/954741a373f3