592964 - Use nsDependentAtomString instead of nsAtomString for temporary variables

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Daniel Holbert [:dholbert]]

Per Jonas's description of nsAtomString & nsDependentAtomString, here...
http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/26708ed289c3e3b0/2d5c790e28dd1016
... nsDependentAtomString is slightly cheaper than nsAtomString, since it doesn't do an AddRef/Release.

As a result, in places where the object is just being created on the stack for temporary use, it's slightly better to use nsDependentAtomString.

Filing this bug on using nsDependentAtomString in these three places in XSLT code:
> /content/xslt/src/xslt/txMozillaXMLOutput.cpp (View Hg log or Hg annotations)
> line 632 -- rv = mDocument->CreateElem(nsAtomString(nsGkAtoms::result), nsGkAtoms::transformiix,
> /content/xslt/src/xslt/txEXSLTFunctions.cpp (View Hg log or Hg annotations)
> line 175 -- nsresult rv = doc->CreateElem(nsAtomString(aName), nsnull, kNameSpaceID_None, PR_FALSE,
> /content/xslt/src/xslt/txMozillaTextOutput.cpp (View Hg log or Hg annotations)
> line 222 -- rv = mDocument->CreateElem(nsAtomString(nsGkAtoms::result), nsGkAtoms::transformiix,

Comment 1

[Daniel Holbert [:dholbert]]

I was going to file separate bugs for other /content/ subdirectories, but I think that's silly, since this is just a series of trivial one-liners that are all in the same general area of code.

Here are the other places that could stand to be fixed (based on an mxr search for "nsAtomString(":

> /content/svg/content/src/nsSVGElement.cpp
> line 1172 -- nsCSSProps::LookupProperty(nsAtomString(aMappedAttrName));
> line 2194 -- nsCSSProperty prop = nsCSSProps::LookupProperty(nsAtomString(aName));
> /content/xul/document/src/nsXULDocument.cpp
> line 3836 -- nsresult rv = document->CreateElem(nsAtomString(nsGkAtoms::treechildren),
> /content/base/src/nsNodeInfoManager.cpp
> line 77 -- return HashString(nsAtomString(node->mName));

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: fix with checkin comment (noting review+approval)

Thanks jst!

Here's the fix for checkin, with commit message included, so this can hopefully be landed as a ridealong.

Comment 4

[Josh Matthews [:jdm]]

http://hg.mozilla.org/mozilla-central/rev/ebce58252cf8

Comment 5

[Josh Matthews [:jdm]]

Backed out along with the other patches that caused Moth test failures. This one's almost certainly not the cause.

Comment 6

[Joe Drew (not getting mail)]

http://hg.mozilla.org/mozilla-central/rev/b57dda2ee56d