233890 - some cleanup in a few layout files

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 141169 [details] [diff] [review]
patch

>Index: content/shared/src/nsImageMapUtils.cpp
>+        nsCOMPtr<nsIDOMHTMLMapElement> map(do_QueryInterface(element));
>+        return map;

This makes me a bit queasy (as there has been some talk of putting some of the
ODM interfaces in tearoffs and that would make this code very very unhappy.

I'll try to read the rest sometime...

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

ok, I can use CallQueryInterface and return an already_AddRefed pointer if you want

Comment 4

[Boris Zbarsky [:bzbarsky]]

That would be better.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

now uses already_AddRefed.

this also contains a few additional changes (nsImageLoader and nsPresShell)

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 141254 [details] [diff] [review]
patch v2

>Index: layout/xul/base/src/nsImageBoxFrame.cpp
>+nsImageBoxFrame::GetLoadGroup()
>+  return doc->GetDocumentLoadGroup().get(); // already_AddRefed

No need for the .get() there.

>Index: layout/xul/base/src/nsImageBoxFrame.h

>-  nsCOMPtr<nsIURI> mURI; // The URI of the image.
>+  nsCOMPtr<nsIURI> mURI; ///< The URI of the image.

What's the reason for this change?

>-  PRPackedBool mUseSrcAttr; // Whether or not the image src comes from an attribute.
>+  PRPackedBool mUseSrcAttr; ///< Whether or not the image src comes from an attribute.
>-  nsRect mSubRect; // If set, indicates that only the portion of the image specified by the rect should be used.
>+  nsRect mSubRect; ///< If set, indicates that only the portion of the image specified by the rect should be used.

Same.

r+sr=bzbarsky with that issue addressed.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

doxygen can generate documentation for ///< comments. The < indicates that it
referes to the thing declared in front of the comment. I believe /**< */ would
also work.

see http://www.stack.nl/~dimitri/doxygen/docblocks.html#memberdoc

I'll make the other change.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It looks like this may have slowed down pageload a little, although it's hard to
tell.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

hm... luna and monkey show no Tp change...