1247478 - nsDocument::FirstAdditionalAuthorSheet() is unclear about its lack-of-infallibility

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Holbert [:dholbert]]

In Gecko (or layout/style at least), there's a convention that getter-functions that return pointers should include "Get" in the name if they might return null, or should omit "Get" if they promise to return non-null.

nsDocument::FirstAdditionalAuthorSheet() seems to be an exception to this convention right now:
4431 CSSStyleSheet*
4432 nsDocument::FirstAdditionalAuthorSheet()
4433 {
4434   return mAdditionalSheets[eAuthorSheet].SafeElementAt(0, nullptr);
4435 }

http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDocument.cpp?rev=8de87e81529a#4431

Note the "SafeElementAt" call there -- that'll return nullptr (the second arg that was passed to it) if the array doesn't have enough entries (in this case, if the array is empty).

So, this FirstAdditionalAuthorSheet function is explicitly returning null (via the fallback value it passes to SafeElementAt) in some cases.

So, we should probably rename it to "GetFirstAdditionalAuthorSheet".

(Note that we're changing this function to return a StyleSheetHandle instead of a CSSStyleSheet* in bug 1244074 part 4. This bug is still valid, regardless, since StyleSheetHandle is still pointer-like, and the function is still allowing itself to return a default-constructed (null-ish) StyleSheetHandle.)

Marking this bug as depending on bug 1244074, because we shouldn't fix this until bug 1244074 has landed, to avoid bitrotting those patches.

Comment 1

[Cameron McCormack (:heycam)]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> So, we should probably rename it to "GetFirstAdditionalAuthorSheet".

Sounds good to me.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix v1: Rename to add "Get"

Here's a patch to do this rename.

(I rewrapped one contextual comment that was over 80 characters, too.)

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/be17cdb8e8e0

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/be17cdb8e8e0