Bug 1261177 - split GetOrCreateAccessible method into two (Get and Create versions), r=yzen
authorAlexander Surkov <surkov.alexander@gmail.com>
Wed, 06 Apr 2016 07:23:41 -0400
changeset 315699 be4f368296b7ba048ca030ad5e5e447ad0456cfe
parent 315698 f33a85c0997b0497f305f9e16bd1e6002746ab49
child 315700 3e2a9133cb5cb70cf5458db451f451de1223ab10
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen
bugs1261177
milestone48.0a1
Bug 1261177 - split GetOrCreateAccessible method into two (Get and Create versions), r=yzen
accessible/base/TreeWalker.cpp
accessible/base/nsAccessibilityService.cpp
accessible/base/nsAccessibilityService.h
accessible/generic/DocAccessible.cpp
--- a/accessible/base/TreeWalker.cpp
+++ b/accessible/base/TreeWalker.cpp
@@ -270,33 +270,33 @@ TreeWalker::Prev()
 
   mPhase = eAtStart;
   return nullptr;
 }
 
 Accessible*
 TreeWalker::AccessibleFor(nsIContent* aNode, uint32_t aFlags, bool* aSkipSubtree)
 {
-  Accessible* child = nullptr;
-  if (aFlags & eWalkCache) {
-    child = mDoc->GetAccessible(aNode);
-  }
-  else if (mContext->IsAcceptableChild(aNode)) {
-    child = GetAccService()->
-      GetOrCreateAccessible(aNode, mContext, aSkipSubtree);
+  // Ignore the accessible and its subtree if it was repositioned by means
+  // of aria-owns.
+  Accessible* child = mDoc->GetAccessible(aNode);
+  if (child) {
+    if (child->IsRelocated()) {
+      *aSkipSubtree = true;
+      return nullptr;
+    }
+    return child;
   }
 
-  // Ignore the accessible and its subtree if it was repositioned by means
-  // of aria-owns.
-  if (child && child->IsRelocated()) {
-    *aSkipSubtree = true;
-    return nullptr;
+  // Create an accessible if allowed.
+  if (!(aFlags & eWalkCache) && mContext->IsAcceptableChild(aNode)) {
+    return GetAccService()->CreateAccessible(aNode, mContext, aSkipSubtree);
   }
 
-  return child;
+  return nullptr;
 }
 
 dom::AllChildrenIterator*
 TreeWalker::PopState()
 {
   size_t length = mStateStack.Length();
   mStateStack.RemoveElementAt(length - 1);
   return mStateStack.IsEmpty() ? nullptr : &mStateStack.LastElement();
--- a/accessible/base/nsAccessibilityService.cpp
+++ b/accessible/base/nsAccessibilityService.cpp
@@ -1005,36 +1005,30 @@ nsAccessibilityService::IsLogged(const n
 
   return NS_OK;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsAccessibilityService public
 
 Accessible*
-nsAccessibilityService::GetOrCreateAccessible(nsINode* aNode,
-                                              Accessible* aContext,
-                                              bool* aIsSubtreeHidden)
+nsAccessibilityService::CreateAccessible(nsINode* aNode,
+                                         Accessible* aContext,
+                                         bool* aIsSubtreeHidden)
 {
-  NS_PRECONDITION(aContext && aNode && !gIsShutdown,
-                  "Maybe let'd do a crash? Oh, yes, baby!");
+  MOZ_ASSERT(aContext, "No context provided");
+  MOZ_ASSERT(aNode, "No node to create an accessible for");
+  MOZ_ASSERT(!gIsShutdown, "No creation after shutdown");
 
   if (aIsSubtreeHidden)
     *aIsSubtreeHidden = false;
 
   DocAccessible* document = aContext->Document();
-
-  // Check to see if we already have an accessible for this node in the cache.
-  // XXX: we don't have context check here. It doesn't really necessary until
-  // we have in-law children adoption.
-  Accessible* cachedAccessible = document->GetAccessible(aNode);
-  if (cachedAccessible)
-    return cachedAccessible;
-
-  // No cache entry, so we must create the accessible.
+  MOZ_ASSERT(!document->GetAccessible(aNode),
+             "We already have an accessible for this node.");
 
   if (aNode->IsNodeOfType(nsINode::eDOCUMENT)) {
     // If it's document node then ask accessible document loader for
     // document accessible, otherwise return null.
     nsCOMPtr<nsIDocument> document(do_QueryInterface(aNode));
     return GetDocAccessible(document);
   }
 
--- a/accessible/base/nsAccessibilityService.h
+++ b/accessible/base/nsAccessibilityService.h
@@ -175,26 +175,25 @@ public:
   // nsAccessibiltiyService
 
   /**
    * Return true if accessibility service has been shutdown.
    */
   static bool IsShutdown() { return gIsShutdown; }
 
   /**
-   * Return an accessible for the given DOM node from the cache or create new
-   * one.
+   * Creates an accessible for the given DOM node.
    *
    * @param  aNode             [in] the given node
    * @param  aContext          [in] context the accessible is created in
    * @param  aIsSubtreeHidden  [out, optional] indicates whether the node's
    *                             frame and its subtree is hidden
    */
-  Accessible* GetOrCreateAccessible(nsINode* aNode, Accessible* aContext,
-                                    bool* aIsSubtreeHidden = nullptr);
+  Accessible* CreateAccessible(nsINode* aNode, Accessible* aContext,
+                               bool* aIsSubtreeHidden = nullptr);
 
   mozilla::a11y::role MarkupRole(const nsIContent* aContent) const
   {
     const mozilla::a11y::MarkupMapInfo* markupMap =
       mMarkupMaps.Get(aContent->NodeInfo()->NameAtom());
     return markupMap ? markupMap->role : mozilla::a11y::roles::NOTHING;
   }
 
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -1810,18 +1810,20 @@ void
 DocAccessible::ProcessContentInserted(Accessible* aContainer, nsIContent* aNode)
 {
   if (!aContainer->IsInDocument()) {
     return;
   }
 
   TreeWalker walker(aContainer);
   if (aContainer->IsAcceptableChild(aNode) && walker.Seek(aNode)) {
-    Accessible* child =
-      GetAccService()->GetOrCreateAccessible(aNode, aContainer);
+    Accessible* child = GetAccessible(aNode);
+    if (!child) {
+      child = GetAccService()->CreateAccessible(aNode, aContainer);
+    }
 
     if (child) {
       RefPtr<AccReorderEvent> reorderEvent = new AccReorderEvent(aContainer);
 
       AutoTreeMutation mt(aContainer);
       aContainer->InsertAfter(child, walker.Prev());
       mt.AfterInsertion(child);
       mt.Done();