Closed Bug 1441009 Opened 2 years ago Closed 2 years ago
Don't null-check Owner
No description provided.
Comment on attachment 8953836 [details] Bug 1441009: Don't null-check OwnerDoc. https://reviewboard.mozilla.org/r/223010/#review228876
Attachment #8953836 - Flags: review?(bugs) → review+
Comment on attachment 8953837 [details] Bug 1441009: bonus: Don't read garbage in CanvasUtils. https://reviewboard.mozilla.org/r/223012/#review228878 In practice this doesn't change anything, since isFileURL is always wet if valid scheme is passed as param.
Attachment #8953837 - Flags: review?(bugs) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/7019615c84b2 Don't null-check OwnerDoc. r=smaug https://hg.mozilla.org/integration/autoland/rev/c0ef85232ad3 Use a bit less dangerous error handling in CanvasUtils. r=smaug
OwnerDoc() should return NotNull<nsIDocument*> if we know it will never be null.
(In reply to Masatoshi Kimura [:emk] from comment #6) > OwnerDoc() should return NotNull<nsIDocument*> if we know it will never be > null. Or a reference, just sayin ;)
In DOM especially methods which return Foo* but don't have Get* in the method name aren't supposed to return null ever. NotNull is way newer code than that convention. But using NotNull sounds reasonable if one wants to write the patch.
Yeah, returning reference might make even more sense.
Though, then people might start to write code like nsIDocument& doc = element->OwnerDoc(); without thinking whether doc should have been stored in a strong reference. So, don't like reference after all.
(In reply to Olli Pettay [:smaug] from comment #10) > Though, then people might start to write code like > nsIDocument& doc = element->OwnerDoc(); without thinking whether doc should > have been stored in a strong reference. So, don't like reference after all. FWIW, the way WebKit deals with this is there's a Ref<T> type, which holds a strong reference just like nsCOMPtr / RefPtr, but is constructible from a reference. Obviously it internally uses a pointer, but that means that you no longer need pointers or awkwardness (&) to store strong refs. Maybe we could do something like that eventually, though given the differences between COM / Ref ptrs it may not be so trivial.
We have OwningNonNull<T> for non-null strong references.
You need to log in before you can comment on or make changes to this bug.