Closed Bug 947022 Opened 6 years ago Closed 6 years ago
statically type docshell stuff for ns
IPres Shell and ns Pres Context
No description provided.
Trevor, how did you audit to make sure you got all the callers? In particular, I'm worried about cases where a caller used to get an addrefed pointer but now gets a raw pointer...
(In reply to Boris Zbarsky [:bz] from comment #2) > Trevor, how did you audit to make sure you got all the callers? In > particular, I'm worried about cases where a caller used to get an addrefed > pointer but now gets a raw pointer... Well, this used to return an already_AddRefed not a raw pointer so you'd have to cast the return value, or do something pretty strange to have something dangerous happen. I also grepped for all the places that called GetContainer to see which could use GetDocShell() in the process of doing that I think I saw all the callers and non did anything funny with raw pointers they all just assigned to a nsCOMPtr which is still fine. I think there aren't too many callers left so renaming it to double check shouldn't be too bad if you'd like.
> so you'd have to cast the return value, or do something pretty strange to have > something dangerous happen. Something as simple as: already_AddRefed(foo->GetContainer()) would change behavior pretty significantly. And the failure mode is pretty unpleasant. Thank God we no longer allow returning a raw pointer to auto-construct an already_AddRefed, at least... If you're willing to do the renaming test, I would very much appreciate it! I know it's a pain, but I'd rather not end up with deleted-object references.
Comment on attachment 8343432 [details] [diff] [review] bug 947022 - type nsIPresShell::mForwardingContainer and nsPresContext::mContainer >+++ b/layout/base/nsCSSFrameConstructor.cpp > nsCSSFrameConstructor::FindXULMenubarData(Element* aElement, >+ nsCOMPtr<nsIDocShell> treeItem = > aStyleContext->PresContext()->GetContainer(); That can't possibly compile... Should be GetDocShell. I think we should just rename GetContainer() to GetContainerWeak() so binary extensions won't get screwed over too. r=me with those done
Attachment #8343432 - Flags: review?(bzbarsky) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.