Bug 543961 - nsAccessibilityService::GetAccessible() shouldn't try to get document accessible from global cache twice, r=davidb, marcoz
--- a/accessible/public/nsIAccessibleDocument.idl
+++ b/accessible/public/nsIAccessibleDocument.idl
@@ -53,17 +53,17 @@ interface nsIDOMWindow;
* You can QueryInterface to nsIAccessibleDocument from
* the nsIAccessible or nsIAccessNode for the root node
* of a document. You can also get one from
* nsIAccessNode::GetAccessibleDocument() or
* nsIAccessibleEvent::GetAccessibleDocument()
*
* @status UNDER_REVIEW
*/
-[scriptable, uuid(b7ae45bd-21e9-4ed5-a67e-86448b25d56b)]
+[scriptable, uuid(427597a3-1737-4743-bf43-2311a1ed5fbd)]
interface nsIAccessibleDocument : nsISupports
{
/**
* The URL of the document
*/
readonly attribute AString URL;
/**
@@ -98,25 +98,16 @@ interface nsIAccessibleDocument : nsISup
/**
* The window handle for the OS window the document is being displayed in.
* For example, in Windows you can static cast it to an HWND.
*/
[noscript] readonly attribute voidPtr windowHandle;
/**
- * Returns the access node cached by this document
- * @param aUniqueID The unique ID used to cache the node.
- * This matches up with the uniqueID attribute on
- * nsIAccessNode.
- * @return The nsIAccessNode cached for this particular unique ID.
- */
- [noscript] nsIAccessNode getCachedAccessNode(in voidPtr aUniqueID);
-
- /**
* Returns the first accessible parent of a DOM node.
* Guaranteed not to return nsnull if the DOM node is in a document.
* @param aDOMNode The DOM node we need an accessible for.
* @param aCanCreate Can accessibles be created or must it be the first
* cached accessible in the parent chain?
* @return An first nsIAccessible found by crawling up the DOM node
* to the document root.
*/
--- a/accessible/public/nsIAccessibleRetrieval.idl
+++ b/accessible/public/nsIAccessibleRetrieval.idl
@@ -51,17 +51,17 @@ interface nsIDOMDOMStringList;
* An interface for in-process accessibility clients
* wishing to get an nsIAccessible or nsIAccessNode for
* a given DOM node.
* More documentation at:
* http://www.mozilla.org/projects/ui/accessibility
*
* @status UNDER_REVIEW
*/
-[scriptable, uuid(7eb49afb-6298-4ce6-816f-9615936540f4)]
+[scriptable, uuid(5f5a6f98-835d-4771-8bfe-a0c7cdc85fec)]
interface nsIAccessibleRetrieval : nsISupports
{
/**
* Return an nsIAccessible for a DOM node in pres shell 0.
* Create a new accessible of the appropriate type if necessary,
* or use one from the accessibility cache if it already exists.
* @param aNode The DOM node to get an accessible for.
* @return The nsIAccessible for the given DOM node.
@@ -96,36 +96,16 @@ interface nsIAccessibleRetrieval : nsISu
* Return an nsIAccessible for a DOM node in the given pres shell.
* Create a new accessible of the appropriate type if necessary,
* or use one from the accessibility cache if it already exists.
* @param aNode The DOM node to get an accessible for.
* @param aPresShell The presentation shell which contains layout info for the DOM node.
* @return The nsIAccessible for the given DOM node.
*/
nsIAccessible getAccessibleInShell(in nsIDOMNode aNode, in nsIPresShell aPresShell);
-
- /**
- * Return an nsIAccessNode for an already created DOM node in the given weak shell.
- * Does not create a new one -- only returns cached access nodes.
- * @param aNode The DOM node to get an access node for.
- * @param aPresShell The presentation shell which contains layout info for the DOM node.
- * @return The nsIAccessNode for the given DOM node or null if
- * an access node does not already exist for this DOM node.
- */
- nsIAccessNode getCachedAccessNode(in nsIDOMNode aNode, in nsIWeakReference aShell);
-
- /**
- * Return an nsIAccessible for an already created DOM node in the given weak shell.
- * Does not create a new one -- only returns cached accessibles.
- * @param aNode The DOM node to get an accessible for.
- * @param aPresShell The presentation shell which contains layout info for the DOM node.
- * @return The nsIAccessible for the given DOM node or null if
- * an accessible does not already exist for this DOM node.
- */
- nsIAccessible getCachedAccessible(in nsIDOMNode aNode, in nsIWeakReference aShell);
/**
* Returns accessible role as a string.
*
* @param aRole - the accessible role constants.
*/
AString getStringRole(in unsigned long aRole);
--- a/accessible/src/base/nsAccessNode.cpp
+++ b/accessible/src/base/nsAccessNode.cpp
@@ -459,19 +459,18 @@ nsAccessNode::ScrollToPoint(PRUint32 aCo
return NS_OK;
}
nsresult
nsAccessNode::MakeAccessNode(nsIDOMNode *aNode, nsIAccessNode **aAccessNode)
{
*aAccessNode = nsnull;
- nsCOMPtr<nsIAccessNode> accessNode;
- GetAccService()->GetCachedAccessNode(aNode, mWeakShell,
- getter_AddRefs(accessNode));
+ nsCOMPtr<nsIAccessNode> accessNode =
+ GetAccService()->GetCachedAccessNode(aNode, mWeakShell);
if (!accessNode) {
nsCOMPtr<nsIAccessible> accessible;
GetAccService()->GetAccessibleInWeakShell(aNode, mWeakShell,
getter_AddRefs(accessible));
accessNode = do_QueryInterface(accessible);
}
--- a/accessible/src/base/nsAccessibilityService.cpp
+++ b/accessible/src/base/nsAccessibilityService.cpp
@@ -933,40 +933,29 @@ nsAccessibilityService::CreateHTMLCaptio
*_retval = new nsHTMLCaptionAccessible(node, weakShell);
if (! *_retval)
return NS_ERROR_OUT_OF_MEMORY;
NS_ADDREF(*_retval);
return NS_OK;
}
-NS_IMETHODIMP nsAccessibilityService::GetCachedAccessible(nsIDOMNode *aNode,
- nsIWeakReference *aWeakShell,
- nsIAccessible **aAccessible)
-{
- nsCOMPtr<nsIAccessNode> accessNode;
- nsresult rv = GetCachedAccessNode(aNode, aWeakShell, getter_AddRefs(accessNode));
- nsCOMPtr<nsIAccessible> accessible(do_QueryInterface(accessNode));
- NS_IF_ADDREF(*aAccessible = accessible);
- return rv;
-}
-
-NS_IMETHODIMP nsAccessibilityService::GetCachedAccessNode(nsIDOMNode *aNode,
- nsIWeakReference *aWeakShell,
- nsIAccessNode **aAccessNode)
+already_AddRefed<nsIAccessNode>
+nsAccessibilityService::GetCachedAccessNode(nsIDOMNode *aNode,
+ nsIWeakReference *aWeakShell)
{
nsCOMPtr<nsIAccessibleDocument> accessibleDoc =
nsAccessNode::GetDocAccessibleFor(aWeakShell);
- if (!accessibleDoc) {
- *aAccessNode = nsnull;
- return NS_ERROR_FAILURE;
- }
+ if (!accessibleDoc)
+ return nsnull;
- return accessibleDoc->GetCachedAccessNode(static_cast<void*>(aNode), aAccessNode);
+ nsRefPtr<nsDocAccessible> docAccessible =
+ nsAccUtils::QueryObject<nsDocAccessible>(accessibleDoc);
+ return docAccessible->GetCachedAccessNode(static_cast<void*>(aNode));
}
NS_IMETHODIMP
nsAccessibilityService::GetStringRole(PRUint32 aRole, nsAString& aString)
{
if ( aRole >= NS_ARRAY_LENGTH(kRoleNames)) {
aString.AssignLiteral("unknown");
return NS_OK;
@@ -1289,54 +1278,50 @@ nsAccessibilityService::GetAccessible(ns
nsCOMPtr<nsIDOMElement> element(do_QueryInterface(aNode));
if (element) {
element->GetAttribute(NS_LITERAL_STRING("type"), attrib);
if (attrib.EqualsLiteral("statusbarpanel"))
printf("## aaronl debugging attribute\n");
}
#endif
- // Check to see if we already have an accessible for this
- // node in the cache
- nsCOMPtr<nsIAccessNode> accessNode;
- GetCachedAccessNode(aNode, aWeakShell, getter_AddRefs(accessNode));
+ // Check to see if we already have an accessible for this node in the cache.
+ nsCOMPtr<nsIAccessNode> cachedAccessNode =
+ GetCachedAccessNode(aNode, aWeakShell);
+ if (cachedAccessNode) {
+ // XXX: the cache might contain the access node for the DOM node that is not
+ // accessible because of wrong cache update. In this case try to create new
+ // accessible.
+ CallQueryInterface(cachedAccessNode, aAccessible);
+ NS_ASSERTION(*aAccessible, "Bad cache update!");
+ if (*aAccessible)
+ return NS_OK;
+ }
nsCOMPtr<nsIAccessible> newAcc;
- if (accessNode) {
- // Retrieved from cache. QI might not succeed if it's a node that's not
- // accessible. In this case try to create new accessible because one and
- // the same DOM node may be accessible or not in time (for example,
- // when it is visible or hidden).
- newAcc = do_QueryInterface(accessNode);
- if (newAcc) {
- NS_ADDREF(*aAccessible = newAcc);
- return NS_OK;
- }
- }
-
nsCOMPtr<nsIContent> content(do_QueryInterface(aNode));
// No cache entry, so we must create the accessible
// Check to see if hidden first
nsCOMPtr<nsIDocument> nodeIsDoc;
if (!content) {
// This happens when we're on the document node, which will not QI to an
// nsIContent.
nodeIsDoc = do_QueryInterface(aNode);
NS_ENSURE_TRUE(nodeIsDoc, NS_ERROR_FAILURE); // No content, and not doc node
+#ifdef DEBUG
+ // XXX: remove me if you don't see an assertion.
nsCOMPtr<nsIAccessibleDocument> accessibleDoc =
- nsAccessNode::GetDocAccessibleFor(aWeakShell);
- if (accessibleDoc) {
- newAcc = do_QueryInterface(accessibleDoc);
- NS_ASSERTION(newAcc, "nsIAccessibleDocument is not an nsIAccessible");
- }
- else {
- CreateRootAccessible(aPresShell, nodeIsDoc, getter_AddRefs(newAcc)); // Does Init() for us
- }
+ nsAccessNode::GetDocAccessibleFor(nodeIsDoc);
+ NS_ASSERTION(!accessibleDoc,
+ "Trying to create already cached accessible document!");
+#endif
+
+ CreateRootAccessible(aPresShell, nodeIsDoc, getter_AddRefs(newAcc)); // Does Init() for us
NS_IF_ADDREF(*aAccessible = newAcc);
return NS_OK;
}
NS_ASSERTION(content->GetDocument(), "Creating accessible for node with no document");
if (!content->GetDocument()) {
return NS_ERROR_FAILURE;
@@ -1380,17 +1365,20 @@ nsAccessibilityService::GetAccessible(ns
if (imageFrame && areaElmt) {
nsCOMPtr<nsIAccessible> imageAcc;
CreateHTMLImageAccessible(weakFrame.GetFrame(), getter_AddRefs(imageAcc));
if (imageAcc) {
// cache children
PRInt32 childCount;
imageAcc->GetChildCount(&childCount);
// area accessible should be in cache now
- return GetCachedAccessible(aNode, aWeakShell, aAccessible);
+ nsCOMPtr<nsIAccessNode> cachedAreaAcc =
+ GetCachedAccessNode(aNode, aWeakShell);
+ if (cachedAreaAcc)
+ CallQueryInterface(cachedAreaAcc, aAccessible);
}
}
return NS_OK;
}
}
}
--- a/accessible/src/base/nsAccessibilityService.h
+++ b/accessible/src/base/nsAccessibilityService.h
@@ -105,16 +105,27 @@ public:
* @param aNode [in] the given node.
* @param aPresShell [in] the presentation shell of the given node.
* @param aAccessible [out] the nsIAccessible for the given node.
*/
nsresult GetAccessibleInWeakShell(nsIDOMNode *aNode,
nsIWeakReference *aPresShell,
nsIAccessible **aAccessible);
+ /**
+ * Return an access node for the DOM node in the given presentation shell if
+ * the access node already exists, otherwise null.
+ *
+ * @param aNode [in] the DOM node to get an access node for
+ * @param aPresShell [in] the presentation shell which contains layout info
+ * for the DOM node
+ */
+ already_AddRefed<nsIAccessNode> GetCachedAccessNode(nsIDOMNode *aNode,
+ nsIWeakReference *aShell);
+
private:
/**
* Return presentation shell, DOM node for the given frame.
*
* @param aFrame - the given frame
* @param aShell [out] - presentation shell for DOM node associated with the
* given frame
* @param aContent [out] - DOM node associated with the given frame
--- a/accessible/src/base/nsDocAccessible.cpp
+++ b/accessible/src/base/nsDocAccessible.cpp
@@ -534,34 +534,48 @@ NS_IMETHODIMP nsDocAccessible::GetAssoci
PRBool isEditable;
editor->GetIsDocumentEditable(&isEditable);
if (isEditable) {
NS_ADDREF(*aEditor = editor);
}
return NS_OK;
}
-NS_IMETHODIMP nsDocAccessible::GetCachedAccessNode(void *aUniqueID, nsIAccessNode **aAccessNode)
+already_AddRefed<nsIAccessNode>
+nsDocAccessible::GetCachedAccessNode(void *aUniqueID)
{
- GetCacheEntry(mAccessNodeCache, aUniqueID, aAccessNode); // Addrefs for us
+ nsIAccessNode* accessNode = nsnull;
+ GetCacheEntry(mAccessNodeCache, aUniqueID, &accessNode);
+
+ // No accessible in the cache, check if the given ID is unique ID of this
+ // document accesible.
+ if (!accessNode) {
+ void* thisUniqueID = nsnull;
+ GetUniqueID(&thisUniqueID);
+ if (thisUniqueID == aUniqueID) {
+ NS_ADDREF(this);
+ return this;
+ }
+ }
+
#ifdef DEBUG
// All cached accessible nodes should be in the parent
// It will assert if not all the children were created
// when they were first cached, and no invalidation
// ever corrected parent accessible's child cache.
nsRefPtr<nsAccessible> acc =
- nsAccUtils::QueryObject<nsAccessible>(*aAccessNode);
+ nsAccUtils::QueryObject<nsAccessible>(accessNode);
if (acc) {
nsAccessible* parent(acc->GetCachedParent());
if (parent)
parent->TestChildCache(acc);
}
#endif
- return NS_OK;
+ return accessNode;
}
// nsDocAccessible public method
void
nsDocAccessible::CacheAccessNode(void *aUniqueID, nsIAccessNode *aAccessNode)
{
// If there is an access node for the given unique ID then let's shutdown it.
// The unique ID may be presented in the cache if originally we created
@@ -596,19 +610,16 @@ nsresult
nsDocAccessible::Init()
{
PutCacheEntry(gGlobalDocAccessibleCache, mDocument, this);
AddEventListeners();
GetParent(); // Ensure outer doc mParent accessible.
- nsresult rv = nsHyperTextAccessibleWrap::Init();
- NS_ENSURE_SUCCESS(rv, rv);
-
// Initialize event queue.
mEventQueue = new nsAccEventQueue(this);
// Fire reorder event to notify new accessible document has been created and
// attached to the tree.
nsRefPtr<nsAccEvent> reorderEvent =
new nsAccReorderEvent(mParent, PR_FALSE, PR_TRUE, mDOMNode);
NS_ENSURE_TRUE(reorderEvent, NS_ERROR_OUT_OF_MEMORY);
@@ -1758,18 +1769,17 @@ nsDocAccessible::ProcessPendingEvent(nsA
}
}
}
}
}
void nsDocAccessible::InvalidateChildrenInSubtree(nsIDOMNode *aStartNode)
{
- nsCOMPtr<nsIAccessNode> accessNode;
- GetCachedAccessNode(aStartNode, getter_AddRefs(accessNode));
+ nsCOMPtr<nsIAccessNode> accessNode = GetCachedAccessNode(aStartNode);
nsRefPtr<nsAccessible> acc(nsAccUtils::QueryAccessible(accessNode));
if (acc)
acc->InvalidateChildren();
// Invalidate accessible children in the DOM subtree
nsCOMPtr<nsINode> node = do_QueryInterface(aStartNode);
PRInt32 index, numChildren = node->GetChildCount();
for (index = 0; index < numChildren; index ++) {
@@ -1780,18 +1790,17 @@ void nsDocAccessible::InvalidateChildren
}
void nsDocAccessible::RefreshNodes(nsIDOMNode *aStartNode)
{
if (mAccessNodeCache.Count() <= 1) {
return; // All we have is a doc accessible. There is nothing to invalidate, quit early
}
- nsCOMPtr<nsIAccessNode> accessNode;
- GetCachedAccessNode(aStartNode, getter_AddRefs(accessNode));
+ nsCOMPtr<nsIAccessNode> accessNode = GetCachedAccessNode(aStartNode);
// Shut down accessible subtree, which may have been created for
// anonymous content subtree
nsCOMPtr<nsIAccessible> accessible(do_QueryInterface(accessNode));
if (accessible) {
// Fire menupopup end if a menu goes away
PRUint32 role = nsAccUtils::Role(accessible);
if (role == nsIAccessibleRole::ROLE_MENUPOPUP) {
@@ -1937,18 +1946,17 @@ nsDocAccessible::InvalidateCacheSubtree(
return;
}
// else: user input, so we must fall through and for full handling,
// e.g. fire the mutation events. Note: user input could cause DOM_CREATE
// during page load if user typed into an input field or contentEditable area
}
// Update last change state information
- nsCOMPtr<nsIAccessNode> childAccessNode;
- GetCachedAccessNode(childNode, getter_AddRefs(childAccessNode));
+ nsCOMPtr<nsIAccessNode> childAccessNode = GetCachedAccessNode(childNode);
nsCOMPtr<nsIAccessible> childAccessible = do_QueryInterface(childAccessNode);
#ifdef DEBUG_A11Y
nsAutoString localName;
childNode->GetLocalName(localName);
const char *hasAccessible = childAccessible ? " (acc)" : "";
if (aChangeType == nsIAccessibilityService::FRAME_HIDE)
printf("[Hide %s %s]\n", NS_ConvertUTF16toUTF8(localName).get(), hasAccessible);
@@ -2119,18 +2127,17 @@ nsDocAccessible::GetAccessibleInParentCh
if (NS_SUCCEEDED(GetAccService()->GetRelevantContentNodeFor(currentNode, getter_AddRefs(relevantNode))) && relevantNode) {
currentNode = relevantNode;
}
if (aCanCreate) {
GetAccService()->GetAccessibleInWeakShell(currentNode, mWeakShell,
aAccessible);
}
else { // Only return cached accessibles, don't create anything
- nsCOMPtr<nsIAccessNode> accessNode;
- GetCachedAccessNode(currentNode, getter_AddRefs(accessNode)); // AddRefs
+ nsCOMPtr<nsIAccessNode> accessNode = GetCachedAccessNode(currentNode);
if (accessNode) {
CallQueryInterface(accessNode, aAccessible); // AddRefs
}
}
} while (!*aAccessible);
return NS_OK;
}
@@ -2144,18 +2151,17 @@ nsDocAccessible::FireShowHideEvents(nsID
EIsFromUserInput aIsFromUserInput)
{
NS_ENSURE_ARG(aDOMNode);
nsCOMPtr<nsIAccessible> accessible;
if (!aAvoidOnThisNode) {
if (aEventType == nsIAccessibleEvent::EVENT_HIDE) {
// Don't allow creation for accessibles when nodes going away
- nsCOMPtr<nsIAccessNode> accessNode;
- GetCachedAccessNode(aDOMNode, getter_AddRefs(accessNode));
+ nsCOMPtr<nsIAccessNode> accessNode = GetCachedAccessNode(aDOMNode);
accessible = do_QueryInterface(accessNode);
} else {
// Allow creation of new accessibles for show events
GetAccService()->GetAttachedAccessibleFor(aDOMNode,
getter_AddRefs(accessible));
}
}
--- a/accessible/src/base/nsDocAccessible.h
+++ b/accessible/src/base/nsDocAccessible.h
@@ -146,16 +146,28 @@ public:
*
* @param aContent [in] the child that is changing
* @param aEvent [in] the event from nsIAccessibleEvent that caused
* the change.
*/
void InvalidateCacheSubtree(nsIContent *aContent, PRUint32 aEvent);
/**
+ * Return the cached access node by the given unique ID if it's in subtree of
+ * this document accessible or the document accessible itself, otherwise null.
+ *
+ * @note the unique ID matches with the uniqueID attribute on nsIAccessNode
+ *
+ * @param aUniqueID [in] the unique ID used to cache the node.
+ *
+ * @return the access node object
+ */
+ already_AddRefed<nsIAccessNode> GetCachedAccessNode(void *aUniqueID);
+
+ /**
* Cache access node.
*
* @param aUniquID [in] the unique identifier of accessible
* @param aAccessNode [in] accessible to cache
*/
void CacheAccessNode(void *aUniqueID, nsIAccessNode *aAccessNode);
/**
--- a/accessible/src/msaa/nsDocAccessibleWrap.cpp
+++ b/accessible/src/msaa/nsDocAccessibleWrap.cpp
@@ -281,26 +281,27 @@ struct nsSearchAccessibleInCacheArg
nsCOMPtr<nsIAccessNode> mAccessNode;
void *mUniqueID;
};
static PLDHashOperator
SearchAccessibleInCache(const void* aKey, nsIAccessNode* aAccessNode,
void* aUserArg)
{
- nsCOMPtr<nsIAccessibleDocument> docAccessible(do_QueryInterface(aAccessNode));
- NS_ASSERTION(docAccessible,
+ nsCOMPtr<nsIAccessibleDocument> accessibleDoc(do_QueryInterface(aAccessNode));
+ NS_ASSERTION(accessibleDoc,
"No doc accessible for the object in doc accessible cache!");
+ nsRefPtr<nsDocAccessible> docAccessible =
+ nsAccUtils::QueryObject<nsDocAccessible>(accessibleDoc);
if (docAccessible) {
nsSearchAccessibleInCacheArg* arg =
static_cast<nsSearchAccessibleInCacheArg*>(aUserArg);
- nsCOMPtr<nsIAccessNode> accessNode;
- docAccessible->GetCachedAccessNode(arg->mUniqueID,
- getter_AddRefs(accessNode));
+ nsCOMPtr<nsIAccessNode> accessNode =
+ docAccessible->GetCachedAccessNode(arg->mUniqueID);
if (accessNode) {
arg->mAccessNode = accessNode;
return PL_DHASH_STOP;
}
}
return PL_DHASH_NEXT;
}