668296 - Share code between nsISMILType::Destroy specializations

Status: NEW

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Daniel Holbert [:dholbert]]

(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.)

Comment 2

[Daniel Holbert [:dholbert]]

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);")