Per bug 896050 comment 44, we're hitting assertions in test_smilMappedAttrFromTo.xhtml with that bug's patch applied. Sample log: https://tbpl.mozilla.org/php/getParsedLog.php?id=27235958&full=1&branch=try#error0 The backtrace is to the ToString() invocation added in bug 904158. I probably need to tweak something from that patch.

Ahhh, this is from another nsCheapString usage: > 97 if (oldValStrBuf && valStr.Equals(nsCheapString(oldValStrBuf))) { http://mxr.mozilla.org/mozilla-central/source/content/smil/nsSMILMappedAttribute.cpp#97 We need to use the exact same NS_strlen / animValBuf->ToString() stuff from bug 904158, in place of nsCheapString.

(In reply to Daniel Holbert [:dholbert] from comment #0) > The backtrace is to the ToString() invocation added in bug 904158. (That^^ was wrong, BTW: the backtrace is actually to the ToString() invocation in the nsCheapString constructor quoted in comment 1 here.)

This patch just factors out bug 904158's code* into a nsContentUtils helper-method, and then calls that helper-method in one additional place. Reftest included, using "hg cp" from bug 904158's reftest, to change <set> to <animate>. The test fails an assertion before the patch; it passes after. *(bug 904158's logic = the code to convert a nsStringBuffer to a nsString, using NS_strlen to determine the length, and doing an allocated-length sanity-check to make sure the resulting string is in-bounds for the nsStringBuffer.)

(forgot to qref in some minor cleanup. fixed now.)

This version fixes one bug I just noticed in the original patches here. My old patches made PopulateStringFromStringBuffer() handle null buffers gracefully by returning an empty string. However, we don't actually want that behavior. Null is a special case in nsSMILMappedAttribute::SetAnimValue() -- it means we have *no* old animated value. In that case, we definitely don't want to take the early-return. (With my old patch here, if the old value were null and the new value were the empty string, we would take the early return; but we shouldn't.)

Comment on attachment 799107 [details] [diff] [review] fix v1b r=dbaron

Comment on attachment 799107 [details] [diff] [review] fix v1b Review of attachment 799107 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/public/nsContentUtils.h @@ +1715,5 @@ > + * Populates aResultString with the contents of the string-buffer aBuf, up > + * to aBuf's null-terminator. aBuf must not be null. Ownership of the string > + * is not transferred. > + */ > + static void PopulateStringFromStringBuffer(nsStringBuffer* aBuf, Why not make this a member function on nsStringBuffer?

(In reply to :Ms2ger (away 11-21 September) from comment #7) > Why not make this a member function on nsStringBuffer? StringBuffer is an old and "lean" API; I didn't want to just add a new method to it (built on top of its already-exposed public API, so with no need to have access to anything internal) just because I need it in one spot. For now, I think this makes enough sense as a "utils" method. If it makes a difference, we can maybe move it into nsStringBuffer in a future bug.