bug 1340579 - look up this DocAccessibleParent in live docs instead of using this in DocAccessibleParent::Destroy() r=davidb
authorTrevor Saunders <tbsaunde@tbsaunde.org>
Thu, 23 Feb 2017 13:31:06 -0500
changeset 374560 5c809b2f8930537446845f548626692a9b7caa9e
parent 374559 beca679d682be56233f8ad362ebcf190626be20d
child 374561 05c3c56d71a7266d116afbe23fc1b3459217f2c4
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidb
bugs1340579
milestone54.0a1
bug 1340579 - look up this DocAccessibleParent in live docs instead of using this in DocAccessibleParent::Destroy() r=davidb
accessible/ipc/DocAccessibleParent.cpp
accessible/ipc/DocAccessibleParent.h
dom/ipc/TabParent.cpp
--- a/accessible/ipc/DocAccessibleParent.cpp
+++ b/accessible/ipc/DocAccessibleParent.cpp
@@ -16,16 +16,17 @@
 #include "AccessibleWrap.h"
 #include "Compatibility.h"
 #include "nsWinUtils.h"
 #include "RootAccessible.h"
 #endif
 
 namespace mozilla {
 namespace a11y {
+uint64_t DocAccessibleParent::sMaxDocID = 0;
 
 mozilla::ipc::IPCResult
 DocAccessibleParent::RecvShowEvent(const ShowEventData& aData,
                                    const bool& aFromUser)
 {
   if (mShutdown)
     return IPC_OK();
 
@@ -428,30 +429,29 @@ DocAccessibleParent::AddChildDoc(DocAcce
   // here.
   if (outerDoc->ChildrenCount() > 1 ||
       (outerDoc->ChildrenCount() == 1 && !outerDoc->ChildAt(0)->IsDoc())) {
     return IPC_FAIL(this, "binding to proxy that can't be a outerDoc!");
   }
 
   aChildDoc->SetParent(outerDoc);
   outerDoc->SetChildDoc(aChildDoc);
-  mChildDocs.AppendElement(aChildDoc->IProtocol::Id());
-  aChildDoc->mParentDoc = IProtocol::Id();
+  mChildDocs.AppendElement(aChildDoc->mActorID);
+  aChildDoc->mParentDoc = mActorID;
 
   if (aCreating) {
     ProxyCreated(aChildDoc, Interfaces::DOCUMENT | Interfaces::HYPERTEXT);
   }
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 DocAccessibleParent::RecvShutdown()
 {
-  MOZ_DIAGNOSTIC_ASSERT(LiveDocs().Contains(IProtocol::Id()));
   Destroy();
 
   auto mgr = static_cast<dom::TabParent*>(Manager());
   if (!mgr->IsDestroyed()) {
     if (!PDocAccessibleParent::Send__delete__(this)) {
       return IPC_FAIL_NO_REASON(mgr);
     }
   }
@@ -465,41 +465,70 @@ DocAccessibleParent::Destroy()
   // If we are already shutdown that is because our containing tab parent is
   // shutting down in which case we don't need to do anything.
   if (mShutdown) {
     return;
   }
 
   mShutdown = true;
 
+  MOZ_DIAGNOSTIC_ASSERT(LiveDocs().Contains(mActorID));
   uint32_t childDocCount = mChildDocs.Length();
   for (uint32_t i = 0; i < childDocCount; i++) {
     for (uint32_t j = i + 1; j < childDocCount; j++) {
       MOZ_DIAGNOSTIC_ASSERT(mChildDocs[i] != mChildDocs[j]);
     }
   }
 
-  for (uint32_t i = childDocCount - 1; i < childDocCount; i--)
-    ChildDocAt(i)->Destroy();
+  // XXX This indirection through the hash map of live documents shouldn't be
+  // needed, but be paranoid for now.
+  int32_t actorID = mActorID;
+  for (uint32_t i = childDocCount - 1; i < childDocCount; i--) {
+    DocAccessibleParent* thisDoc = LiveDocs().Get(actorID);
+    MOZ_ASSERT(thisDoc);
+    if (!thisDoc) {
+      return;
+    }
+
+    thisDoc->ChildDocAt(i)->Destroy();
+  }
 
   for (auto iter = mAccessibles.Iter(); !iter.Done(); iter.Next()) {
     MOZ_ASSERT(iter.Get()->mProxy != this);
     ProxyDestroyed(iter.Get()->mProxy);
     iter.Remove();
   }
 
+  DocAccessibleParent* thisDoc = LiveDocs().Get(actorID);
+  MOZ_ASSERT(thisDoc);
+  if (!thisDoc) {
+    return;
+  }
+
   // The code above should have already completely cleared these, but to be
   // extra safe make sure they are cleared here.
-  mAccessibles.Clear();
-  mChildDocs.Clear();
+  thisDoc->mAccessibles.Clear();
+  thisDoc->mChildDocs.Clear();
+
+  DocManager::NotifyOfRemoteDocShutdown(thisDoc);
+  thisDoc = LiveDocs().Get(actorID);
+  MOZ_ASSERT(thisDoc);
+  if (!thisDoc) {
+    return;
+  }
 
-  DocManager::NotifyOfRemoteDocShutdown(this);
-  ProxyDestroyed(this);
-  if (DocAccessibleParent* parentDoc = ParentDoc())
-    parentDoc->RemoveChildDoc(this);
+  ProxyDestroyed(thisDoc);
+  thisDoc = LiveDocs().Get(actorID);
+  MOZ_ASSERT(thisDoc);
+  if (!thisDoc) {
+    return;
+  }
+
+  if (DocAccessibleParent* parentDoc = thisDoc->ParentDoc())
+    parentDoc->RemoveChildDoc(thisDoc);
   else if (IsTopLevel())
     GetAccService()->RemoteDocShutdown(this);
 }
 
 DocAccessibleParent*
 DocAccessibleParent::ParentDoc() const
 {
   if (mParentDoc == kNoParentDoc) {
--- a/accessible/ipc/DocAccessibleParent.h
+++ b/accessible/ipc/DocAccessibleParent.h
@@ -28,20 +28,26 @@ class DocAccessibleParent : public Proxy
 {
 public:
   DocAccessibleParent() :
     ProxyAccessible(this), mParentDoc(kNoParentDoc),
     mTopLevel(false), mShutdown(false)
 #if defined(XP_WIN)
                                       , mEmulatedWindowHandle(nullptr)
 #endif // defined(XP_WIN)
-  { MOZ_COUNT_CTOR_INHERITED(DocAccessibleParent, ProxyAccessible); }
+  {
+    MOZ_COUNT_CTOR_INHERITED(DocAccessibleParent, ProxyAccessible);
+    sMaxDocID++;
+    mActorID = sMaxDocID;
+    MOZ_ASSERT(!LiveDocs().Get(mActorID));
+    LiveDocs().Put(mActorID, this);
+  }
   ~DocAccessibleParent()
   {
-    LiveDocs().Remove(IProtocol::Id());
+    LiveDocs().Remove(mActorID);
     MOZ_COUNT_DTOR_INHERITED(DocAccessibleParent, ProxyAccessible);
     MOZ_ASSERT(mChildDocs.Length() == 0);
     MOZ_ASSERT(!ParentDoc());
   }
 
   void SetTopLevel() { mTopLevel = true; }
   bool IsTopLevel() const { return mTopLevel; }
 
@@ -54,21 +60,16 @@ public:
    */
   void MarkAsShutdown()
   {
     MOZ_ASSERT(mChildDocs.IsEmpty());
     MOZ_ASSERT(mAccessibles.Count() == 0);
     mShutdown = true;
   }
 
-  /**
-   * Add this document to the set of tracked documents.
-   */
-  void AddToMap() { LiveDocs().Put(IProtocol::Id(), this); }
-
   /*
    * Called when a message from a document in a child process notifies the main
    * process it is firing an event.
    */
   virtual mozilla::ipc::IPCResult RecvEvent(const uint64_t& aID, const uint32_t& aType)
     override;
 
   virtual mozilla::ipc::IPCResult RecvShowEvent(const ShowEventData& aData, const bool& aFromUser)
@@ -113,17 +114,17 @@ public:
       Destroy();
   }
 
   /*
    * Return the main processes representation of the parent document (if any)
    * of the document this object represents.
    */
   DocAccessibleParent* ParentDoc() const;
-  static const int32_t kNoParentDoc = INT32_MIN;
+  static const int32_t kNoParentDoc = UINT64_MAX;
 
   /*
    * Called when a document in a content process notifies the main process of a
    * new child document.
    */
   ipc::IPCResult AddChildDoc(DocAccessibleParent* aChildDoc,
                              uint64_t aParentID, bool aCreating = true);
 
@@ -133,17 +134,17 @@ public:
    */
   void RemoveChildDoc(DocAccessibleParent* aChildDoc)
   {
     ProxyAccessible* parent = aChildDoc->Parent();
     MOZ_ASSERT(parent);
     if (parent) {
       aChildDoc->Parent()->ClearChildDoc(aChildDoc);
     }
-    DebugOnly<bool> result = mChildDocs.RemoveElement(aChildDoc->IProtocol::Id());
+    DebugOnly<bool> result = mChildDocs.RemoveElement(aChildDoc->mActorID);
     aChildDoc->mParentDoc = kNoParentDoc;
     MOZ_ASSERT(result);
     MOZ_ASSERT(aChildDoc->mChildDocs.Length() == 0);
   }
 
   void RemoveAccessible(ProxyAccessible* aAccessible)
   {
     MOZ_DIAGNOSTIC_ASSERT(mAccessibles.GetEntry(aAccessible->ID()));
@@ -225,19 +226,21 @@ private:
   HWND mEmulatedWindowHandle;
 #endif
 
   /*
    * Conceptually this is a map from IDs to proxies, but we store the ID in the
    * proxy object so we can't use a real map.
    */
   nsTHashtable<ProxyEntry> mAccessibles;
+  uint64_t mActorID;
   bool mTopLevel;
   bool mShutdown;
 
+  static uint64_t sMaxDocID;
   static nsDataHashtable<nsUint64HashKey, DocAccessibleParent*>&
     LiveDocs()
     {
       static nsDataHashtable<nsUint64HashKey, DocAccessibleParent*> sLiveDocs;
       return sLiveDocs;
     }
 };
 
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -918,17 +918,16 @@ mozilla::ipc::IPCResult
 TabParent::RecvPDocAccessibleConstructor(PDocAccessibleParent* aDoc,
                                          PDocAccessibleParent* aParentDoc,
                                          const uint64_t& aParentID,
                                          const uint32_t& aMsaaID,
                                          const IAccessibleHolder& aDocCOMProxy)
 {
 #ifdef ACCESSIBILITY
   auto doc = static_cast<a11y::DocAccessibleParent*>(aDoc);
-  doc->AddToMap();
 
   // If this tab is already shutting down just mark the new actor as shutdown
   // and ignore it.  When the tab actor is destroyed it will be too.
   if (mIsDestroyed) {
     doc->MarkAsShutdown();
     return IPC_OK();
   }