272899 - cleanup svg base type creation

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[tor]

Comment 1

[tor]

Attachment: cleanup

Comment 2

[Jonathan Watt [:jwatt]]

Comment on attachment 167723 [details] [diff] [review]
cleanup

Is it okay to initialise mRefCnt to 1? I seem to recall objections being raised
to that somewhere.

I would rather that 'result' be the last parameter in NS_NewSVGXxx to be
consistent, but I didn't really look to see if we need the default values for
the other parameters. Not a big deal I guess.

In nsSVGMatrix::Translate return the error code returned by NS_NewSVGMatrix.

While you're changing that part of nsSVGPointList.cpp, I'd rather we have:

    rv = NS_NewSVGPoint(getter_AddRefs(point), (float)x, (float)y);
    if (!point)
      break;

In nsSVGSVGElement.cpp you could use the fact that NS_NewSVGPoint has default
values for the parameters instead of assigning them 0.0f anyway.

In nsSVGForeignObjectFrame.cpp can you put an:

  if (!point)
    return;

after the NS_NewSVGPoint call please.

In nsSVGOuterSVGFrame.cpp perhaps you should check if an error occured during
the NS_NewSVGRect call. Not sure what you would do if it did.

In nsSVGTSpanFrame.cpp please return the rv from NS_NewSVGRect.

In nsSVGTextFrame.cpp please return the rv from NS_NewSVGRect.

Comment 3

[tor]

(In reply to comment #2)
> While you're changing that part of nsSVGPointList.cpp, I'd rather we have:
> 
>     rv = NS_NewSVGPoint(getter_AddRefs(point), (float)x, (float)y);
>     if (!point)
>       break;

That would allow a partial list to be returned if we ran out of memory.
I'd rather return a failure.

> In nsSVGOuterSVGFrame.cpp perhaps you should check if an error occured during
> the NS_NewSVGRect call. Not sure what you would do if it did.

SetCoordCtxRect handles a null parameter; just using that behavior probably
isn't any better/worse than returning at that point.

Checked in.