Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Cleanup nsBoxFrame::SetLayoutManager

RESOLVED FIXED

Status

()

Core
XP Toolkit/Widgets: XUL
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Neil Deakin (not available until Aug 9), Assigned: Neil Deakin (not available until Aug 9))

Tracking

Trunk
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 541674 [details] [diff] [review]
patch

Simple patch to remove some reference counting and use return values rather than outpointers.
Attachment #541674 - Flags: review?(roc)
Comment on attachment 541674 [details] [diff] [review]
patch

Review of attachment 541674 [details] [diff] [review]:
-----------------------------------------------------------------

I guess instead of returning already_AddRefed you could create a method nsIBoxLayout::AsGridPart that returns an nsIGridPart* or null. Then you can effectively QI to nsIGridPart without addreffing.

::: layout/generic/nsIFrame.h
@@ +2590,5 @@
>    NS_IMETHOD GetBorder(nsMargin& aBorder)=0;
>    NS_IMETHOD GetPadding(nsMargin& aBorderAndPadding)=0;
>    NS_IMETHOD GetMargin(nsMargin& aMargin)=0;
> +  virtual void SetLayoutManager(nsIBoxLayout* aLayout) { }
> +  virtual nsIBoxLayout* GetLayoutManager() { return NULL; }

nsnull

::: layout/xul/base/src/grid/nsGrid.cpp
@@ +297,5 @@
>         NS_ASSERTION(scrolledFrame,"Error no scroll frame!!");
>         child = do_QueryFrame(scrolledFrame);
>      }
>  
> +    nsCOMPtr<nsIGridPart> monument(do_QueryInterface(child->GetLayoutManager()));

Shouldn't you call nsGrid::GetPartFromBox(child) here and in the other places you're cleaning up?

@@ +594,4 @@
>  {
> +  if (aBox) {
> +    nsCOMPtr<nsIGridPart> part(do_QueryInterface(aBox->GetLayoutManager()));
> +    return part;

since you have to take a reference anyway, and some callers might want to hold a reference, why not return already_AddRefed<nsIGridPart> and part.forget() here?

@@ +599,2 @@
>  
> +  return NULL;

nsnull

::: layout/xul/base/src/grid/nsGrid.h
@@ +113,5 @@
>                            nsGridRow*& aLastRow,
>                            PRBool aIsHorizontal);
>  
>  private:
> +  nsIGridPart* GetPartFromBox(nsIBox* aBox);

This should be static, right?

::: layout/xul/base/src/grid/nsGridRowLayout.cpp
@@ +80,5 @@
>    ChildAddedOrRemoved(aBox, aState);
>  }
>  
> +nsIGridPart*
> +nsGridRowLayout::GetParentGridPart(nsIBox* aBox, nsIBox** aParentBox)

Same here, return already_AddRefed<nsIGridPart>.
Created attachment 543151 [details] [diff] [review]
patch v2
Attachment #541674 - Attachment is obsolete: true
Attachment #543151 - Flags: review?(roc)
Attachment #541674 - Flags: review?(roc)
Comment on attachment 543151 [details] [diff] [review]
patch v2

Review of attachment 543151 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #543151 - Flags: review?(roc) → review+
http://hg.mozilla.org/mozilla-central/rev/3d6a355a8176
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.