265988 - prepare nsSVGDocument for sr

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

The files nsSVGDocument.h and nsSVGDocument.cpp need improved before this can
happen. Things I address in my upcomming patch are:

1) remove nsString.h and nsIURI.h from nsSVGDocument.h
2) remove nsIDOMClassInfo.h, nsIHttpChannel.h, nsReadableUtils.h and nsCOMPtr.h
from nsSVGDocument.cpp
3) make GetDomain return a null string when appropriate
4) make GetURL return a null string when appropriate
5) simplify GetRootElement's to reduce footprint
6) make NS_NewSVGDocument not warn on NS_ERROR_OUT_OF_MEMORY
7) make NS_NewSVGDocument return nsnull on failure
8) add attribute IDL comments and generally structure the file to be consistent
with the rest of the SVG code

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch v1

afri, tor: can you review this please? I won't request it directly on the patch
since we might want to get others to use the review flags when requesting
review for getting this code into the official builds.

Comment 2

[Jonathan Watt [:jwatt]]

Or maybe we should even be defering GetRootElement to
nsDocument::GetDocumentElement(nsIDOMElement** aDocumentElement)See:

http://lxr.mozilla.org/mozilla/source/content/base/src/nsDocument.cpp#2243

Simply using:

  return nsXMLDocument::GetDocumentElement(aRootElement);

won't work though since aRootElement is an nsIDOMSVGSVGElement**. IMO this
attribute shouldn't even exist since SVGDocument inherits Document from DOM2
which has the attribute documentElement. 

Comment 3

[[e-mail]]

jwatt, if you want review you really should use the review flags... otherwise
people simply won't know that you want it. please use them ;)

Comment 4

[tor]

Comment on attachment 163327 [details] [diff] [review]
patch v1

 nsresult
 NS_NewSVGDocument(nsIDocument** aInstancePtrResult)
 {
...
   if (NS_FAILED(rv)) {
+    *aInstancePtrResult = nsnull;
...

The clearing of aInstancePtrResult should be moved to the top of the
function so the output value is nulled if we fail to allocate the
nsSVGDocument.	r=tor with that fixed.

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: patch v2 - addressing tor's comments

Given our discussion on IRC can you check in this patch and close this bug Tim?

Comment 6

[tor]

Checking in nsSVGDocument.cpp;
/cvsroot/mozilla/content/svg/document/src/nsSVGDocument.cpp,v  <-- 
nsSVGDocument.cpp
new revision: 1.17; previous revision: 1.16
done
Checking in nsSVGDocument.h;
/cvsroot/mozilla/content/svg/document/src/nsSVGDocument.h,v  <--  nsSVGDocument.h
new revision: 1.7; previous revision: 1.6
done