604147 - nsSMILTimedElement::GetNextInterval should return PRBool instead of nsresult

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

nsSMILTimedElement::GetNextInterval() returns nsresult right now, but:
 (a) there are only two states it returns -- NS_OK or NS_ERROR_FAILURE
 (b) the failure case isn't particularly "fail"-ish -- it's just "there's no next interval"

Consequently, I think we should simplify it to just return PRBool. (PR_TRUE on success, PR_FALSE on failure)

birtles, any objections?

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

This patch
 - makes the nsresult --> PRBool fix
 - adds "const" to the static constant variable "zeroTime", for clarity
 - for two of the modified return statements: adds curly braces for clarity, since these early-returns were very close to a different clause's opening or closing brace (which makes the if-nesting harder to see).

Comment 2

[Brian Birtles (:birtles)]

Comment on attachment 482962 [details] [diff] [review]
fix

Looks great.

Comment 3

[Daniel Holbert [:dholbert]]

Comment on attachment 482962 [details] [diff] [review]
fix

Requesting approval to land.

Low-risk -- no functional change, just code simplification. (Replacing an unnecessary nsresult return value with PRBool for success/failure.)

Comment 4

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/b0cbb2028bbf

Comment 5

[Daniel Holbert [:dholbert]]

Thanks, Ehsan!