See bug 190735 comment 25 and bug 190735 comment 26. As a first pass, I'd like to add some inline accessors to nsIPresContext and nsIPresShell (probably some other places too, but these will help with removing |aPresContext| parameters from nsIFrameManager methods. Does this seem like the direction we want to go?

slightly more accessors

Comment on attachment 125049 [details] [diff] [review] add inline accessors to nsIPresShell and nsIPresContext Is this the direction we want to go?

Comment on attachment 125049 [details] [diff] [review] add inline accessors to nsIPresShell and nsIPresContext The direction looks fine (great, even). Two questions: 1) What happens if the prescontext's mShell pointer is null and one of the other accessors is called? 2) Why the move from nsCOMPtrs to raw pointers?

> 1) What happens if the prescontext's mShell pointer is null and one of the > other accessors is called? The same thing that happens to the few hundred places currently in the code where we do things like: nsCOMPtr<nsIPresShell> shell; aPresContext->GetShell(getter_AddRefs(shell)); shell->do_something(); > 2) Why the move from nsCOMPtrs to raw pointers? So that I don't have to include headers, which could vastly increase dependencies (slow down compilation and likely mess up REQUIRES). (And the ability to forward-declare nsCOMPtrs wouldn't solve this, since the inline functions use them.)

Comment on attachment 125049 [details] [diff] [review] add inline accessors to nsIPresShell and nsIPresContext >Index: base/src/nsPresContext.cpp >+ *aManager = GetEventStateManager(); >+ if (!*aManager) >+ return NS_ERROR_UNEXPECTED; NS_ERROR_OUT_OF_MEMORY seems more appropriate here... >Index: html/base/src/nsPresShell.cpp >+ mDocument = aDocument; >+ NS_IF_ADDREF(mDocument); We already assert that the document is non-null, so this may as well be NS_ADDREF. sr=me with those minor nits.

Comment on attachment 125049 [details] [diff] [review] add inline accessors to nsIPresShell and nsIPresContext Yummy

Comment on attachment 125049 [details] [diff] [review] add inline accessors to nsIPresShell and nsIPresContext Patch checked in to trunk, 2003-06-19 11:16 -0700.

This adds nsIFrame::GetPresContext as I suggested in bug 190735 comment 29. It also cleans up some related signatures on nsStyleContext and nsRuleNode, and removes an unused method from nsRuleNode.

Comment on attachment 126053 [details] [diff] [review] remove |aPresContext| and |aPresShell| parameters from nsIFrameManager methods r+sr=me.

Comment on attachment 126053 [details] [diff] [review] remove |aPresContext| and |aPresShell| parameters from nsIFrameManager methods Patch checked in to trunk, 2003-06-19 16:52 -0700.

Comment on attachment 126064 [details] [diff] [review] add nsIFrame::GetPresContext and clean up some nsRuleNode/nsStyleContext signatures fabulous, r+sr=roc

Comment on attachment 126064 [details] [diff] [review] add nsIFrame::GetPresContext and clean up some nsRuleNode/nsStyleContext signatures Patch checked in to trunk, 2003-06-19 18:22 -0700.

This bug made inspector crash sometimes.

Comment on attachment 126591 [details] [diff] [review] fix regression from attachment 126064 [details] [diff] [review] looks familiar :-)

Comment on attachment 126591 [details] [diff] [review] fix regression from attachment 126064 [details] [diff] [review] Patch checked in to trunk, 2003-06-27 14:06 -0700.

Comment on attachment 127579 [details] [diff] [review] add frame manager accessors, clean up nsIFrame::(Get|Set)View + f && (f->GetStateBits() & NS_FRAME_HAS_CHILD_WITH_VIEW); Shouldnt't that be !(f->GetStateBits() & NS_FRAME_HAS_CHILD_WITH_VIEW ? With that change, r+sr=roc

Comment on attachment 127579 [details] [diff] [review] add frame manager accessors, clean up nsIFrame::(Get|Set)View Patch checked in to trunk (with correction), 2003-07-11 17:46/48/50 -0700.

Should this bug still be open? It looks like all the work was done and checked in years ago.