All users were logged out of Bugzilla on October 13th, 2018

Clarify ownership model for nsISecureBrowserUI

RESOLVED WORKSFORME

Status

()

RESOLVED WORKSFORME
13 years ago
21 days ago

People

(Reporter: sfraser_bugs, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

13 years ago
Bug 303839 reveals a leak caused by a ref cycle between docShell and its
nsISecureBrowserUI, since nsDocShell retains its nsISecureBrowserUI, and the
nsSecureBrowserUIImpl retains the docShell via mToplevelEventSink.

In addition, nsWebBrowser assumes knowledge of the implementation of
nsISecureBrowserUI, since the block in nsWebBrowser::Create() where we create
and init the nsISecureBrowserUI doesn't retain a reference (it "knows" that
nsSecureBrowserUIImpl gets from the domWindow to the docShell, and calls
docShell->SetSecurityUI()).

All this needs cleaning up. It seems that nsWebBrowser should be the one to call
docShell->SetSecurityUI(), and that nsISecureBrowserUI should be able to clean
up its cycle-causing back-references somehow.
Assignee: bryner → nobody
QA Contact: adamlock → docshell
(Is this related to bug 372107 ?)

Comment 2

21 days ago
I think this is no longer the case now that Dana reworked this, so we can close as wfm - Dana, is that right?
Flags: needinfo?(dkeeler)
I think the ownership is clear at this point (docShell owns nsSecureBrowserUIImpl, nsSecureBrowserUIImpl only holds weak references to the docShell/web progress), but comment 0 is still correct in that the setup is needlessly complicated. Maybe let's just morph this a bit?
Flags: needinfo?(dkeeler)
Summary: Clarify ownership model for nsISecureBrowserUI → simplify setup of docShell/nsISecureBrowserUI
Actually I guess bug 1493655 would address this.
Status: NEW → RESOLVED
Last Resolved: 21 days ago
Resolution: --- → WORKSFORME
Summary: simplify setup of docShell/nsISecureBrowserUI → Clarify ownership model for nsISecureBrowserUI
You need to log in before you can comment on or make changes to this bug.