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 394510 5c809b2f8930537446845f548626692a9b7caa9e
parent 394509 beca679d682be56233f8ad362ebcf190626be20d
child 394511 05c3c56d71a7266d116afbe23fc1b3459217f2c4
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidb
bugs1340579
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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();
   }