From deb5725f5b83447ae60fff86919f1b260b8b33e2 Mon Sep 17 00:00:00 2001 From: Alexander Surkov Date: Wed, 29 Jul 2009 17:01:48 +0800 Subject: [PATCH] Bug 468659 - Crash [@ nsAccessNode::GetDocAccessibleFor(nsIDocShellTreeItem*, int) ], r=ginn.chen --- accessible/src/base/nsAccessNode.cpp | 47 +++++------ accessible/src/base/nsAccessNode.h | 6 +- .../src/base/nsAccessibilityService.cpp | 77 +++++++++++++------ accessible/src/base/nsAccessibilityService.h | 10 +++ accessible/src/base/nsDocAccessible.cpp | 4 +- accessible/src/msaa/nsAccessNodeWrap.cpp | 9 +-- 6 files changed, 88 insertions(+), 65 deletions(-) diff --git a/accessible/src/base/nsAccessNode.cpp b/accessible/src/base/nsAccessNode.cpp index e8d3c59f0691..49ab420022cc 100644 --- a/accessible/src/base/nsAccessNode.cpp +++ b/accessible/src/base/nsAccessNode.cpp @@ -40,7 +40,7 @@ #include "nsIAccessible.h" #include "nsAccessibilityAtoms.h" #include "nsHashtable.h" -#include "nsIAccessibilityService.h" +#include "nsAccessibilityService.h" #include "nsIAccessibleDocument.h" #include "nsIDocShell.h" #include "nsIDocShellTreeItem.h" @@ -82,27 +82,19 @@ nsIStringBundle *nsAccessNode::gStringBundle = 0; nsIStringBundle *nsAccessNode::gKeyStringBundle = 0; nsITimer *nsAccessNode::gDoCommandTimer = 0; nsIDOMNode *nsAccessNode::gLastFocusedNode = 0; +#ifdef DEBUG PRBool nsAccessNode::gIsAccessibilityActive = PR_FALSE; -PRBool nsAccessNode::gIsShuttingDownApp = PR_FALSE; +#endif PRBool nsAccessNode::gIsCacheDisabled = PR_FALSE; PRBool nsAccessNode::gIsFormFillEnabled = PR_FALSE; nsAccessNodeHashtable nsAccessNode::gGlobalDocAccessibleCache; nsApplicationAccessibleWrap *nsAccessNode::gApplicationAccessible = nsnull; -nsIAccessibilityService *nsAccessNode::sAccService = nsnull; -nsIAccessibilityService *nsAccessNode::GetAccService() +nsIAccessibilityService* +nsAccessNode::GetAccService() { - if (!gIsAccessibilityActive) - return nsnull; - - if (!sAccService) { - nsresult rv = CallGetService("@mozilla.org/accessibilityService;1", - &sAccService); - NS_ASSERTION(NS_SUCCEEDED(rv), "No accessibility service"); - } - - return sAccService; + return nsAccessibilityService::GetAccessibilityService(); } /* @@ -245,9 +237,7 @@ NS_IMETHODIMP nsAccessNode::GetOwnerWindow(void **aWindow) already_AddRefed nsAccessNode::GetApplicationAccessible() { - if (!gIsAccessibilityActive) { - return nsnull; - } + NS_ASSERTION(gIsAccessibilityActive, "Accessibility wasn't initialized!"); if (!gApplicationAccessible) { nsApplicationAccessibleWrap::PreCreate(); @@ -273,9 +263,8 @@ nsAccessNode::GetApplicationAccessible() void nsAccessNode::InitXPAccessibility() { - if (gIsAccessibilityActive) { - return; - } + NS_ASSERTION(!gIsAccessibilityActive, + "Accessibility was initialized already!"); nsCOMPtr stringBundleService = do_GetService(NS_STRINGBUNDLE_CONTRACTID); @@ -297,11 +286,13 @@ void nsAccessNode::InitXPAccessibility() prefBranch->GetBoolPref("browser.formfill.enable", &gIsFormFillEnabled); } +#ifdef DEBUG gIsAccessibilityActive = PR_TRUE; - NotifyA11yInitOrShutdown(); +#endif + NotifyA11yInitOrShutdown(PR_TRUE); } -void nsAccessNode::NotifyA11yInitOrShutdown() +void nsAccessNode::NotifyA11yInitOrShutdown(PRBool aIsInit) { nsCOMPtr obsService = do_GetService("@mozilla.org/observer-service;1"); @@ -310,7 +301,7 @@ void nsAccessNode::NotifyA11yInitOrShutdown() static const PRUnichar kInitIndicator[] = { '1', 0 }; static const PRUnichar kShutdownIndicator[] = { '0', 0 }; obsService->NotifyObservers(nsnull, "a11y-init-or-shutdown", - gIsAccessibilityActive ? kInitIndicator : kShutdownIndicator); + aIsInit ? kInitIndicator : kShutdownIndicator); } } @@ -320,16 +311,12 @@ void nsAccessNode::ShutdownXPAccessibility() // which happens when xpcom is shutting down // at exit of program - if (!gIsAccessibilityActive) { - return; - } - gIsShuttingDownApp = PR_TRUE; + NS_ASSERTION(gIsAccessibilityActive, "Accessibility was shutdown already!"); NS_IF_RELEASE(gStringBundle); NS_IF_RELEASE(gKeyStringBundle); NS_IF_RELEASE(gDoCommandTimer); NS_IF_RELEASE(gLastFocusedNode); - NS_IF_RELEASE(sAccService); nsApplicationAccessibleWrap::Unload(); ClearCache(gGlobalDocAccessibleCache); @@ -339,8 +326,10 @@ void nsAccessNode::ShutdownXPAccessibility() NS_IF_RELEASE(gApplicationAccessible); gApplicationAccessible = nsnull; +#ifdef DEBUG gIsAccessibilityActive = PR_FALSE; - NotifyA11yInitOrShutdown(); +#endif + NotifyA11yInitOrShutdown(PR_FALSE); } PRBool diff --git a/accessible/src/base/nsAccessNode.h b/accessible/src/base/nsAccessNode.h index 46f9e2270e5b..da3ecb5faf9f 100644 --- a/accessible/src/base/nsAccessNode.h +++ b/accessible/src/base/nsAccessNode.h @@ -174,21 +174,21 @@ protected: /** * Notify global nsIObserver's that a11y is getting init'd or shutdown */ - static void NotifyA11yInitOrShutdown(); + static void NotifyA11yInitOrShutdown(PRBool aIsInit); // Static data, we do our own refcounting for our static data static nsIStringBundle *gStringBundle; static nsIStringBundle *gKeyStringBundle; static nsITimer *gDoCommandTimer; +#ifdef DEBUG static PRBool gIsAccessibilityActive; - static PRBool gIsShuttingDownApp; +#endif static PRBool gIsCacheDisabled; static PRBool gIsFormFillEnabled; static nsAccessNodeHashtable gGlobalDocAccessibleCache; private: - static nsIAccessibilityService *sAccService; static nsApplicationAccessibleWrap *gApplicationAccessible; }; diff --git a/accessible/src/base/nsAccessibilityService.cpp b/accessible/src/base/nsAccessibilityService.cpp index 4cebad4ac760..116bc73d5e82 100644 --- a/accessible/src/base/nsAccessibilityService.cpp +++ b/accessible/src/base/nsAccessibilityService.cpp @@ -113,6 +113,7 @@ #endif nsAccessibilityService *nsAccessibilityService::gAccessibilityService = nsnull; +PRBool nsAccessibilityService::gIsShutdown = PR_TRUE; /** * nsAccessibilityService @@ -120,6 +121,7 @@ nsAccessibilityService *nsAccessibilityService::gAccessibilityService = nsnull; nsAccessibilityService::nsAccessibilityService() { + // Add observers. nsCOMPtr observerService = do_GetService("@mozilla.org/observer-service;1"); if (!observerService) @@ -132,13 +134,15 @@ nsAccessibilityService::nsAccessibilityService() nsIWebProgress::NOTIFY_STATE_DOCUMENT | nsIWebProgress::NOTIFY_LOCATION); } + + // Initialize accessibility. nsAccessNodeWrap::InitAccessibility(); } nsAccessibilityService::~nsAccessibilityService() { - nsAccessibilityService::gAccessibilityService = nsnull; - nsAccessNodeWrap::ShutdownAccessibility(); + NS_ASSERTION(gIsShutdown, "Accessibility wasn't shutdown!"); + gAccessibilityService = nsnull; } NS_IMPL_THREADSAFE_ISUPPORTS5(nsAccessibilityService, nsIAccessibilityService, nsIAccessibleRetrieval, @@ -151,6 +155,8 @@ nsAccessibilityService::Observe(nsISupports *aSubject, const char *aTopic, const PRUnichar *aData) { if (!nsCRT::strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID)) { + + // Remove observers. nsCOMPtr observerService = do_GetService("@mozilla.org/observer-service;1"); if (observerService) { @@ -159,8 +165,8 @@ nsAccessibilityService::Observe(nsISupports *aSubject, const char *aTopic, nsCOMPtr progress(do_GetService(NS_DOCUMENTLOADER_SERVICE_CONTRACTID)); if (progress) progress->RemoveProgressListener(static_cast(this)); - nsAccessNodeWrap::ShutdownAccessibility(); - // Cancel and release load timers + + // Cancel and release load timers. while (mLoadTimers.Count() > 0 ) { nsCOMPtr timer = mLoadTimers.ObjectAt(0); void *closure = nsnull; @@ -172,7 +178,18 @@ nsAccessibilityService::Observe(nsISupports *aSubject, const char *aTopic, timer->Cancel(); mLoadTimers.RemoveObjectAt(0); } + + // Application is going to be closed, shutdown accessibility and mark + // accessibility service as shutdown to prevent calls of its methods. + // Don't null accessibility service static member at this point to be safe + // if someone will try to operate with it. + + NS_ASSERTION(!gIsShutdown, "Accessibility was shutdown already"); + + gIsShutdown = PR_TRUE; + nsAccessNodeWrap::ShutdownAccessibility(); } + return NS_OK; } @@ -182,7 +199,8 @@ NS_IMETHODIMP nsAccessibilityService::OnStateChange(nsIWebProgress *aWebProgress { NS_ASSERTION(aStateFlags & STATE_IS_DOCUMENT, "Other notifications excluded"); - if (!aWebProgress || 0 == (aStateFlags & (STATE_START | STATE_STOP))) { + if (gIsShutdown || !aWebProgress || + (aStateFlags & (STATE_START | STATE_STOP)) == 0) { return NS_OK; } @@ -260,23 +278,26 @@ nsAccessibilityService::FireAccessibleEvent(PRUint32 aEvent, void nsAccessibilityService::StartLoadCallback(nsITimer *aTimer, void *aClosure) { - nsIAccessibilityService *accService = nsAccessNode::GetAccService(); - if (accService) - accService->ProcessDocLoadEvent(aTimer, aClosure, nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_START); + if (gAccessibilityService) + gAccessibilityService-> + ProcessDocLoadEvent(aTimer, aClosure, + nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_START); } void nsAccessibilityService::EndLoadCallback(nsITimer *aTimer, void *aClosure) { - nsIAccessibilityService *accService = nsAccessNode::GetAccService(); - if (accService) - accService->ProcessDocLoadEvent(aTimer, aClosure, nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_COMPLETE); + if (gAccessibilityService) + gAccessibilityService-> + ProcessDocLoadEvent(aTimer, aClosure, + nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_COMPLETE); } void nsAccessibilityService::FailedLoadCallback(nsITimer *aTimer, void *aClosure) { - nsIAccessibilityService *accService = nsAccessNode::GetAccService(); - if (accService) - accService->ProcessDocLoadEvent(aTimer, aClosure, nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_STOPPED); + if (gAccessibilityService) + gAccessibilityService-> + ProcessDocLoadEvent(aTimer, aClosure, + nsIAccessibleEvent::EVENT_DOCUMENT_LOAD_STOPPED); } /* void onProgressChange (in nsIWebProgress aWebProgress, in nsIRequest aRequest, in long aCurSelfProgress, in long aMaxSelfProgress, in long aCurTotalProgress, in long aMaxTotalProgress); */ @@ -1315,7 +1336,7 @@ NS_IMETHODIMP nsAccessibilityService::GetAccessible(nsIDOMNode *aNode, NS_ENSURE_ARG_POINTER(aAccessible); NS_ENSURE_ARG_POINTER(aFrameHint); *aAccessible = nsnull; - if (!aPresShell || !aWeakShell) { + if (!aPresShell || !aWeakShell || gIsShutdown) { return NS_ERROR_FAILURE; } @@ -2056,22 +2077,28 @@ NS_IMETHODIMP nsAccessibilityService::InvalidateSubtreeFor(nsIPresShell *aShell, nsresult nsAccessibilityService::GetAccessibilityService(nsIAccessibilityService** aResult) { - NS_PRECONDITION(aResult != nsnull, "null ptr"); - if (! aResult) - return NS_ERROR_NULL_POINTER; - + NS_ENSURE_TRUE(aResult, NS_ERROR_NULL_POINTER); *aResult = nsnull; - if (!nsAccessibilityService::gAccessibilityService) { + + if (!gAccessibilityService) { gAccessibilityService = new nsAccessibilityService(); - if (!gAccessibilityService ) { - return NS_ERROR_OUT_OF_MEMORY; - } + NS_ENSURE_TRUE(gAccessibilityService, NS_ERROR_OUT_OF_MEMORY); + + gIsShutdown = PR_FALSE; } - *aResult = nsAccessibilityService::gAccessibilityService; - NS_ADDREF(*aResult); + + NS_ADDREF(*aResult = gAccessibilityService); return NS_OK; } +nsIAccessibilityService* +nsAccessibilityService::GetAccessibilityService() +{ + NS_ASSERTION(!gIsShutdown, + "Going to deal with shutdown accessibility service!"); + return gAccessibilityService; +} + nsresult NS_GetAccessibilityService(nsIAccessibilityService** aResult) { diff --git a/accessible/src/base/nsAccessibilityService.h b/accessible/src/base/nsAccessibilityService.h index 6b673d5c13bf..9bd43b7bfc98 100644 --- a/accessible/src/base/nsAccessibilityService.h +++ b/accessible/src/base/nsAccessibilityService.h @@ -84,6 +84,16 @@ public: */ static nsresult GetAccessibilityService(nsIAccessibilityService** aResult); + /** + * Return cached accessibility service. + */ + static nsIAccessibilityService* GetAccessibilityService(); + + /** + * Indicates whether accessibility service was shutdown. + */ + static PRBool gIsShutdown; + private: /** * Return presentation shell, DOM node for the given frame. diff --git a/accessible/src/base/nsDocAccessible.cpp b/accessible/src/base/nsDocAccessible.cpp index 5da571d6bedf..0977c237408d 100644 --- a/accessible/src/base/nsDocAccessible.cpp +++ b/accessible/src/base/nsDocAccessible.cpp @@ -39,7 +39,7 @@ #include "nsRootAccessible.h" #include "nsAccessibilityAtoms.h" #include "nsAccessibleEventData.h" -#include "nsIAccessibilityService.h" +#include "nsAccessibilityService.h" #include "nsIMutableArray.h" #include "nsICommandManager.h" #include "nsIDocShell.h" @@ -670,7 +670,7 @@ nsDocAccessible::Shutdown() // Remove from the cache after other parts of Shutdown(), so that Shutdown() procedures // can find the doc or root accessible in the cache if they need it. // We don't do this during ShutdownAccessibility() because that is already clearing the cache - if (!gIsShuttingDownApp) + if (!nsAccessibilityService::gIsShutdown) gGlobalDocAccessibleCache.Remove(static_cast(kungFuDeathGripDoc)); return NS_OK; diff --git a/accessible/src/msaa/nsAccessNodeWrap.cpp b/accessible/src/msaa/nsAccessNodeWrap.cpp index c0d5b581a524..c58d57e74fcb 100644 --- a/accessible/src/msaa/nsAccessNodeWrap.cpp +++ b/accessible/src/msaa/nsAccessNodeWrap.cpp @@ -593,9 +593,8 @@ __try { void nsAccessNodeWrap::InitAccessibility() { - if (gIsAccessibilityActive) { - return; - } + NS_ASSERTION(!gIsAccessibilityActive, + "Accessibility was initialized already!"); nsCOMPtr prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID)); if (prefBranch) { @@ -623,9 +622,7 @@ void nsAccessNodeWrap::ShutdownAccessibility() NS_IF_RELEASE(gTextEvent); ::DestroyCaret(); - if (!gIsAccessibilityActive) { - return; - } + NS_ASSERTION(gIsAccessibilityActive, "Accessibility was shutdown already!"); nsAccessNode::ShutdownXPAccessibility(); }