Bug 529442. Don't hold a frame pointer in nsAccessibilityService::GetAccessible. r=roc,dbolter
authorAlexander Surkov <h<surkov.alexander@gmail.com>
Thu, 19 Nov 2009 12:42:18 +1300
changeset 35026 26a2aacf9e3a
parent 35025 93a0acf68dd6
child 35027 9f90a4c4a5bf
child 35119 93db5ba9356c
push id10413
push userrocallahan@mozilla.com
push date2009-11-18 23:43 +0000
treeherdermozilla-central@26a2aacf9e3a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc, dbolter
bugs529442
milestone1.9.3a1pre
Bug 529442. Don't hold a frame pointer in nsAccessibilityService::GetAccessible. r=roc,dbolter
accessible/src/base/nsAccessibilityService.cpp
--- a/accessible/src/base/nsAccessibilityService.cpp
+++ b/accessible/src/base/nsAccessibilityService.cpp
@@ -1329,17 +1329,17 @@ NS_IMETHODIMP nsAccessibilityService::Ge
   NS_ASSERTION(aNode, "GetAccessible() called with no node.");
 
   *aIsHidden = PR_FALSE;
 
   // Frames can be deallocated when we flush layout, or when we call into code
   // that can flush layout, either directly, or via DOM manipulation, or some
   // CSS styles like :hover. We use the weak frame checks to avoid calling
   // methods on a dead frame pointer.
-  nsWeakFrame weakFrame = *aFrameHint;
+  nsWeakFrame weakFrame(*aFrameHint);
 
 #ifdef DEBUG_A11Y
   // Please leave this in for now, it's a convenient debugging method
   nsAutoString name;
   aNode->GetLocalName(name);
   if (name.LowerCaseEqualsLiteral("h1")) 
     printf("## aaronl debugging tag name\n");
 
@@ -1455,68 +1455,79 @@ NS_IMETHODIMP nsAccessibilityService::Ge
   }
 
   // Check frame to see if it is hidden
   if (!weakFrame.GetFrame() ||
       !weakFrame.GetFrame()->GetStyleVisibility()->IsVisible()) {
     *aIsHidden = PR_TRUE;
   }
 
-  if (*aIsHidden)
+  if (*aIsHidden) {
+    *aFrameHint = weakFrame.GetFrame();
     return NS_OK;
+  }
 
   /**
    * Attempt to create an accessible based on what we know
    */
   if (content->IsNodeOfType(nsINode::eTEXT)) {
     // --- Create HTML for visible text frames ---
     nsIFrame* f = weakFrame.GetFrame();
     if (f && f->IsEmpty()) {
       nsAutoString renderedWhitespace;
       f->GetRenderedText(&renderedWhitespace, nsnull, nsnull, 0, 1);
       if (renderedWhitespace.IsEmpty()) {
         // Really empty -- nothing is rendered
         *aIsHidden = PR_TRUE;
+        *aFrameHint = weakFrame.GetFrame();
         return NS_OK;
       }
     }
     if (weakFrame.IsAlive()) {
       weakFrame.GetFrame()->GetAccessible(getter_AddRefs(newAcc));
     }
-    return InitAccessible(newAcc, aAccessible, nsnull);
+
+    nsresult rv = InitAccessible(newAcc, aAccessible, nsnull);
+    *aFrameHint = weakFrame.GetFrame();
+    return rv;
   }
 
   PRBool isHTML = content->IsHTML();
   if (isHTML && content->Tag() == nsAccessibilityAtoms::map) {
     // Create hyper text accessible for HTML map if it is used to group links
     // (see http://www.w3.org/TR/WCAG10-HTML-TECHS/#group-bypass). If the HTML
     // map doesn't have 'name' attribute (or has empty name attribute) then we
     // suppose it is used for links grouping. Otherwise we think it is used in
     // conjuction with HTML image element and in this case we don't create any
     // accessible for it and don't walk into it. The accessibles for HTML area
     // (nsHTMLAreaAccessible) the map contains are attached as children of the
     // appropriate accessible for HTML image (nsHTMLImageAccessible).
     nsAutoString name;
     content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::name, name);
     if (!name.IsEmpty()) {
       *aIsHidden = PR_TRUE;
+      *aFrameHint = weakFrame.GetFrame();
       return NS_OK;
     }
     
     nsresult rv =
       CreateHyperTextAccessible(weakFrame.GetFrame(), getter_AddRefs(newAcc));
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_FAILED(rv)) {
+      *aFrameHint = weakFrame.GetFrame();
+      return rv;
+    }
   }
 
   nsRoleMapEntry *roleMapEntry = nsAccUtils::GetRoleMapEntry(aNode);
   if (roleMapEntry && !nsCRT::strcmp(roleMapEntry->roleString, "presentation") &&
       !content->IsFocusable()) { // For presentation only
     // Only create accessible for role of "presentation" if it is focusable --
     // in that case we need an accessible in case it gets focused, we
     // don't want focus ever to be 'lost'
+    *aFrameHint = weakFrame.GetFrame();
     return NS_OK;
   }
 
   if (weakFrame.IsAlive() && !newAcc && isHTML) {  // HTML accessibles
     PRBool tryTagNameOrFrame = PR_TRUE;
 
     nsIAtom *frameType = weakFrame.GetFrame()->GetType();
 
@@ -1564,16 +1575,17 @@ NS_IMETHODIMP nsAccessibilityService::Ge
                        "No accessible for parent table and it didn't have role of presentation");
 #endif
 
           if (!roleMapEntry && !content->IsFocusable()) {
             // Table-related descendants of presentation table are also
             // presentation if they aren't focusable and have not explicit ARIA
             // role (don't create accessibles for them unless they need to fire
             // focus events).
+            *aFrameHint = weakFrame.GetFrame();
             return NS_OK;
           }
 
           // otherwise create ARIA based accessible.
           tryTagNameOrFrame = PR_FALSE;
           break;
         }
 
@@ -1611,17 +1623,20 @@ NS_IMETHODIMP nsAccessibilityService::Ge
       // Prefer to use markup (mostly tag name, perhaps attributes) to
       // decide if and what kind of accessible to create.
       // The method creates accessibles for table related content too therefore
       // we do not call it if accessibles for table related content are
       // prevented above.
       nsresult rv =
         CreateHTMLAccessibleByMarkup(weakFrame.GetFrame(), aWeakShell, aNode,
                                      getter_AddRefs(newAcc));
-      NS_ENSURE_SUCCESS(rv, rv);
+      if (NS_FAILED(rv)) {
+        *aFrameHint = weakFrame.GetFrame();
+        return rv;
+      }
 
       if (!newAcc) {
         // Do not create accessible object subtrees for non-rendered table
         // captions. This could not be done in
         // nsTableCaptionFrame::GetAccessible() because the descendants of
         // the table caption would still be created. By setting
         // *aIsHidden = PR_TRUE we ensure that no descendant accessibles are
         // created.
@@ -1629,28 +1644,32 @@ NS_IMETHODIMP nsAccessibilityService::Ge
         if (!f) {
           f = aPresShell->GetRealPrimaryFrameFor(content);
         }
         if (f->GetType() == nsAccessibilityAtoms::tableCaptionFrame &&
            f->GetRect().IsEmpty()) {
           // XXX This is not the ideal place for this code, but right now there
           // is no better place:
           *aIsHidden = PR_TRUE;
+          *aFrameHint = weakFrame.GetFrame();
           return NS_OK;
         }
         f->GetAccessible(getter_AddRefs(newAcc)); // Try using frame to do it
       }
     }
   }
 
   if (!newAcc) {
     // Elements may implement nsIAccessibleProvider via XBL. This allows them to
     // say what kind of accessible to create.
     nsresult rv = GetAccessibleByType(aNode, getter_AddRefs(newAcc));
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_FAILED(rv)) {
+      *aFrameHint = weakFrame.GetFrame();
+      return rv;
+    }
   }
 
   if (!newAcc) {
     // Create generic accessibles for SVG and MathML nodes.
     if (content->GetNameSpaceID() == kNameSpaceID_SVG &&
         content->Tag() == nsAccessibilityAtoms::svg) {
       newAcc = new nsEnumRoleAccessible(aNode, aWeakShell,
                                         nsIAccessibleRole::ROLE_DIAGRAM);
@@ -1683,17 +1702,19 @@ NS_IMETHODIMP nsAccessibilityService::Ge
       CreateHyperTextAccessible(weakFrame.GetFrame(), getter_AddRefs(newAcc));
     }
     else {  // XUL, SVG, MathML etc.
       // Interesting generic non-HTML container
       newAcc = new nsAccessibleWrap(aNode, aWeakShell);
     }
   }
 
-  return InitAccessible(newAcc, aAccessible, roleMapEntry);
+  nsresult rv = InitAccessible(newAcc, aAccessible, roleMapEntry);
+  *aFrameHint = weakFrame.GetFrame();
+  return rv;
 }
 
 PRBool
 nsAccessibilityService::HasUniversalAriaProperty(nsIContent *aContent,
                                                  nsIWeakReference *aWeakShell)
 {
   // ARIA attributes that take token values (NMTOKEN, bool) are special cased.
   return nsAccUtils::HasDefinedARIAToken(aContent, nsAccessibilityAtoms::aria_atomic) ||