1024138 - Remove the GetPresShell() and GetPresContext() methods from nsFrameManager

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jonathan Watt [:jwatt]]

Nobody calls the GetPresShell() and GetPresContext() methods on nsFrameManager, so we should remove them.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Daniel Holbert [:dholbert]]

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

># HG changeset patch
># Parent ef38952a39abd6be00c72693bbdffdb7a4be04c1
># User Jonathan Watt <[e-mail]>
>Bug 1024138 - Remove the GetPresShell() and GetPresContext() methods from nsFrameManager. r=dholbert

(maybe "Remove unused", instead of "Remove the", to make it clearer from the commit message what the scope of this is?)

>diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp
> nsCSSFrameConstructor::nsCSSFrameConstructor(nsIDocument *aDocument,
>                                              nsIPresShell *aPresShell,
>                                              nsStyleSet* aStyleSet)
>-  : nsFrameManager(aPresShell, aStyleSet)
>-  , mDocument(aDocument)
>+  : mDocument(aDocument)

I don't think we should make this change to the constructors. (shifting responsibility to nsCSSFrameConstructor, for initializing these members)

As long as nsFrameManager itself* manages mPresShell and mStyleSet, it should be the one to set those values in the constructor.

* (OK, technically these members are declared on its evil appendage nsFrameManagerBase, not on nsFrameManager. Nonetheless, nsFrameManager works with them, and e.g. its destructor has assertions about mPresShell at least, so its constructor should still be responsible for setting them.)

If we end up folding it together with nsCSSFrameConstructor, then we can merge their member-data-initialization, but I don't think it makes sense to do that here.

>@@ -152,16 +146,11 @@ public:
>-
>-  nsIPresShell* GetPresShell() const { return mPresShell; }
>-  nsPresContext* GetPresContext() const {
>-    return mPresShell->GetPresContext();
>-  }


So, r=me on this part ^. :)

Comment 3

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/78609a59418f

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/78609a59418f