412691 - Remove unnecessary nsCOMPtr's from caps code.

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[Johnny Stenback (:jst)]

Attachment: Remove unnecessary nsCOMPtr and replace QI with a cast.

CheckPropertyAccessImpl() uses a nsCOMPtr for a principal when it doesn't need to, and also does a QI on all objects that claim to be nsIClassInfo::CONTENT_NODE objects to QI from nsISupports to nsIContent. Removing the former is obvious, the latter can safely be replaced with a cast because the only objects that claim to be CONTENT_NODE objects are ones that implement nsIContent, and their nsISupport pointers are safe to cast to nsIContent (we depend on that elsewhere). And also note, that if the QI that's there now would fail, we'd crash in the current code, so we can already be sure that nothing that doesn't implement nsIContent comes into this code.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 297435 [details] [diff] [review]
Remove unnecessary nsCOMPtr and replace QI with a cast.

I think technically 'kungFuDeathGrip' is wrong here. I think we call this *Owner or something in other places, so principalOwner here or some such.

r/sr=me either way

Comment 2

[Johnny Stenback (:jst)]

Checked in with s/kungFuDeathGrip/principalHolder/. Marking FIXED.

Comment 3

[[e-mail]]

Comment on attachment 297435 [details] [diff] [review]
Remove unnecessary nsCOMPtr and replace QI with a cast.

>-        nsCOMPtr<nsIContent> content(do_QueryInterface(aObj));
>+        nsIContent *content = static_cast<nsIContent*>(aObj);
>         NS_ASSERTION(content, "classinfo had CONTENT_NODE set but node did not"
>                               "implement nsIContent!  Fasten your seat belt.");
Unlike the QI, the cast will always "succeed", so the ASSERTION is useless, no?