Open Bug 243723 Opened 21 years ago Updated 2 years ago

deCOMtaminate nsIViewManager

Categories

(Core :: Web Painting, defect, P5)

x86
Linux
defect

Tracking

()

People

(Reporter: bzbarsky, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Some methods on this interface (eg GetWidgetForView()) should really not be using COM-like signatures (I'm picking on GetWidgetForView() because it and especially its callers actually showed up in a profile of some DHTML stuff I was looking at).
Blocks: 243726
Note profile is in bug 243726
Actually what I want to do is to move a lot of view manager methods to nsIView and deCOMtaiminate them at the same time. It's happening slowly.
OK. So is that the plan for GetWidgetForView()? If so, I'll put it on my "to do as soon as I pass my topic" list... ;)
Actually we can just remove GetWidgetForView and have callers call nsIView::GetNearestWidget instead.
cleanup to recognize that (unless we have to back it out) we always have a root view now.
Hmm... Is the aPoint-twiddling done by GetNearestWidget() going to be made up for by the lack of AddRef/Release? Would it make more sense to have an nsIFrame-like API (with GetNearestWidget being like GetClosestView and GetOffsetFromWidget being like GetOffsetFromView)?
I think if you just pass nsnull as the point parameter, you won't notice the overhead.
Attached patch Do that. (obsolete) (deleted) — Splinter Review
So I couldn't sleep..... On that testcase, with this patch, overall time is not changed much (GetWidgetForView was not a huge part of it, all things considered), but instead of having: 84 176 nsViewManager::GetWidgetForView(nsIView*, nsIWidget**) 60 nsWindow::AddRef() 32 nsWidget::AddRef() We now have: 93 93 nsIView::GetNearestWidget(nsPoint*) So the time in the function itself increased by 12% or so (probably plus minus 5% ;) ), but not having to addref those widgets/windows sure helps.
Attachment #148640 - Flags: superreview?(roc)
Attachment #148640 - Flags: review?(roc)
Attachment #148640 - Flags: superreview?(roc)
Attachment #148640 - Flags: superreview+
Attachment #148640 - Flags: review?(roc)
Attachment #148640 - Flags: review+
Comment on attachment 148637 [details] [diff] [review] tiny cleanup related to the above r+sr=bzbarsky on this btw, once the rootview patch relands.
Attachment #148637 - Flags: superreview+
Attachment #148637 - Flags: review+
Is this ready for checkin?
The first patch in this bug depends on bug 233441 (per comments). The second patch was checked in on May 17, 2004 (as a trivial check of the relevant CVS logs would have told you).
Depends on: 233441
Comment on attachment 148640 [details] [diff] [review] Do that. This is checked in.
Attachment #148640 - Attachment is obsolete: true
I just filed 276015 for cleaining GetOffsetFromView and all its callers
QA Contact: ian → layout.view-rendering
Blocks: deCOM
Should this bug still be open? AFAIK the current plan is to eliminate the view manager altogether.
Component: Layout: View Rendering → Layout: Web Painting
Priority: P1 → P5
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: