Open Bug 668296 Opened 13 years ago Updated 1 year ago

Share code between nsISMILType::Destroy specializations


(Core :: SVG, defect)





(Reporter: dholbert, Unassigned)


Right now, all of our nsISMILType::Destroy impls do something like:
>  aValue.mU.whateverTypeImUsing = someZeroishValue; // nsnull or 0.0 or 0
>  aValue.mType = &nsSMILNullType::sSingleton;

Each class does this in its own way (using its particular subcomponent(s) of mU), but that's kind of silly -- it's one extra piece of boilerplate that has to be copied (with manual tweaking) into each new SMIL class & tweaked.

The goal of all this is just to zero out mU (simply as a foolproofing technique, in case we accidentally re-use a nsSMILValue after destroying it).  There's no reason that we need to do this in a class-specific way. We should just stick this code in an inherited nsISMILType::Destroy method -- this would share code better, and it'd also remove the need for any Destroy impl at all in many of our nsISMILType classes.
(I started to write up this patch, but it's not as dead-simple-straightforward as I initially thought, because nsISMILType is only a .h file, and it can't touch nsSMILValue nor nsSMILNullType::sSingleton, since the files that define those things #include nsISMILType themselves.  Holding off on this for now; feel free to take this & fix it.)
But basically, the idea here is to have something like this (possibly in a new nsISMILType.cpp file, if that's not too icky):
>  /* virtual */
>  void nsISMILType::Destroy(nsSMILValue& aValue) const {
>    NS_PRECONDITION(aValue.mType == this, "Unexpected SMIL value");
>    memset(&aValue.mU, 0, sizeof(aValue.mU));
>    aValue.mType = &nsSMILNullType::sSingleton;
>  }

and then get rid of all the ::Destroy() impls
(except for the ones that delete mU.mPtr -- those would need to remain, but their boilerplate code could still be replaced with a call to "nsISMILType::Destroy(aValue);")
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.