330300 - move mFrame to nsDisplayItem

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Alfred Peng]

Every nsDisplayItem subclass declares its own mFrame. Move these mFrames up to nsDisplayItem, make it nsIFrame*, make GetUnderlyingFrame() nonvirtual and inline, and use NS_STATIC_CAST wherever the nsDisplayItem subclasses need to get the frame with the right type.

Comment 1

[Alfred Peng]

Attachment: Patchv1 for Trunk

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This looks great.

Please make CVS diffs with the -t option.

+  nsDisplayItem(nsIFrame* aFrame) : nsDisplayItemLink(), mFrame(aFrame) {}

You don't need "nsDisplayItemLink()", it's implied.

+ const nsStyleFont* font =
+    NS_STATIC_CAST(nsHTMLContainerFrame*, mFrame)->GetStyleFont();

You don't need the cast here.

+  const nsStyleVisibility* visibility =
+    NS_STATIC_CAST(nsHTMLContainerFrame*, mFrame)->GetStyleVisibility();      

Don't need the cast here.

-      mFrame->PaintTextDecorationLine(*aCtx, pt, mLine, mColor, offset, ascent, size);
+      NS_STATIC_CAST(nsHTMLContainerFrame*, mFrame)->
+        PaintTextDecorationLine(*aCtx, pt, mLine, mColor, offset, ascent, size);
     } else if (mDecoration == NS_STYLE_TEXT_DECORATION_OVERLINE) {
-      mFrame->PaintTextDecorationLine(*aCtx, pt, mLine, mColor, ascent, ascent, size);
+      NS_STATIC_CAST(nsHTMLContainerFrame*, mFrame)->
+        PaintTextDecorationLine(*aCtx, pt, mLine, mColor, ascent, ascent, size);
     }
   } else {
     fm->GetStrikeout(offset, size);
-    mFrame->PaintTextDecorationLine(*aCtx, pt, mLine, mColor, offset, ascent, size);
+    NS_STATIC_CAST(nsHTMLContainerFrame*, mFrame)->
+      PaintTextDecorationLine(*aCtx, pt, mLine, mColor, offset, ascent, size);

Could simplify this by putting "nsHTMLContainerFrame* f = NS_STATIC_CAST(nsHTMLContainerFrame*, mFrame)" up above.

-  nsDisplayXULDebug(nsComboboxControlFrame* aFrame) : mFrame(aFrame) {
+  nsDisplayXULDebug(nsComboboxControlFrame* aFrame) : nsDisplayItem(aFrame) {

Can you make the parameter here be just an nsIFrame*? This was broken when I checked it in, it isn't normally part of the build.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 214872 [details] [diff] [review]
Patchv1 for Trunk

r+sr assuming you accept the comments I just made.

Comment 4

[Alfred Peng]

Attachment: Patchv2 for Trunk

This patch has updated all the comments above. I use -u8t for the diff option this time.
I failed to find out the definition of GetStyleVisibility and GetStyleFont by LXR. Where can I get it?
Robert, if you're ok with the patch, please help me check it in.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #4)
> This patch has updated all the comments above. I use -u8t for the diff option
> this time.
> I failed to find out the definition of GetStyleVisibility and GetStyleFont by
> LXR. Where can I get it?

They're in nsIFrame, generated by macros.

> Robert, if you're ok with the patch, please help me check it in.

I will check it in. Thanks!

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in.