From fd8893c4db25559f83268ef7de3713b19073a169 Mon Sep 17 00:00:00 2001 From: "bent.mozilla@gmail.com" Date: Thu, 28 Feb 2008 18:05:57 -0800 Subject: [PATCH] Bug 419655 - "Refreshing a page leaks an nsGlobalWindow until shutdown". r=peterv, sr=jst, a1.9b4+=schrep. --- dom/src/base/nsFocusController.cpp | 2 + dom/src/base/nsGlobalWindow.cpp | 21 +++++- .../boot/src/nsSecureBrowserUIImpl.cpp | 74 ++++++++++++++----- .../manager/boot/src/nsSecureBrowserUIImpl.h | 2 +- 4 files changed, 74 insertions(+), 25 deletions(-) diff --git a/dom/src/base/nsFocusController.cpp b/dom/src/base/nsFocusController.cpp index 3fa581794c9..3c259e64288 100644 --- a/dom/src/base/nsFocusController.cpp +++ b/dom/src/base/nsFocusController.cpp @@ -102,7 +102,9 @@ NS_INTERFACE_MAP_END NS_IMPL_CYCLE_COLLECTION_UNLINK_0(nsFocusController) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsFocusController) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mCurrentElement) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mPreviousElement) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mCurrentWindow) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mPreviousWindow) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mPopupNode) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END diff --git a/dom/src/base/nsGlobalWindow.cpp b/dom/src/base/nsGlobalWindow.cpp index 4b1435a5211..fea9503949a 100644 --- a/dom/src/base/nsGlobalWindow.cpp +++ b/dom/src/base/nsGlobalWindow.cpp @@ -659,8 +659,8 @@ nsGlobalWindow::nsGlobalWindow(nsGlobalWindow *aOuterWindow) } #ifdef DEBUG printf("++DOMWINDOW == %d (%p) [serial = %d] [Outer = %p]\n", gRefCnt, - static_cast(this), ++gSerialCounter, - aOuterWindow); + static_cast(static_cast(this)), + ++gSerialCounter, aOuterWindow); mSerial = gSerialCounter; #endif @@ -680,8 +680,9 @@ nsGlobalWindow::~nsGlobalWindow() NS_IF_RELEASE(gEntropyCollector); } #ifdef DEBUG - printf("--DOMWINDOW == %d (%p) [serial = %d] [Outer = %p]\n", gRefCnt, - static_cast(this), mSerial, mOuterWindow); + printf("--DOMWINDOW == %d (%p) [serial = %d] [Outer = %p]\n", + gRefCnt, static_cast(static_cast(this)), + mSerial, mOuterWindow); #endif #ifdef PR_LOGGING @@ -1938,6 +1939,18 @@ nsGlobalWindow::SetNewDocument(nsIDocument* aDocument, OBJECT_TO_JSVAL(nav), nsnull, nsnull, JSPROP_ENUMERATE | JSPROP_PERMANENT | JSPROP_READONLY); + + // The Navigator's prototype object keeps a reference to the + // window in which it was first created and can thus cause that + // window to stay alive for too long. Reparenting it here allows + // the window to be collected sooner. + nsIDOMNavigator* navigator = + static_cast(mNavigator); + + xpc-> + ReparentWrappedNativeIfFound(cx, nav, newInnerWindow->mJSObject, + navigator, + getter_AddRefs(navigatorHolder)); } } } diff --git a/security/manager/boot/src/nsSecureBrowserUIImpl.cpp b/security/manager/boot/src/nsSecureBrowserUIImpl.cpp index 8d96bb71b16..48edb428484 100644 --- a/security/manager/boot/src/nsSecureBrowserUIImpl.cpp +++ b/security/manager/boot/src/nsSecureBrowserUIImpl.cpp @@ -170,13 +170,18 @@ NS_IMPL_ISUPPORTS6(nsSecureBrowserUIImpl, NS_IMETHODIMP -nsSecureBrowserUIImpl::Init(nsIDOMWindow *window) +nsSecureBrowserUIImpl::Init(nsIDOMWindow *aWindow) { - PR_LOG(gSecureDocLog, PR_LOG_DEBUG, - ("SecureUI:%p: Init: mWindow: %p, window: %p\n", this, mWindow.get(), - window)); - if (!window) { +#ifdef PR_LOGGING + nsCOMPtr window(do_QueryReferent(mWindow)); + + PR_LOG(gSecureDocLog, PR_LOG_DEBUG, + ("SecureUI:%p: Init: mWindow: %p, aWindow: %p\n", this, + window.get(), aWindow)); +#endif + + if (!aWindow) { NS_WARNING("Null window passed to nsSecureBrowserUIImpl::Init()"); return NS_ERROR_INVALID_ARG; } @@ -185,9 +190,10 @@ nsSecureBrowserUIImpl::Init(nsIDOMWindow *window) NS_WARNING("Trying to init an nsSecureBrowserUIImpl twice"); return NS_ERROR_ALREADY_INITIALIZED; } - - nsresult rv = NS_OK; - mWindow = window; + + nsresult rv; + mWindow = do_GetWeakReference(aWindow, &rv); + NS_ENSURE_SUCCESS(rv, rv); nsCOMPtr service(do_GetService(NS_STRINGBUNDLE_CONTRACTID, &rv)); if (NS_FAILED(rv)) return rv; @@ -206,7 +212,7 @@ nsSecureBrowserUIImpl::Init(nsIDOMWindow *window) rv = svc->AddObserver(this, NS_FORMSUBMIT_SUBJECT, PR_TRUE); } - nsCOMPtr piwindow(do_QueryInterface(mWindow)); + nsCOMPtr piwindow(do_QueryInterface(aWindow)); if (!piwindow) return NS_ERROR_FAILURE; nsIDocShell *docShell = piwindow->GetDocShell(); @@ -353,12 +359,12 @@ static PRUint32 GetSecurityStateFromChannel(nsIChannel* aChannel) NS_IMETHODIMP nsSecureBrowserUIImpl::Notify(nsIDOMHTMLFormElement* aDOMForm, - nsIDOMWindowInternal* window, nsIURI* actionURL, + nsIDOMWindowInternal* aWindow, nsIURI* actionURL, PRBool* cancelSubmit) { // Return NS_OK unless we want to prevent this form from submitting. *cancelSubmit = PR_FALSE; - if (!window || !actionURL || !aDOMForm) + if (!aWindow || !actionURL || !aDOMForm) return NS_OK; nsCOMPtr formNode = do_QueryInterface(aDOMForm); @@ -391,8 +397,11 @@ nsSecureBrowserUIImpl::Notify(nsIDOMHTMLFormElement* aDOMForm, return NS_OK; } + nsCOMPtr window = do_QueryReferent(mWindow); + NS_ASSERTION(window, "Window has gone away?!"); + PRBool isChild; - IsChildOfDomWindow(mWindow, postingWindow, &isChild); + IsChildOfDomWindow(window, postingWindow, &isChild); // This notify call is not for our window, ignore it. if (!isChild) @@ -616,7 +625,10 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress, nsCOMPtr windowForProgress; aWebProgress->GetDOMWindow(getter_AddRefs(windowForProgress)); - const PRBool isToplevelProgress = (windowForProgress.get() == mWindow.get()); + nsCOMPtr window = do_QueryReferent(mWindow); + NS_ASSERTION(window, "Window has gone away?!"); + + const PRBool isToplevelProgress = (windowForProgress == window); #ifdef PR_LOGGING if (windowForProgress) @@ -1211,7 +1223,10 @@ nsSecureBrowserUIImpl::OnLocationChange(nsIWebProgress* aWebProgress, nsCOMPtr windowForProgress; aWebProgress->GetDOMWindow(getter_AddRefs(windowForProgress)); - if (windowForProgress.get() == mWindow.get()) { + nsCOMPtr window = do_QueryReferent(mWindow); + NS_ASSERTION(window, "Window has gone away?!"); + + if (windowForProgress == window) { // For toplevel channels, update the security state right away. return EvaluateAndUpdateSecurityState(aRequest); } @@ -1395,6 +1410,7 @@ nsUIContext::~nsUIContext() /* void getInterface (in nsIIDRef uuid, [iid_is (uuid), retval] out nsQIResult result); */ NS_IMETHODIMP nsUIContext::GetInterface(const nsIID & uuid, void * *result) { + NS_ENSURE_TRUE(mWindow, NS_ERROR_FAILURE); nsresult rv; if (uuid.Equals(NS_GET_IID(nsIPrompt))) { @@ -1446,7 +1462,10 @@ ConfirmEnteringSecure() GetNSSDialogs(getter_AddRefs(dialogs)); if (!dialogs) return PR_FALSE; // Should this allow PR_TRUE for unimplemented? - nsCOMPtr ctx = new nsUIContext(mWindow); + nsCOMPtr window = do_QueryReferent(mWindow); + NS_ASSERTION(window, "Window has gone away?!"); + + nsCOMPtr ctx = new nsUIContext(window); PRBool confirms; dialogs->ConfirmEnteringSecure(ctx, &confirms); @@ -1462,7 +1481,10 @@ ConfirmEnteringWeak() GetNSSDialogs(getter_AddRefs(dialogs)); if (!dialogs) return PR_FALSE; // Should this allow PR_TRUE for unimplemented? - nsCOMPtr ctx = new nsUIContext(mWindow); + nsCOMPtr window = do_QueryReferent(mWindow); + NS_ASSERTION(window, "Window has gone away?!"); + + nsCOMPtr ctx = new nsUIContext(window); PRBool confirms; dialogs->ConfirmEnteringWeak(ctx, &confirms); @@ -1478,7 +1500,10 @@ ConfirmLeavingSecure() GetNSSDialogs(getter_AddRefs(dialogs)); if (!dialogs) return PR_FALSE; // Should this allow PR_TRUE for unimplemented? - nsCOMPtr ctx = new nsUIContext(mWindow); + nsCOMPtr window = do_QueryReferent(mWindow); + NS_ASSERTION(window, "Window has gone away?!"); + + nsCOMPtr ctx = new nsUIContext(window); PRBool confirms; dialogs->ConfirmLeavingSecure(ctx, &confirms); @@ -1494,7 +1519,10 @@ ConfirmMixedMode() GetNSSDialogs(getter_AddRefs(dialogs)); if (!dialogs) return PR_FALSE; // Should this allow PR_TRUE for unimplemented? - nsCOMPtr ctx = new nsUIContext(mWindow); + nsCOMPtr window = do_QueryReferent(mWindow); + NS_ASSERTION(window, "Window has gone away?!"); + + nsCOMPtr ctx = new nsUIContext(window); PRBool confirms; dialogs->ConfirmMixedMode(ctx, &confirms); @@ -1517,7 +1545,10 @@ ConfirmPostToInsecure() GetNSSDialogs(getter_AddRefs(dialogs)); if (!dialogs) return PR_FALSE; // Should this allow PR_TRUE for unimplemented? - nsCOMPtr ctx = new nsUIContext(mWindow); + nsCOMPtr window = do_QueryReferent(mWindow); + NS_ASSERTION(window, "Window has gone away?!"); + + nsCOMPtr ctx = new nsUIContext(window); PRBool result; @@ -1542,7 +1573,10 @@ ConfirmPostToInsecureFromSecure() GetNSSDialogs(getter_AddRefs(dialogs)); if (!dialogs) return PR_FALSE; // Should this allow PR_TRUE for unimplemented? - nsCOMPtr ctx = new nsUIContext(mWindow); + nsCOMPtr window = do_QueryReferent(mWindow); + NS_ASSERTION(window, "Window has gone away?!"); + + nsCOMPtr ctx = new nsUIContext(window); PRBool result; diff --git a/security/manager/boot/src/nsSecureBrowserUIImpl.h b/security/manager/boot/src/nsSecureBrowserUIImpl.h index 54c08d74bec..8c358e064bf 100644 --- a/security/manager/boot/src/nsSecureBrowserUIImpl.h +++ b/security/manager/boot/src/nsSecureBrowserUIImpl.h @@ -93,7 +93,7 @@ public: protected: - nsCOMPtr mWindow; + nsWeakPtr mWindow; nsCOMPtr mStringBundle; nsCOMPtr mCurrentURI; nsCOMPtr mToplevelEventSink;