Closed Bug 931169 Opened 6 years ago Closed 6 years ago

waitForCondition should guard the condition evaluation

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 27

People

(Reporter: jaws, Assigned: jaws)

Details

Attachments

(1 file)

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.
Attached patch PatchSplinter Review
Attachment #822510 - Flags: review?(dkeeler)
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!
Attachment #822510 - Flags: review?(dkeeler) → review+
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
https://hg.mozilla.org/mozilla-central/rev/954741a373f3
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
You need to log in before you can comment on or make changes to this bug.