Bug 1321935: Make ProxyAccessibleBase::SetChildDoc tolerate replacing one document with another, and add a new ClearChildDoc for removing that document; r=tbsaunde a=jcristau
authorAaron Klotz <aklotz@mozilla.com>
Fri, 02 Dec 2016 14:50:00 -0700
changeset 353085 06fd54cb05583d9ae414d747b03d885e1a09b5b6
parent 353084 4cdf04be5d8eed2c994ff077a534f07400fbaa03
child 353086 70c74e162506f62cbb5023659a7237604c037bd7
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstbsaunde, jcristau
bugs1321935
milestone52.0a2
Bug 1321935: Make ProxyAccessibleBase::SetChildDoc tolerate replacing one document with another, and add a new ClearChildDoc for removing that document; r=tbsaunde a=jcristau MozReview-Commit-ID: 2GgiUPf4gi0
accessible/ipc/DocAccessibleParent.h
accessible/ipc/ProxyAccessibleBase.cpp
accessible/ipc/ProxyAccessibleBase.h
--- a/accessible/ipc/DocAccessibleParent.h
+++ b/accessible/ipc/DocAccessibleParent.h
@@ -106,17 +106,17 @@ public:
                    bool aCreating = true);
 
   /*
    * Called when the document in the content process this object represents
    * notifies the main process a child document has been removed.
    */
   void RemoveChildDoc(DocAccessibleParent* aChildDoc)
   {
-    aChildDoc->Parent()->SetChildDoc(nullptr);
+    aChildDoc->Parent()->ClearChildDoc(aChildDoc);
     mChildDocs.RemoveElement(aChildDoc);
     aChildDoc->mParentDoc = nullptr;
     MOZ_ASSERT(aChildDoc->mChildDocs.Length() == 0);
   }
 
   void RemoveAccessible(ProxyAccessible* aAccessible)
   {
     MOZ_DIAGNOSTIC_ASSERT(mAccessibles.GetEntry(aAccessible->ID()));
--- a/accessible/ipc/ProxyAccessibleBase.cpp
+++ b/accessible/ipc/ProxyAccessibleBase.cpp
@@ -47,25 +47,41 @@ ProxyAccessibleBase<Derived>::Shutdown()
 
   mChildren.Clear();
   ProxyDestroyed(static_cast<Derived*>(this));
   mDoc->RemoveAccessible(static_cast<Derived*>(this));
 }
 
 template <class Derived>
 void
-ProxyAccessibleBase<Derived>::SetChildDoc(DocAccessibleParent* aParent)
+ProxyAccessibleBase<Derived>::SetChildDoc(DocAccessibleParent* aChildDoc)
 {
-  if (aParent) {
-    MOZ_ASSERT(mChildren.IsEmpty());
-    mChildren.AppendElement(aParent);
-    mOuterDoc = true;
+  // DocAccessibleParent::AddChildDoc tolerates replacing one document with
+  // another. We must reflect that here.
+  MOZ_ASSERT(aChildDoc);
+  MOZ_ASSERT(mChildren.Length() <= 1);
+  if (mChildren.IsEmpty()) {
+    mChildren.AppendElement(aChildDoc);
   } else {
-    MOZ_ASSERT(mChildren.Length() == 1);
-    mChildren.Clear();
+    mChildren.ReplaceElementAt(0, aChildDoc);
+  }
+  mOuterDoc = true;
+}
+
+template <class Derived>
+void
+ProxyAccessibleBase<Derived>::ClearChildDoc(DocAccessibleParent* aChildDoc)
+{
+  MOZ_ASSERT(aChildDoc);
+  // This is possible if we're replacing one document with another: Doc 1
+  // has not had a chance to remove itself, but was already replaced by Doc 2
+  // in SetChildDoc(). This could result in two subsequent calls to
+  // ClearChildDoc() even though mChildren.Length() == 1.
+  MOZ_ASSERT(mChildren.Length() <= 1);
+  if (mChildren.RemoveElement(aChildDoc)) {
     mOuterDoc = false;
   }
 }
 
 template <class Derived>
 bool
 ProxyAccessibleBase<Derived>::MustPruneChildren() const
 {
--- a/accessible/ipc/ProxyAccessibleBase.h
+++ b/accessible/ipc/ProxyAccessibleBase.h
@@ -73,17 +73,18 @@ public:
     Parent()->mChildren.IndexOf(static_cast<const Derived*>(this)); }
   uint32_t EmbeddedChildCount() const;
   int32_t IndexOfEmbeddedChild(const Derived* aChild);
   Derived* EmbeddedChildAt(size_t aChildIdx);
   bool MustPruneChildren() const;
 
   void Shutdown();
 
-  void SetChildDoc(DocAccessibleParent*);
+  void SetChildDoc(DocAccessibleParent* aChildDoc);
+  void ClearChildDoc(DocAccessibleParent* aChildDoc);
 
   /**
    * Remove The given child.
    */
   void RemoveChild(Derived* aChild)
     { mChildren.RemoveElement(aChild); }
 
   /**